feat: add support for QueryOptions by olavloite · Pull Request #76 · googleapis/java-spanner-jdbc · GitHub

Adds the ability to set QueryOptions when running Cloud Spanner queries through JDBC. For now, only setting the query_optimizer_version is added.

QueryOptions can be configured through the following mechanisms:

In the connection URL of a JDBC connection using the property optimizerVersion. The version specified here will be applied to all queries executed by the connection.
By executing the statement SET OPTIMIZER_VERSION='<version>' on a JDBC connection. All queries that are executed after this statement will use the version specified.
Through the SPANNER_OPTIMIZER_VERSION environment variable.

If the options are configured through multiple mechanisms then:

Options set at the JDBC connection URL level will override options configured at the environment variable level.
Options set by executing a SET OPTIMIZER_VERSION='<version>' will override options set at a JDBC connection URL level or environment variable level.

If no options are set, the optimizer version will default to:

The optimizer version the database is pinned to.
If the database is not pinned to a specific version, then the Cloud Spanner backend will use the "latest" version.

Depends on changes in the client library that have not yet been merged, meaning that the build of this PR will fail until the required changes have been merged.

LGTM with some minor comments.

One request, if you could update the description of this PR to contain the information about the env variable and the precedence, that would be great. Example PR

LGTM with some minor comments.

One request, if you could update the description of this PR to contain the information about the env variable and the precedence, that would be great. Example PR

Done.

I merged in the v1.51.0 release which should hopefully be available on Maven by the end of your day.

Codecov Report

Merging #76 into master will increase coverage by 0.05%.
The diff coverage is 84.61%.

@@             Coverage Diff              @@
##             master      #76      +/-   ##
============================================
+ Coverage     77.79%   77.84%   +0.05%     
- Complexity     1723     1732       +9     
============================================
  Files            60       60              
  Lines          5836     5881      +45     
  Branches        769      779      +10     
============================================
+ Hits           4540     4578      +38     
- Misses          923      925       +2     
- Partials        373      378       +5

Impacted Files	Coverage Δ	Complexity Δ
...java/com/google/cloud/spanner/jdbc/JdbcDriver.java	26.86% <ø> (ø)	7 <0> (ø)	⬇️
...anner/jdbc/ClientSideStatementValueConverters.java	93.47% <100%> (+0.21%)	0 <0> (ø)	⬇️
...m/google/cloud/spanner/jdbc/ConnectionOptions.java	85.16% <100%> (+0.59%)	57 <3> (+3)	⬆️
...com/google/cloud/spanner/jdbc/StatementResult.java	100% <100%> (ø)	0 <0> (ø)	⬇️
.../spanner/jdbc/ConnectionStatementExecutorImpl.java	100% <100%> (ø)	29 <2> (+2)	⬆️
...com/google/cloud/spanner/jdbc/StatementParser.java	86.59% <76%> (-1.39%)	48 <1> (+2)
.../com/google/cloud/spanner/jdbc/ConnectionImpl.java	82.41% <80%> (-0.14%)	157 <2> (+2)

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 4fff168...bb300e0. Read the comment docs.

Looks like I can't merge in #75 until #73 can be merged in but that can only be done once googleapis/java-core#179 is in a release. Yikes, so many dependencies!

I don't understand this failure in the Linkage Monitor when all other tests pass:

Mar 17, 2020 2:24:16 AM com.google.cloud.tools.dependencies.linkagemonitor.LinkageMonitor run
INFO: BOM Coordinates: com.google.cloud:libraries-bom:4.2.0
Mar 17, 2020 2:27:17 AM com.google.cloud.tools.dependencies.linkagemonitor.LinkageMonitor run
SEVERE: Newly introduced problems:
Class com.google.spanner.v1.ExecuteSqlRequest$QueryOptions is not found
  referenced from com.google.cloud.spanner.jdbc.ConnectionImpl (google-cloud-spanner-jdbc-1.13.1-SNAPSHOT.jar)
  referenced from com.google.cloud.spanner.jdbc.ConnectionOptions (google-cloud-spanner-jdbc-1.13.1-SNAPSHOT.jar)
  referenced from com.google.cloud.spanner.jdbc.StatementParser (google-cloud-spanner-jdbc-1.13.1-SNAPSHOT.jar)
Class com.google.spanner.v1.ExecuteSqlRequest$QueryOptions$Builder is not found
  referenced from com.google.cloud.spanner.jdbc.ConnectionImpl (google-cloud-spanner-jdbc-1.13.1-SNAPSHOT.jar)
  referenced from com.google.cloud.spanner.jdbc.ConnectionOptions (google-cloud-spanner-jdbc-1.13.1-SNAPSHOT.jar)
  referenced from com.google.cloud.spanner.jdbc.StatementParser (google-cloud-spanner-jdbc-1.13.1-SNAPSHOT.jar)
(google-cloud-spanner-1.49.2.jar) com.google.cloud.spanner.Statement's method getQueryOptions() is not found
  referenced from com.google.cloud.spanner.jdbc.StatementParser (google-cloud-spanner-jdbc-1.13.1-SNAPSHOT.jar)
(google-cloud-spanner-1.49.2.jar) com.google.cloud.spanner.Statement's method toBuilder() is not found
  referenced from com.google.cloud.spanner.jdbc.StatementParser (google-cloud-spanner-jdbc-1.13.1-SNAPSHOT.jar)
(google-cloud-spanner-1.49.2.jar) com.google.cloud.spanner.Statement$Builder's method withQueryOptions(com.google.spanner.v1.ExecuteSqlRequest$QueryOptions arg1) is not found
  referenced from com.google.cloud.spanner.jdbc.StatementParser (google-cloud-spanner-jdbc-1.13.1-SNAPSHOT.jar)

google-cloud-spanner-jdbc-1.13.1-SNAPSHOT.jar is at:
  com.google.cloud:google-cloud-spanner-jdbc:1.13.1-SNAPSHOT (compile)
google-cloud-spanner-1.49.2.jar is at:
  com.google.cloud:google-cloud-spanner:1.49.2 (compile)

Mar 17, 2020 2:27:17 AM com.google.cloud.tools.dependencies.linkagemonitor.LinkageMonitor main
SEVERE: Found 5 new linkage errors

Your PR has attempted to merge for 3 hours. Please check that all required checks have passed, you have an automerge label, and that all your reviewers have approved the PR

@skuruppu I keep receiving notification emails due to the label automerge. I just removed it at the moment.

Your PR was not mergeable because either one of your required status checks failed, or one of your required reviews was not approved. See required reviews for your repo here: https://github.com/googleapis/sloth/blob/master/required-checks.json

I don't understand this Linkage Monitor error either. I just tried to execute it locally and it executed without any problems. I also don't understand why the log seems to indicate that Cloud Spanner 1.49.2 is being used, while it clearly uses 1.51.0. The output locally was:

loite@loite-XPS-15-9575:~/wsgoogle/java-spanner-jdbc$ mv ~/Downloads/linkage-monitor-latest-all-deps.jar .
loite@loite-XPS-15-9575:~/wsgoogle/java-spanner-jdbc$ java -jar linkage-monitor-latest-all-deps.jar com.google.cloud:libraries-bom
SLF4J: Failed to load class "org.slf4j.impl.StaticLoggerBinder".
SLF4J: Defaulting to no-operation (NOP) logger implementation
SLF4J: See http://www.slf4j.org/codes.html#StaticLoggerBinder for further details.
Mar 17, 2020 2:09:30 PM com.google.cloud.tools.dependencies.linkagemonitor.LinkageMonitor run
INFO: BOM Coordinates: com.google.cloud:libraries-bom:4.2.0
Mar 17, 2020 2:12:29 PM com.google.cloud.tools.dependencies.linkagemonitor.LinkageMonitor run
INFO: Snapshot versions have the same 816 errors as baseline
Mar 17, 2020 2:12:29 PM com.google.cloud.tools.dependencies.linkagemonitor.LinkageMonitor main
INFO: No new problem found

@skuruppu The IT failure has been fixed, but the error in the Linkage Monitor step is preventing this from being merged.

@chingor13 would you be able to help us with the two unexplained test failures.

We don't know why the Linkage Monitor is failing. It seems like it's relying on an older version of the client library despite having updated the pom.xml file.
The samples tests will not pass since this change needs to be merged in before it can pass.

Any help is appreciated.

(Linkage Monitor developer here). The failure looks good to me; no need to fix it.

GCP Libraries BOM 4.2.0 has com.google.cloud:google-cloud-spanner:1.49.2
https://storage.googleapis.com/cloud-opensource-java-dashboard/com.google.cloud/libraries-bom/4.2.0/artifact_details.html. The error means if we (GCP Libraries BOM maintainer) upgrade only google-cloud-spanner-jdbc's version in the BOM, there would be a conflict with google-cloud-spanner:1.49.2. Therefore we (GCP Libraries BOM maintainer) need to upgrade google-cloud-spanner-jdbc and google-cloud-spanner together next time we update the content of the BOM.

We don't know why the Linkage Monitor is failing. It seems like it's relying on an older version of the client library despite having updated the pom.xml file.

GCP Library BOM does not look at google-cloud-spanner-jdbc's pom.xml.

The error means if we (GCP Libraries BOM maintainer) upgrade only google-cloud-spanner-jdbc's version in the BOM, there would be a conflict with google-cloud-spanner:1.49.2. Therefore we (GCP Libraries BOM maintainer) need to upgrade google-cloud-spanner-jdbc and google-cloud-spanner together next time we update the content of the BOM.

Thanks for the explanation @suztomo. When will the BOM contents be updated? This PR needs to be merged in before the end of the week as it's a blocker for a feature launch. We can't merge it currently because the PR requires the Linkage Monitor step to pass.

I don't think of a better way than skipping the check. (repository owners can unmark "required")

libraries-bom:4.2.0 has google-cloud-spanner-bom:1.49.2 through com.google.cloud:google-cloud-bom:0.122.3-alpha
The latest com.google.cloud:google-cloud-bom:0.122.4-alpha has google-cloud-spanner-bom:1.50.0 (not the latest 1.51.0).
Therefore, if we wait for libraries-bom to have google-cloud-spanner:1.51.0, the BOM needs to wait for a new google-cloud-bom that contains google-cloud-spanner:1.51.0.

@elharo Do you think of a better way than either skipping the check or waiting for google-cloud-bom release?

The reason this check is here is that having versions out of sync like this causes hard-to-debug problems for customers. Looking at the output of the linkage monitor, this is exactly the sort of thing they're going to hit.

I think @suztomo's analysis is correct. google-cloud-bom should push a new version that includes google-cloud-spanner:1.51.0. You don't need to wait for this to happen. Just ask @chingor13 to release it. Once that's out--it takes about 4 hours from start to finish, mostly waiting on Sonatype to publish--we can push a new version of libraries-bom that pulls that in.

Thanks @elharo and @suztomo for the explanations. @chingor13 would you be able to help us with the google-cloud-bom release?

Yep, I can do one

Looks like java-cloud-bom/releases/tag/v0.123.2 has java-cloud-bom/pull/406. Are there any other steps required @elharo?

we still need a libraries-bom release that incorporates this. One is in progress.

@chingor13 Can you rerun the Linkage Monitor check? GCP Libraries BOM 4.3.0 has been released today.

@chingor13 I see it succeeded. Thank you.

Thanks so much @chingor13, @suztomo and @elharo for your help with this. We really appreciate it.

🤖 I have created a release \*beep\* \*boop\* --- ## [1.15.0](https://www.github.com/googleapis/java-spanner-jdbc/compare/v1.14.0...v1.15.0) (2020-03-24) ### Features * add support for QueryOptions ([#76](https://www.github.com/googleapis/java-spanner-jdbc/issues/76)) ([b3f4cf7](https://www.github.com/googleapis/java-spanner-jdbc/commit/b3f4cf7852a2fd5f22660cc3f25a6253b9a118ab)) ### Dependencies * update spanner.version to v1.52.0 ([#95](https://www.github.com/googleapis/java-spanner-jdbc/issues/95)) ([cdf9d30](https://www.github.com/googleapis/java-spanner-jdbc/commit/cdf9d30e8ca387d87a6ffe00fa09818d135547f4)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please).

b3f4cf7 commit b3f4cf7 Author: Knut Olav Løite <koloite@gmail.com> Date: Mon Mar 23 20:04:19 2020 +0100 feat: add support for QueryOptions (#76) * feat: add support for QueryOptions * fix: review comments * deps: temporarily update because of build and test errors * deps: update to released versions * fix: use grpc 1.27.2 * fix: fix invalid query hint in IT Co-authored-by: skuruppu <skuruppu@google.com> pom.xml src/main/java/com/google/cloud/spanner/jdbc/ClientSideStatementValueConverters.java src/main/java/com/google/cloud/spanner/jdbc/Connection.java src/main/java/com/google/cloud/spanner/jdbc/ConnectionImpl.java src/main/java/com/google/cloud/spanner/jdbc/ConnectionOptions.java src/main/java/com/google/cloud/spanner/jdbc/ConnectionStatementExecutor.java src/main/java/com/google/cloud/spanner/jdbc/ConnectionStatementExecutorImpl.java src/main/java/com/google/cloud/spanner/jdbc/JdbcDriver.java src/main/java/com/google/cloud/spanner/jdbc/StatementParser.java src/main/java/com/google/cloud/spanner/jdbc/StatementResult.java src/main/resources/com/google/cloud/spanner/jdbc/ClientSideStatements.json src/test/java/com/google/cloud/spanner/jdbc/AbortedTest.java src/test/java/com/google/cloud/spanner/jdbc/AbstractConnectionImplTest.java src/test/java/com/google/cloud/spanner/jdbc/AbstractMockServerTest.java src/test/java/com/google/cloud/spanner/jdbc/ConnectionImplTest.java src/test/java/com/google/cloud/spanner/jdbc/ConnectionStatementExecutorTest.java src/test/java/com/google/cloud/spanner/jdbc/ConnectionStatementWithNoParametersTest.java src/test/java/com/google/cloud/spanner/jdbc/ConnectionStatementWithOneParameterTest.java src/test/java/com/google/cloud/spanner/jdbc/ConnectionTest.java src/test/java/com/google/cloud/spanner/jdbc/JdbcQueryOptionsTest.java src/test/java/com/google/cloud/spanner/jdbc/JdbcTypeConverterTest.java src/test/java/com/google/cloud/spanner/jdbc/it/ITJdbcQueryOptionsTest.java src/test/java/com/google/cloud/spanner/jdbc/it/ITJdbcSqlScriptTest.java src/test/resources/com/google/cloud/spanner/jdbc/ClientSideStatementsTest.sql src/test/resources/com/google/cloud/spanner/jdbc/ConnectionImplGeneratedSqlScriptTest.sql src/test/resources/com/google/cloud/spanner/jdbc/ITSqlScriptTest_TestQueryOptions.sql

10ee396 commit 10ee396 Author: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com> Date: Tue Mar 24 03:26:38 2020 +0000 chore: release 1.15.0 (#96) 🤖 I have created a release \*beep\* \*boop\* --- ## [1.15.0](https://www.github.com/googleapis/java-spanner-jdbc/compare/v1.14.0...v1.15.0) (2020-03-24) ### Features * add support for QueryOptions ([#76](https://www.github.com/googleapis/java-spanner-jdbc/issues/76)) ([b3f4cf7](https://www.github.com/googleapis/java-spanner-jdbc/commit/b3f4cf7852a2fd5f22660cc3f25a6253b9a118ab)) ### Dependencies * update spanner.version to v1.52.0 ([#95](https://www.github.com/googleapis/java-spanner-jdbc/issues/95)) ([cdf9d30](https://www.github.com/googleapis/java-spanner-jdbc/commit/cdf9d30e8ca387d87a6ffe00fa09818d135547f4)) --- This PR was generated with [Release Please](https://github.com/googleapis/release-please). CHANGELOG.md README.md pom.xml versions.txt

googlebot added the cla: yes This human has signed the Contributor License Agreement. label Mar 5, 2020

olavloite requested review from hengfengli and skuruppu March 5, 2020 19:03

skuruppu approved these changes Mar 9, 2020

View reviewed changes

olavloite mentioned this pull request Mar 10, 2020

feat: add QueryOptions samples for JDBC GoogleCloudPlatform/java-docs-samples#2347

Merged

olavloite added 3 commits March 12, 2020 16:43

feat: add support for QueryOptions

10adde0

fix: review comments

25b44a1

deps: temporarily update because of build and test errors

4e2edfb

olavloite force-pushed the query-options branch from 5cf4f49 to 4e2edfb Compare March 12, 2020 16:58

deps: update to released versions

8cf99be

olavloite added api: spanner Issues related to the googleapis/java-spanner-jdbc API. kokoro:force-run Add this label to force Kokoro to re-run the tests. labels Mar 13, 2020

yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Mar 13, 2020

Merge branch 'master' into query-options

0968f19

skuruppu added automerge Merge the pull request once unit tests and other checks pass. kokoro:force-run Add this label to force Kokoro to re-run the tests. labels Mar 17, 2020

yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Mar 17, 2020

hengfengli removed the automerge Merge the pull request once unit tests and other checks pass. label Mar 17, 2020

fix: use grpc 1.27.2

f2e1572

skuruppu added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Mar 17, 2020

yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Mar 17, 2020

skuruppu added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Mar 17, 2020

yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Mar 17, 2020

fix: fix invalid query hint in IT

bb300e0

skuruppu added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Mar 20, 2020

yoshi-kokoro removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Mar 20, 2020

olavloite merged commit b3f4cf7 into master Mar 23, 2020

olavloite deleted the query-options branch March 23, 2020 19:04

feat: add support for QueryOptions by olavloite · Pull Request #76 · googleapis/java-spanner-jdbc · GitHub

Uh oh!

feat: add support for QueryOptions - #76

feat: add support for QueryOptions#76olavloite merged 7 commits intomastergoogleapis/java-spanner-jdbc:masterfrom query-optionsgoogleapis/java-spanner-jdbc:query-optionsCopy head branch name to clipboard

Conversation

olavloite commented Mar 5, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

skuruppu left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

olavloite commented Mar 10, 2020

Uh oh!

skuruppu commented Mar 13, 2020

Uh oh!

codecov Bot commented Mar 13, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Codecov Report

Uh oh!

skuruppu commented Mar 14, 2020

Uh oh!

skuruppu commented Mar 17, 2020

Uh oh!

gcf-merge-on-green Bot commented Mar 17, 2020

Uh oh!

gcf-merge-on-green Bot commented Mar 17, 2020

Uh oh!

gcf-merge-on-green Bot commented Mar 17, 2020

Uh oh!

gcf-merge-on-green Bot commented Mar 17, 2020

Uh oh!

hengfengli commented Mar 17, 2020

Uh oh!

gcf-merge-on-green Bot commented Mar 17, 2020

Uh oh!

olavloite commented Mar 17, 2020

Uh oh!

olavloite commented Mar 18, 2020

Uh oh!

skuruppu commented Mar 18, 2020

Uh oh!

suztomo commented Mar 19, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

skuruppu commented Mar 19, 2020

Uh oh!

suztomo commented Mar 19, 2020 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

elharo commented Mar 19, 2020

Uh oh!

skuruppu commented Mar 19, 2020

Uh oh!

chingor13 commented Mar 19, 2020

Uh oh!

skuruppu commented Mar 20, 2020

Uh oh!

elharo commented Mar 20, 2020

Uh oh!

suztomo commented Mar 23, 2020

Uh oh!

suztomo commented Mar 23, 2020

Uh oh!

skuruppu commented Mar 23, 2020

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

8 participants

feat: add support for QueryOptions#76
olavloite merged 7 commits into
masterfrom
query-options

olavloite commented Mar 5, 2020 •
edited
Loading

codecov Bot commented Mar 13, 2020 •
edited
Loading

suztomo commented Mar 19, 2020 •
edited
Loading

suztomo commented Mar 19, 2020 •
edited
Loading