feat: add PG OID support #2736

tlhquynh · 2023-11-20T12:51:57Z

Added support for PG OID type which is an int64 value with a PG_OID type annotation code.

Remove the example partition token from the SQL test scripts, as the tokens are not stable when the encoded classes are changed. E.g. modifying the Value classes will cause these tests to fail with a de-serialization error, as the serialized version in the token no longer corresponds with the version in code. Fixes the build error in #2736

* test: remove partition token from tests Remove the example partition token from the SQL test scripts, as the tokens are not stable when the encoded classes are changed. E.g. modifying the Value classes will cause these tests to fail with a de-serialization error, as the serialized version in the token no longer corresponds with the version in code. Fixes the build error in #2736 * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md --------- Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>

tlhquynh · 2023-12-29T05:51:19Z

Both aforementioned TODOs regarding class incompatible due to mismatched serialVersionUID and rebasing after #2703 is merged are done.

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

olavloite

Looks generally good to me. The biggest question that I have is whether there is any value in adding all the getPgOid(..) methods, or whether it would be better to just say 'use getLong(..) for both normal INT64 and PG.OID values'.

olavloite · 2024-03-22T08:36:51Z

.java-version

@@ -0,0 +1 @@
+17.0.9


Remove this file (and potentially add the file name to .gitignore to prevent it from being included in commits in the future)

olavloite · 2024-03-22T08:37:16Z

.vscode/settings.json

@@ -0,0 +1,4 @@
+{


Same as above: Remove file from GitHub and add the filename to .gitignore

olavloite · 2024-03-22T08:40:16Z

google-cloud-spanner/src/main/java/com/google/cloud/spanner/AbstractStructReader.java

@@ -61,6 +61,8 @@ protected String getPgJsonbInternal(int columnIndex) {
 throw new UnsupportedOperationException("Not implemented");
 }

+ protected abstract long getPgOidInternal(int columnIndex);


Add a default implementation for this and the other methods that are being added to this class. The default implementation should just throw UnsupportedOperationException. This prevents this change from being a breaking change, as anyone who has a class that extends AbstractStructReader does not get a compile error.

That should also remove the requirement to add the method names to clirr-ignored-differences.xml

olavloite · 2024-03-22T08:40:55Z

google-cloud-spanner/clirr-ignored-differences.xml

@@ -192,6 +192,63 @@
 <className>com/google/cloud/spanner/StructReader</className>
 <method>java.util.List getPgJsonbList(java.lang.String)</method>
 </difference>
+ <!-- PG OID -->
+ <difference>


It should not be necessary to add the internal methods to this file, if those methods are given a default implementation that just throws UnsupportedOperationException

These entries in clirr-ignored-differences.xml for StructReader can be removed, as we don't have those methods anymore.

final nit (I promise :-)): I think that the two remaining additions to this file for getPgOid() and getPgOidArray() can also be removed, as these methods are no longer added to the Value class.

olavloite · 2024-03-22T08:49:05Z

google-cloud-spanner/src/main/java/com/google/cloud/spanner/AbstractStructReader.java

+ @Override
+ public long getPgOid(int columnIndex) {
+ checkNonNullOfType(columnIndex, Type.pgOid(), columnIndex);
+ return getPgOidInternal(columnIndex);
+ }
+
+ @Override
+ public long getPgOid(String columnName) {
+ int columnIndex = getColumnIndex(columnName);
+ checkNonNullOfType(columnIndex, Type.pgOid(), columnName);
+ return getPgOidInternal(columnIndex);
+ }


Would it be worth considering not adding any of this at all, and instead instruct users that they have to get PG.OID values in the same way as INT64? So just call getLong(..) and the other corresponding methods. This is otherwise all just a copy-paste of those methods, and there's no additional validation of the values that we are getting.
I know that it is not what we have done for other types (e.g. we could in theory have done the same for JSON by telling users to just call getString(...)). But this might be the right time to start to prevent us from doing double work for future types, while it does not add any additional type safety or features for users.

@skuruppu Any thoughts on this?

Done.
I had an offline chat with @skuruppu and treating OID as Long is actually what we are doing for the other clients. I have removed all external PG.OID getters.

olavloite · 2024-03-22T08:56:50Z

...-cloud-spanner/src/test/java/com/google/cloud/spanner/connection/AllTypesMockServerTest.java

@@ -572,22 +613,40 @@ public void testInsertAllTypes() {
 ExecuteSqlRequest request = mockSpanner.getRequestsOfType(ExecuteSqlRequest.class).get(0);
 Map<String, Type> paramTypes = request.getParamTypesMap();
 Map<String, Value> params = request.getParams().getFieldsMap();
- assertEquals(20, paramTypes.size());
- assertEquals(20, params.size());
+ System.out.println("Dialect = " + dialect);


nit: remove

* chore: keep session pool ordering when pinging Pinging sessions would move the sessions that were pinged to either the front or the back of the pool (dependingin the session pool configuration), instead of keeping the sessions in the place where they were when being pinged. Bringing a session that is pinged to the front of the pool means that we will prefer using a session that has not really been used for a while, other than for the ping. Keeping the sessions in place is therefore preferable. * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md --------- Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>

…3.38.0 (googleapis#2942)

To enable Direct Access, [both `setAttemptDirectPath` and `setAttemptDirectPathXds` should be called](https://togithub.com/googleapis/sdk-platform-java/blob/4b44a7851dc1d4fd2ac21a54df6c24db5625223c/gax-java/gax-grpc/src/main/java/com/google/api/gax/grpc/InstantiatingGrpcChannelProvider.java#L373-L386) for gax to append the correct google-c2p scheme. This PR adds a env var `GOOGLE_SPANNER_ENABLE_DIRECT_ACCESS` to control the enable/disable of Direct Access. When it is true, it calls `setAttemptDirectPathXds` which effectively turns on Direct Access (as `options.isAttemptDirectPath` is by default true and we don't need to call `setAttemptDirectPath` again).

…r-plugin to v3.13.0 (googleapis#2956)

…y-plugin to v3.7.1 (googleapis#2955)

…v3.28.1 (googleapis#2952)

olavloite · 2024-03-28T07:46:29Z

google-cloud-spanner/clirr-ignored-differences.xml

@@ -192,6 +192,63 @@
 <className>com/google/cloud/spanner/StructReader</className>
 <method>java.util.List getPgJsonbList(java.lang.String)</method>
 </difference>
+ <!-- PG OID -->
+ <difference>


These entries in clirr-ignored-differences.xml for StructReader can be removed, as we don't have those methods anymore.

olavloite · 2024-03-28T07:50:50Z

google-cloud-spanner/src/main/java/com/google/cloud/spanner/AbstractStructReader.java

@@ -61,6 +61,10 @@ protected String getPgJsonbInternal(int columnIndex) {
 throw new UnsupportedOperationException("Not implemented");
 }

+ protected long getPgOidInternal(int columnIndex) {


I think that you can remove this method (and the equivalent array methods below) as well, and instead just use the getLongInternal method.

olavloite · 2024-03-28T07:51:15Z

google-cloud-spanner/src/main/java/com/google/cloud/spanner/AbstractStructReader.java

+ checkNonNullOfCodes(
+ columnIndex, Arrays.asList(Code.ENUM, Code.PG_OID, Code.INT64), columnIndex);
+ return getColumnType(columnIndex).getCode() == Code.PG_OID
+ ? getPgOidInternal(columnIndex)


I think that this can be simplified to just calling getLongInternal. The same also applies to the array methods below.

olavloite · 2024-03-28T07:52:17Z

google-cloud-spanner/src/main/java/com/google/cloud/spanner/GrpcStruct.java

@@ -460,6 +468,12 @@ protected String getPgJsonbInternal(int columnIndex) {
 return (String) rowData.get(columnIndex);
 }

+ @Override
+ protected long getPgOidInternal(int columnIndex) {


remove and rely on getLongInternal instead

olavloite · 2024-03-28T07:52:54Z

google-cloud-spanner/src/main/java/com/google/cloud/spanner/GrpcStruct.java

@@ -771,6 +789,18 @@ protected List<String> getPgJsonbListInternal(int columnIndex) {
 return Collections.unmodifiableList((List<String>) rowData.get(columnIndex));
 }

+ @Override
+ protected Int64Array getPgOidListInternal(int columnIndex) {


Remove and use getLongListInternal instead

google-cloud-spanner/src/main/java/com/google/cloud/spanner/Value.java

…ionFuture interfaces. (googleapis#2973) * chore: add multiplexed session implementations for CachedSession/SessionFuture interfaces. * chore: add comments. * chore: add session replacement handler for multiplexed session. * chore: address comments. * chore: fix comments. * chore: fix comments.

…3.39.0 (googleapis#2966)

🤖 I have created a release *beep* *boop* --- ## [6.62.1](https://togithub.com/googleapis/java-spanner/compare/v6.62.0...v6.62.1) (2024-03-28) ### Dependencies * Update dependency com.google.cloud:google-cloud-monitoring to v3.39.0 ([googleapis#2966](https://togithub.com/googleapis/java-spanner/issues/2966)) ([a5cb1dd](https://togithub.com/googleapis/java-spanner/commit/a5cb1ddd065100497d9215eff30d57361d7e84de)) * Update dependency com.google.cloud:google-cloud-trace to v2.38.0 ([googleapis#2967](https://togithub.com/googleapis/java-spanner/issues/2967)) ([b2dc788](https://togithub.com/googleapis/java-spanner/commit/b2dc788d5a54244d83a192ecac894ff931f884c4)) --- This PR was generated with [Release Please](https://togithub.com/googleapis/release-please). See [documentation](https://togithub.com/googleapis/release-please#release-please).

🤖 I have created a release *beep* *boop* --- ### Updating meta-information for bleeding-edge SNAPSHOT release. --- This PR was generated with [Release Please](https://togithub.com/googleapis/release-please). See [documentation](https://togithub.com/googleapis/release-please#release-please).

…googleapis#2959) * Add support for transaction-level exclusion from change streams * cleanup * refactor: introduce PartitionedUpdateOption * Revert "refactor: introduce PartitionedUpdateOption" This reverts commit 96b508b. * Add error handling in DML update APIs where excludeTxnFromChangeStreams option is not applicable * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md --------- Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>

googleapis#2988)

…2986)

…3.40.0 (googleapis#2987) * deps: update dependency com.google.cloud:google-cloud-monitoring to v3.40.0 * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md --------- Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>

…35.0 (googleapis#2989) * chore(deps): update dependency com.google.cloud:libraries-bom to v26.35.0 * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md --------- Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>

Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>

🤖 I have created a release *beep* *boop* --- ### Updating meta-information for bleeding-edge SNAPSHOT release. --- This PR was generated with [Release Please](https://togithub.com/googleapis/release-please). See [documentation](https://togithub.com/googleapis/release-please#release-please).

* chore: clean up some warnings and malformed comments * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md --------- Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>

…to v6.63.0 (googleapis#2992) * chore(deps): update dependency com.google.cloud:google-cloud-spanner to v6.63.0 * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md --------- Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>

Adds an 'endpoint' connection URL property for the Connection API. This property can be used instead of adding the endpoint to the host group part of the Connection URL, which again removes the need to actually change the connection URL when connecting to for example the emulator from the JDBC driver. The latter can instead just add the endpoint to the Properties set that is given to the JDBC driver.

* feat: support max_commit_delay in Connection API Adds support for max_commit_delay to the Connection API: 1. Adds a setMaxCommitDelay(Duration) method to Connection 2. Adds a maxCommitDelay connection URL property 3. Adds a SET MAX_COMMIT_DELAY=<duration> SQL statement * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md --------- Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>

* chore: minor improvements to default benchmarks. * chore: lint issues fix. * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md --------- Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>

….12 (googleapis#2996)

googleapis#2971) Source-Link: googleapis/synthtool@ca7a716 Post-Processor: gcr.io/cloud-devrel-public-resources/owlbot-java:latest@sha256:cecae6152a85d55c932a64515643cf2e32a1f1b6e17503080eb07744b2177f28 Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>

googleapis#2932) * feat: Add instance partition support to spanner instance proto PiperOrigin-RevId: 611127452 Source-Link: googleapis/googleapis@618d47c Source-Link: googleapis/googleapis-gen@92d8555 Copy-Tag: eyJwIjoiLmdpdGh1Yi8uT3dsQm90LnlhbWwiLCJoIjoiOTJkODU1NTg4ODI4NDMwZThiNDI4ZWQ3ODIxOWUyM2VlNjY2ZGE3OCJ9 * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * fix(deps): Update the Java code generator (gapic-generator-java) to 2.37.0 PiperOrigin-RevId: 611816371 Source-Link: googleapis/googleapis@2a40f63 Source-Link: googleapis/googleapis-gen@d30ff07 Copy-Tag: eyJwIjoiLmdpdGh1Yi8uT3dsQm90LnlhbWwiLCJoIjoiZDMwZmYwNzY3Nzc3YjM4MWZiMTYxN2Y2N2E5MGUzYWJkM2JkYzZkYyJ9 * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * feat: Add SessionPoolOptions, SpannerOptions protos in executor protos PiperOrigin-RevId: 621265883 Source-Link: googleapis/googleapis@fed9845 Source-Link: googleapis/googleapis-gen@c66a769 Copy-Tag: eyJwIjoiLmdpdGh1Yi8uT3dsQm90LnlhbWwiLCJoIjoiYzY2YTc2OTU3ZTJlMTYzNDdiYzFkZDNmNGM2MzgyMjNmMDY1ZWU4MCJ9 * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md --------- Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>

olavloite · 2024-04-04T14:00:24Z

google-cloud-spanner/clirr-ignored-differences.xml

@@ -192,6 +192,63 @@
 <className>com/google/cloud/spanner/StructReader</className>
 <method>java.util.List getPgJsonbList(java.lang.String)</method>
 </difference>
+ <!-- PG OID -->
+ <difference>


final nit (I promise :-)): I think that the two remaining additions to this file for getPgOid() and getPgOidArray() can also be removed, as these methods are no longer added to the Value class.

tlhquynh requested review from a team as code owners November 20, 2023 12:51

product-auto-label bot added size: xl Pull request size is extra large. api: spanner Issues related to the googleapis/java-spanner API. labels Nov 20, 2023

olavloite mentioned this pull request Nov 21, 2023

test: remove partition token from tests #2738

Merged

tlhquynh added 2 commits December 29, 2023 15:34

feat: add PG OID support

6b3f572

chore: fix lint errors

929abf4

tlhquynh force-pushed the pg-oid branch from 45f4485 to 929abf4 Compare December 29, 2023 04:41

Resolved merge conflict from branch 'main' into pg-oid.

4e7ea9d

Merge branch 'main' into pg-oid

0489d9a

product-auto-label bot added size: l Pull request size is large. and removed size: xl Pull request size is extra large. labels Mar 14, 2024

Update PG.OID implementation according to recent changes.

e5a39c5

product-auto-label bot added size: xl Pull request size is extra large. and removed size: l Pull request size is large. labels Mar 19, 2024

Update PG.OID implementation according to recent changes.

537de96

arpan14 added the owlbot:run Add this label to trigger the Owlbot post processor. label Mar 22, 2024

gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Mar 22, 2024

🦉 Updates from OwlBot post-processor

f9ea807

See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md

olavloite self-requested a review March 22, 2024 07:09

olavloite reviewed Mar 22, 2024

View reviewed changes

olavloite and others added 6 commits March 26, 2024 10:18

deps: update dependency com.google.cloud:google-cloud-monitoring to v…

e505b9e

…3.38.0 (googleapis#2942)

build(deps): update dependency org.apache.maven.plugins:maven-compile…

1d756a3

…r-plugin to v3.13.0 (googleapis#2956)

build(deps): update dependency org.apache.maven.plugins:maven-assembl…

f6193f9

…y-plugin to v3.7.1 (googleapis#2955)

deps: update dependency com.google.cloud:sdk-platform-java-config to …

aed1223

…v3.28.1 (googleapis#2952)

olavloite reviewed Mar 28, 2024

View reviewed changes

arpan14 and others added 21 commits April 3, 2024 17:46

Remove internal PG.OID getters.

60712bd

deps: update dependency com.google.cloud:google-cloud-monitoring to v…

19fbdfb

…3.39.0 (googleapis#2966)

deps: update dependency com.google.cloud:google-cloud-trace to v2.39.0 (

80ade2e

googleapis#2988)

deps: update dependency commons-io:commons-io to v2.16.0 (googleapis#…

ecb87e7

…2986)

chore(main): release 6.63.0 (googleapis#2985)

f103f18

Co-authored-by: release-please[bot] <55107282+release-please[bot]@users.noreply.github.com>

build(deps): update dependency org.jacoco:jacoco-maven-plugin to v0.8…

417a83c

….12 (googleapis#2996)

Merge branch 'main' into pg-oid

b8074b5

olavloite approved these changes Apr 4, 2024

View reviewed changes

googleapis deleted a comment from generated-files-bot bot Apr 4, 2024

tlhquynh added 2 commits April 5, 2024 10:27

Merge branch 'main' into pg-oid

cdd1c91

chore: Remove unused CLIRR entries

b63740d

arpan14 added the owlbot:run Add this label to trigger the Owlbot post processor. label Apr 5, 2024

gcf-owl-bot bot removed the owlbot:run Add this label to trigger the Owlbot post processor. label Apr 5, 2024

thiagotnunes merged commit ba2a4af into googleapis:main Apr 5, 2024
25 of 26 checks passed

release-please bot mentioned this pull request Apr 5, 2024

chore(main): release 6.64.0 #2995

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: add PG OID support #2736

feat: add PG OID support #2736

tlhquynh commented Nov 20, 2023 •

edited

tlhquynh commented Dec 29, 2023

olavloite left a comment

olavloite Mar 22, 2024

tlhquynh Mar 25, 2024

olavloite Mar 22, 2024

tlhquynh Mar 25, 2024

olavloite Mar 22, 2024

tlhquynh Mar 25, 2024

olavloite Mar 22, 2024

tlhquynh Mar 25, 2024

olavloite Mar 28, 2024

tlhquynh Apr 3, 2024

olavloite Apr 4, 2024

olavloite Mar 22, 2024

tlhquynh Mar 28, 2024

olavloite Mar 22, 2024

tlhquynh Mar 28, 2024

olavloite Mar 28, 2024

olavloite Mar 28, 2024

tlhquynh Apr 3, 2024

olavloite Mar 28, 2024

tlhquynh Apr 3, 2024

olavloite Mar 28, 2024

tlhquynh Apr 3, 2024

olavloite Mar 28, 2024

tlhquynh Apr 3, 2024

olavloite Apr 4, 2024

		@@ -0,0 +1 @@
		17.0.9

feat: add PG OID support #2736

feat: add PG OID support #2736

Conversation

tlhquynh commented Nov 20, 2023 • edited

tlhquynh commented Dec 29, 2023

olavloite left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

tlhquynh commented Nov 20, 2023 •

edited