Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Revisit case-sensitive column implementation #271

Closed
elefeint opened this issue Sep 23, 2020 · 0 comments · Fixed by #291
Closed

Revisit case-sensitive column implementation #271

elefeint opened this issue Sep 23, 2020 · 0 comments · Fixed by #291
Assignees
@dmitry-s
Labels
feature New functionality P2 V2
Milestone
0.3.0

Comments

@elefeint
Copy link
Contributor

Cloud Spanner column names are case-sensitive. We've historically been overriding the columnMetadata() and duplicateColumnNames() TCK test methods to reflect that case sensitivity is expected.

However, this implementation technically violates the R2DBC Row and RowMetadata specs that say:

Column names are case insensitive. When a get method contains several columns with same name, then the value of the first
matching column will be returned
* @return the {@link ColumnMetadata} for one column in this row

Consider whether for V2, we should revert to R2DBC spec definition of case-insensitive column names.
@elefeint elefeint added the V2 label Sep 23, 2020
@elefeint elefeint added feature New functionality P2 labels Oct 6, 2020
@elefeint elefeint mentioned this issue Oct 22, 2020
Merged
elefeint added a commit that referenced this issue Oct 23, 2020
@elefeint
Test new tck framework (#270)
a76226b 
Remove TCK test overrides that were necessary due to DML syntax requiring column list in Spanner.

R2DBC SPI 0.8.3 containing the TCK changes was released. I also upgraded Reactor to the same version that's in SPI.

There are still two reasons for overrides:
1) Spanner returning `Long` even when `Integer` is requested -- work tracked in #276.
2) the driver breaking R2DBC spec by treating columns as case insensitive -- decision pending on #271.
@elefeint elefeint added this to the 0.3.0 milestone Nov 12, 2020
@dmitry-s dmitry-s mentioned this issue Nov 17, 2020
Merged
dmitry-s added a commit that referenced this issue Nov 17, 2020
@dmitry-s
support case-insensitive column names (#291)
bd899e5 
fixes #271
@elefeint elefeint mentioned this issue Mar 26, 2021
Merged
elefeint added a commit that referenced this issue Mar 29, 2021
@elefeint
Upgrade to modern Spring Data R2DBC (#347)
6efbf51 
Spring Data dialect updates:
* Many supporting classes moved from Spring Data into Spring Framework.
* Spring Boot integration is no longer in experimental; it's part of mainstream Boot autoconfiguration.
* The need for `SpannerBindMarkerFactoryProvider` is a bit redundant since the same information can be derived from dialect, which is already getting autodiscovered. But it's documented [here](https://docs.spring.io/spring-framework/docs/current/reference/html/data-access.html#r2dbc-DatabaseClient).

Updated sample:
* Explicitly specifying `@Column` is needed because v1 is case-sensitive. We have fixed it to comply with case-insensitive spec for v2 in #271 , so this should become unnecessary when we migrate to v2.

Necessary but not sufficient step towards #314.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees

@dmitry-s dmitry-s

Labels
feature New functionality P2 V2
Projects
None yet
Milestone
0.3.0
Development

Successfully merging a pull request may close this issue.

support case-insensitive column names GoogleCloudPlatform/cloud-spanner-r2dbc
2 participants
@dmitry-s @elefeint