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

@Interleaved(lazy=false) is still lazy for SqlSpannerQuery #2165

Closed
s13o opened this issue Jan 31, 2020 · 4 comments
Closed

@Interleaved(lazy=false) is still lazy for SqlSpannerQuery #2165

s13o opened this issue Jan 31, 2020 · 4 comments
Assignees
Labels

Comments

@s13o
Copy link
Contributor

s13o commented Jan 31, 2020

There is one more place where @Interleaved(lazy=false) still not supported even after the merge of #2110. The method SqlSpannerQuery#executeReadSql will execute a raw SQL without injection of ARRAY (SELECT AS STRUCT ...) for Interleaved lists. But we can do it more or less safely in a method SpannerStatementQueryExecutor.applySortingPagingQueryOptions line 151 by replacing the

 new StringBuilder("SELECT * FROM (").append(sql).append(")"), (o) -> {

with something like

 new StringBuilder("SELECT *").append(<inject Interleaved here>).appenr(" FROM (").append(sql).append(")"), (o) -> {

I'm afraid that it caused a bug with @Query annotation now - @Interleaved(lazy=false) fields are not "proxied" anymore so they will be not loaded from the database, never, am I right?

@dmitry-s
Copy link
Contributor

dmitry-s commented Feb 3, 2020

@Query annotation is intended to be used with custom projection queries. I don't think we want to modify a user provided query, because users might want to retrieve a limited set of fields for performance reasons.

@s13o could you clarify your use case? Is it possible to use name-based query method instead of @Query annotated one?

Thanks

@s13o
Copy link
Contributor Author

s13o commented Feb 3, 2020

  1. We do modify the SQL provided by the @Query. Please, take a look on SqlSpannerQuery#executeReadSql line 245.
  2. We can at least add a "fetchEagerInterleaved" option into the @Query. But, as for me, more natural will be to have skipInterleaved with default value true to skip fetching any Interleaved at all. But if skipnterleaved=false we should fetch them as expected from the definition of @Interleaved annotation.
  3. Let's have a look on an entity like this
@Table(name = "Entity")
public class Entity {
    final UUID id;
    
    @Interleaved(lazy = true)
    private List<ApocProviderHist> lazyInterleaved;
    
    @Interleaved(lazy = false)
    private List<ApocProviderHist> eagerInterleaved;
}

I'm afraid today with @Query("select * from Entity") we will have a list of Entities with a correctly proxied lazyInterleaved fields but eagerInterleaved will always be empty. If it is true - It is not expected behavior and it could affect already implemented logics (it seems we should always "lazy" load any type of interleaved lists for backward compatibility even when skipnterleaved=true. In this case - why not to implement skipnterleaved=false ?)

@dmitry-s
Copy link
Contributor

dmitry-s commented Feb 3, 2020

eagerInterleaved will be retrieved lazily in that case. You can see that here https://github.com/spring-cloud/spring-cloud-gcp/blob/9ce0a8094825c4cccc7433ab9aed4d942d594058/spring-cloud-gcp-data-spanner/src/main/java/org/springframework/cloud/gcp/data/spanner/core/SpannerTemplate.java#L588

If you want to give it a try, feel free to propose a PR. By default we would prefer to have the eager fetch disabled for @Query methods, because it is not obvious for users.

@s13o
Copy link
Contributor Author

s13o commented Feb 3, 2020

I'm not sure that I understood right why propertyValue == null means that the property was not loaded - what if the field of the bean has default value with an empty list? But I'll try.

s13o added a commit to s13o/spring-cloud-gcp that referenced this issue Feb 4, 2020
s13o added a commit to s13o/spring-cloud-gcp that referenced this issue Feb 4, 2020
s13o added a commit to s13o/spring-cloud-gcp that referenced this issue Feb 6, 2020
s13o added a commit to s13o/spring-cloud-gcp that referenced this issue Feb 6, 2020
s13o added a commit to s13o/spring-cloud-gcp that referenced this issue Feb 6, 2020
s13o added a commit to s13o/spring-cloud-gcp that referenced this issue Feb 7, 2020
s13o added a commit to s13o/spring-cloud-gcp that referenced this issue Feb 11, 2020
s13o added a commit to s13o/spring-cloud-gcp that referenced this issue Feb 18, 2020
s13o added a commit to s13o/spring-cloud-gcp that referenced this issue Feb 18, 2020
s13o added a commit to s13o/spring-cloud-gcp that referenced this issue Feb 18, 2020
s13o added a commit to s13o/spring-cloud-gcp that referenced this issue Feb 18, 2020
s13o added a commit to s13o/spring-cloud-gcp that referenced this issue Feb 18, 2020
s13o added a commit to s13o/spring-cloud-gcp that referenced this issue Feb 18, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
Development

No branches or pull requests

3 participants