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

Spanner array param binding #1703

Merged
merged 6 commits into from
Jun 20, 2019
Merged

Spanner array param binding #1703

merged 6 commits into from
Jun 20, 2019

Conversation

ChengyuanZhao
Copy link
Contributor

fixes #1701
array param binding in queries. and IN clause

@ChengyuanZhao ChengyuanZhao self-assigned this Jun 18, 2019
@ChengyuanZhao
Copy link
Contributor Author

The Spanner module builds, but the Streams module fails.....

@dmitry-s

@ChengyuanZhao
Copy link
Contributor Author

@dmitry-s @meltsufin PTAL

@codecov
Copy link

codecov bot commented Jun 19, 2019

Codecov Report

Merging #1703 into master will increase coverage by 0.08%.
The diff coverage is 87.23%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #1703      +/-   ##
============================================
+ Coverage      73.9%   73.98%   +0.08%     
- Complexity     1687     1699      +12     
============================================
  Files           204      204              
  Lines          6193     6232      +39     
  Branches        661      668       +7     
============================================
+ Hits           4577     4611      +34     
- Misses         1299     1303       +4     
- Partials        317      318       +1
Flag Coverage Δ Complexity Δ
#unittests 73.98% <87.23%> (+0.08%) 1699 <17> (+12) ⬆️
Impacted Files Coverage Δ Complexity Δ
...k/cloud/gcp/data/spanner/core/SpannerTemplate.java 73.07% <ø> (ø) 73 <0> (ø) ⬇️
...spanner/repository/query/AbstractSpannerQuery.java 41.17% <ø> (ø) 5 <0> (ø) ⬇️
...vert/ConverterAwareMappingSpannerEntityWriter.java 95.03% <ø> (ø) 50 <0> (ø) ⬇️
...a/spanner/repository/query/SpannerQueryMethod.java 0% <0%> (ø) 0 <0> (ø) ⬇️
...t/ConverterAwareMappingSpannerEntityProcessor.java 73.58% <100%> (ø) 22 <0> (ø) ⬇️
...data/spanner/repository/query/SqlSpannerQuery.java 84.12% <100%> (+0.52%) 28 <3> (+2) ⬆️
...spanner/repository/query/PartTreeSpannerQuery.java 44% <66.66%> (+4.86%) 8 <0> (ø) ⬇️
...epository/query/SpannerStatementQueryExecutor.java 84.95% <89.18%> (+0.46%) 56 <14> (+10) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bd0af62...b51181b. Read the comment docs.

@codecov
Copy link

codecov bot commented Jun 19, 2019 •
edited
Loading

Codecov Report

Merging #1703 into master will increase coverage by 0.05%.
The diff coverage is 87.03%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master    #1703      +/-   ##
============================================
+ Coverage      73.9%   73.95%   +0.05%     
- Complexity     1687     1699      +12     
============================================
  Files           204      204              
  Lines          6193     6232      +39     
  Branches        661      668       +7     
============================================
+ Hits           4577     4609      +32     
- Misses         1299     1305       +6     
- Partials        317      318       +1
Flag Coverage Δ Complexity Δ
#unittests 73.95% <87.03%> (+0.05%) 1699 <18> (+12) ⬆️
Impacted Files Coverage Δ Complexity Δ
...k/cloud/gcp/data/spanner/core/SpannerTemplate.java 73.07% <ø> (ø) 73 <0> (ø) ⬇️
...spanner/repository/query/AbstractSpannerQuery.java 41.17% <ø> (ø) 5 <0> (ø) ⬇️
...a/spanner/repository/query/SpannerQueryMethod.java 0% <0%> (ø) 0 <0> (ø) ⬇️
...t/ConverterAwareMappingSpannerEntityProcessor.java 73.58% <100%> (ø) 22 <0> (ø) ⬇️
...data/spanner/repository/query/SqlSpannerQuery.java 84.12% <100%> (+0.52%) 28 <3> (+2) ⬆️
...spanner/repository/query/PartTreeSpannerQuery.java 44% <66.66%> (+4.86%) 8 <0> (ø) ⬇️
...vert/ConverterAwareMappingSpannerEntityWriter.java 95.03% <83.33%> (ø) 50 <1> (ø) ⬇️
...epository/query/SpannerStatementQueryExecutor.java 84.95% <89.18%> (+0.46%) 56 <14> (+10) ⬆️
...tastore/GcpDatastoreEmulatorAutoConfiguration.java 65.51% <0%> (-6.9%) 6% <0%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update bd0af62...86d22fd. Read the comment docs.

dmitry-s
dmitry-s suggested changes Jun 19, 2019
View reviewed changes
...g/springframework/cloud/gcp/data/spanner/repository/query/SpannerStatementQueryExecutor.java Outdated
BiConsumer<ValueBinder<?>, Iterable> toMethod = getIterableValueBinderBiConsumer(innerType);

// attempt conversion of elements inside
if (toMethod == null) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should this logic be in ConverterAwareMappingSpannerEntityWriter?

Copy link
Contributor Author

@ChengyuanZhao ChengyuanZhao Jun 19, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

EDIT:

actually yea I think most of this already is in ConverterAwareMappingSpannerEntityWriter. let me rework it a bit.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dmitry-s actually great idea. I refactored . PTAL!

docs/src/main/asciidoc/spanner.adoc Show resolved Hide resolved
ChengyuanZhao
ChengyuanZhao commented Jun 19, 2019
View reviewed changes
...gframework/cloud/gcp/data/spanner/core/convert/ConverterAwareMappingSpannerEntityWriter.java
@@ -193,6 +193,81 @@ public SpannerWriteConverter getSpannerWriteConverter() {
return this.writeConverter;
}

/**
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@dmitry-s Don't be alarmed by these big new sections .They're just moved up because now they're public.

dmitry-s
dmitry-s approved these changes Jun 19, 2019
View reviewed changes
Copy link
Contributor

@dmitry-s dmitry-s left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

meltsufin
meltsufin approved these changes Jun 20, 2019
View reviewed changes
Copy link
Contributor

@meltsufin meltsufin left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice work!

@ChengyuanZhao ChengyuanZhao merged commit 09fa722 into master Jun 20, 2019
@ChengyuanZhao ChengyuanZhao deleted the spanner-array-param-binding branch June 20, 2019 15:30
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Reviewers

@dmitry-s dmitry-s dmitry-s approved these changes

@meltsufin meltsufin meltsufin approved these changes

Assignees

@ChengyuanZhao ChengyuanZhao

Labels
spanner
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Spring Data Spanner Repositories In clause queries support
3 participants
@ChengyuanZhao @dmitry-s @meltsufin