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

feat: support DML with Returning clause in Connection API #1978

Merged
merged 35 commits into from
Nov 9, 2022
Merged

feat: support DML with Returning clause in Connection API #1978

merged 35 commits into from
Nov 9, 2022

Conversation

rajatbhatta
Copy link
Contributor

@rajatbhatta rajatbhatta commented Aug 17, 2022 •
edited
Loading

This PR adds support for running DML statements with Returning clause in the Connection API, and incorporates the following:

  • API documentation changes to document the behaviour of execute, executeAsync, executeQuery, executeQueryAsync, executeUpdate, executeUpdateAsync, executeBatchUpdate, and executeBatchUpdateAsync APIs when executing DML statements with Returning clause.
  • Parses the SQL string to check if the DML statement has a Returning clause. Parsing logic has been added for both GoogleStandardSql and PostgreSql dialects.
  • Changes to MockSpannerServiceImpl to put results for DML statements with Returning clause, and to add support in executeBatchDml.
  • Integration tests for running DML statements with Returning clause using each of the above APIs, for both GoogleStandardSql and PostgreSql dialects.
  • Unit tests for abort scenarios while running DML statements with Returning clause.
  • Parser tests for both GoogleStandardSql and PostgreSql dialects.

@product-auto-label product-auto-label bot added size: l Pull request size is large. api: spanner Issues related to the googleapis/java-spanner API. labels Aug 17, 2022
@product-auto-label product-auto-label bot added size: xl Pull request size is extra large. and removed size: l Pull request size is large. labels Aug 22, 2022
@rajatbhatta rajatbhatta marked this pull request as ready for review August 22, 2022 10:28
@rajatbhatta rajatbhatta requested a review from a team as a code owner August 22, 2022 10:28
@rajatbhatta rajatbhatta requested a review from olavloite August 22, 2022 10:28
@rajatbhatta rajatbhatta mentioned this pull request Aug 22, 2022
Merged
@rajatbhatta rajatbhatta requested a review from ansh0l August 22, 2022 17:42
@rajatbhatta
Copy link
Contributor Author

Note: Integration tests are not expected to pass until DML with Returning clause support is launched.

@rajatbhatta rajatbhatta changed the title feat: incorporate dml with returning clause feat: support dml with returning clause Aug 22, 2022
@rajatbhatta rajatbhatta mentioned this pull request Aug 22, 2022
Closed
@rajatbhatta rajatbhatta changed the title feat: support dml with returning clause feat: support DML with Returning clause in Connection API Aug 22, 2022
olavloite
olavloite reviewed Aug 23, 2022
View reviewed changes
google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/Connection.java Show resolved Hide resolved
google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/Connection.java Show resolved Hide resolved
google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ConnectionImpl.java Outdated Show resolved Hide resolved
google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ConnectionImpl.java Outdated Show resolved Hide resolved
google-cloud-spanner/src/main/java/com/google/cloud/spanner/connection/ConnectionImpl.java Outdated Show resolved Hide resolved
google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/StatementParserTest.java Show resolved Hide resolved
google-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/StatementParserTest.java Show resolved Hide resolved
...e-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/it/ITDmlReturningTest.java Outdated Show resolved Hide resolved
...e-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/it/ITDmlReturningTest.java Outdated Show resolved Hide resolved
...e-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/it/ITDmlReturningTest.java Show resolved Hide resolved
@rajatbhatta @olavloite
Update google-cloud-spanner/src/main/java/com/google/cloud/spanner/co…
4024412 
…nnection/ConnectionImpl.java

Co-authored-by: Knut Olav Løite <[email protected]>
olavloite
olavloite approved these changes Aug 24, 2022
View reviewed changes
Copy link
Collaborator

@olavloite olavloite left a comment

Choose a reason for hiding this comment

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

@rajatbhatta Thanks for your patience with me for this PR. LGTM.

@rajatbhatta
Copy link
Contributor Author

@olavloite: Thank you for a very detailed review of this PR. :)

@rajatbhatta
Copy link
Contributor Author

@olavloite: Can you take a quick look at the changes in e2a463e? Thanks!

@olavloite
Copy link
Collaborator

@olavloite: Can you take a quick look at the changes in e2a463e? Thanks!

Can we also add a test for insert into t1 select 10.returning* (this is equivalent with insert into t1 select 10.0 returning *)

@rajatbhatta
Copy link
Contributor Author

@olavloite: Can you take a quick look at the changes in e2a463e? Thanks!

Can we also add a test for insert into t1 select 10.returning* (this is equivalent with insert into t1 select 10.0 returning *)

Added in 60fcb4c.

@olavloite
Copy link
Collaborator

60fcb4c

Thanks. LGTM (once CI errors have been cleared)

@rajatbhatta rajatbhatta requested a review from a team as a code owner November 9, 2022 05:56
@rajatbhatta rajatbhatta added automerge Merge the pull request once unit tests and other checks pass. and removed do not merge Indicates a pull request not ready for merge, due to either quality or timing. labels Nov 9, 2022
@rajatbhatta rajatbhatta added the owlbot:run Add this label to trigger the Owlbot post processor. label Nov 9, 2022
@gcf-owl-bot gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Nov 9, 2022
@rajatbhatta rajatbhatta merged commit aac20be into googleapis:main Nov 9, 2022
@rajatbhatta rajatbhatta deleted the dml-returning-parsing-changes branch November 9, 2022 12:19
@gcf-merge-on-green gcf-merge-on-green bot removed the automerge Merge the pull request once unit tests and other checks pass. label Nov 9, 2022
@release-please release-please bot mentioned this pull request Nov 9, 2022
Merged
gcf-merge-on-green bot pushed a commit that referenced this pull request Nov 17, 2022
@release-please
chore(main): release 6.33.0 (#2137)
820934c 
🤖 I have created a release *beep* *boop*
---


## [6.33.0](https://togithub.com/googleapis/java-spanner/compare/v6.32.0...v6.33.0) (2022-11-17)


### Features

* Adding samples for Jsonb data type ([#2147](https://togithub.com/googleapis/java-spanner/issues/2147)) ([1112203](https://togithub.com/googleapis/java-spanner/commit/1112203bd6bde68fcd04ae68a2a31ec88dd5b1ac))
* Analyze update returns param types ([#2156](https://togithub.com/googleapis/java-spanner/issues/2156)) ([7c5e3da](https://togithub.com/googleapis/java-spanner/commit/7c5e3da4c128cb9220213db8b3e2291e33566715))
* Support DML with Returning clause in Connection API ([#1978](https://togithub.com/googleapis/java-spanner/issues/1978)) ([aac20be](https://togithub.com/googleapis/java-spanner/commit/aac20bedf9ee7a6a2170f87fa88373b7d364ed9f))
* Support PostgreSQL END statement ([#2131](https://togithub.com/googleapis/java-spanner/issues/2131)) ([4c29c17](https://togithub.com/googleapis/java-spanner/commit/4c29c17fb35e51fdad99e393a8f6bb57c914dc8a))
* Update transaction.proto to include different lock modes ([#2112](https://togithub.com/googleapis/java-spanner/issues/2112)) ([d0195b4](https://togithub.com/googleapis/java-spanner/commit/d0195b45423b73969636bc911980613a46dffa97))


### Dependencies

* Update dependency com.google.cloud:google-cloud-monitoring to v3.7.0 ([#2164](https://togithub.com/googleapis/java-spanner/issues/2164)) ([82385b8](https://togithub.com/googleapis/java-spanner/commit/82385b8526e0299e8c85e4435e3c740474de854c))
* Update dependency com.google.cloud:google-cloud-shared-dependencies to v3.0.6 ([#2150](https://togithub.com/googleapis/java-spanner/issues/2150)) ([dba545f](https://togithub.com/googleapis/java-spanner/commit/dba545ff5ebb069a78b42cbffff032d66dc3d062))
* Update dependency com.google.cloud:google-cloud-trace to v2.6.0 ([#2165](https://togithub.com/googleapis/java-spanner/issues/2165)) ([99f2779](https://togithub.com/googleapis/java-spanner/commit/99f277974fdcebf587d1e25ad643575e15cee7ff))
* Update dependency org.graalvm.buildtools:junit-platform-native to v0.9.17 ([#2144](https://togithub.com/googleapis/java-spanner/issues/2144)) ([dd24b89](https://togithub.com/googleapis/java-spanner/commit/dd24b894fd80ccc962a414bb404d9624336f4612))
* Update dependency org.openjdk.jmh:jmh-core to v1.36 ([#2160](https://togithub.com/googleapis/java-spanner/issues/2160)) ([29f9096](https://togithub.com/googleapis/java-spanner/commit/29f9096d1a10bfb9eacdbc4d6dbc4bc9c7ed05c1))
* Update dependency org.openjdk.jmh:jmh-generator-annprocess to v1.36 ([#2161](https://togithub.com/googleapis/java-spanner/issues/2161)) ([9148aa3](https://togithub.com/googleapis/java-spanner/commit/9148aa37bfb61af25023d56bfcf6d0e735e51b9a))

---
This PR was generated with [Release Please](https://togithub.com/googleapis/release-please). See [documentation](https://togithub.com/googleapis/release-please#release-please).
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@bvandiver bvandiver bvandiver left review comments

@c2nes c2nes c2nes left review comments

@olavloite olavloite olavloite approved these changes

@thiagotnunes thiagotnunes thiagotnunes approved these changes

@ansh0l ansh0l Awaiting requested review from ansh0l

Assignees

@rajatbhatta rajatbhatta

Labels
api: spanner Issues related to the googleapis/java-spanner API. size: xl Pull request size is extra large.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@rajatbhatta @olavloite @thiagotnunes @c2nes @bvandiver