Skip to content
This repository has been archived by the owner on Jan 19, 2022. It is now read-only.

@Interleaved(lazy=false) should use Joins for Spanner #2110

Closed
s13o opened this issue Jan 9, 2020 · 12 comments · Fixed by #2132
Closed

@Interleaved(lazy=false) should use Joins for Spanner #2110

s13o opened this issue Jan 9, 2020 · 12 comments · Fixed by #2132
Assignees

Comments

@s13o
Copy link
Contributor

s13o commented Jan 9, 2020

Spanner Template always retries an interleaved properties of an entity by an additional query per each row instead of using joins. It causes huge query performance degradation and makes "@interleaved(lazy=false)" useless - it actually always "more or less lazy" now.

@meltsufin
Copy link
Contributor

Indeed, this is a useful feature request for eagerly loaded relationships.
One way around it is to create a separate repository for the child entity and query them by parent ID.
Alternatively, this is a feature already supported in Hibernate. See: https://github.com/GoogleCloudPlatform/google-cloud-spanner-hibernate.

@meltsufin meltsufin added the P1 label Jan 9, 2020
@s13o
Copy link
Contributor Author

s13o commented Jan 9, 2020

A separate child-repository did not help too much when we querying for a collection.
Well, the Spring Data for Spanner is basically enough. But this "permanent lazy loading" with the method "queryAndResolveChildren" dramatically affects find-methods of the SpannerRepository implementation together with custom @queries. It forced us to use directly template#query(Function<Struct, A>, Statement, options) instead and to write dozens of weird lines of code.

@dmitry-s
Copy link
Contributor

It can be implemented to execute queries like this in case when lazy=false
SELECT Singerid, Name, ARRAY(SELECT AS STRUCT * FROM ALBUMS WHERE Singerid = Singerid) FROM Singers

@s13o
Copy link
Contributor Author

s13o commented Jan 11, 2020

That is how we are doing now. But I'm not sure about the performance of it compare to JOIN. Which way is preferable?

@dmitry-s
Copy link
Contributor

That's something we would have to test. From the implementation viewpoint it is much better to use this query then JOIN, but only makes sense if the performance is about the same. I will post results when I have them.

@dmitry-s
Copy link
Contributor

I made some tests and they show that the sub-query approach is more efficient than the corresponding join-based query.

@s13o
Copy link
Contributor Author

s13o commented Jan 24, 2020

I also have a PR for it, ready for 80-90%, remained to update tests and plan future improvements - we need an option to manage sql-where for "interleaved".
It will be nice to coordinate our efforts, for me, it is a billable time in DaVita

@dmitry-s
Copy link
Contributor

@s13o
My pr is done, minus the docs update.
Please let us know if you are working on something, so we can reduce efforts duplication, and I will notify you when I'm working on issues created by you.

As for future improvements, could you create an issue for that so we can discuss?

Thanks

@meltsufin
Copy link
Contributor

@s13o We definitely need to communicate better going forward to avoid duplicating effort with community contributors.
We appreciate your involvement and would be interested in you helping us review the PR that @dmitry-s put up.

@s13o
Copy link
Contributor Author

s13o commented Jan 24, 2020

I have no idea how to do it better, but the comments are not the best way. Can you suggest me what to do?
I'd seen the PR (but nit detailed yet) of Dmitry-s about this issue - we even used similar names in our branches :)

@s13o
Copy link
Contributor Author

s13o commented Jan 31, 2020

Nice job!
When we can have a release of it?

@dmitry-s
Copy link
Contributor

The next Spring Cloud release is Hoxton.SR2 and it is scheduled for February 28, 2020.
See this page for updates.

Currently you should be able to use a 1.3.0.BUILD-SNAPSHOT release which includes these changes. You'll have to add this to your pom file:

<repository>
	<id>spring-snapshots</id>
	<name>Spring Snapshots</name>
	<url>https://repo.spring.io/libs-snapshot-local</url>
	<snapshots>
		<enabled>true</enabled>
	</snapshots>
</repository>

Let us know if you have any questions.

Thanks

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Development

Successfully merging a pull request may close this issue.

3 participants