feat: Support for Proto Messages & Enums #2155

gauravpurohit06 · 2022-11-09T06:48:44Z

To add support for Proto Columns which includes messages and enums.

Commit will be reverted, once PROTO changes are available publicly.

Adding default implementation for newly added methods ByteArray compatability changes for Proto Messages

Adding Additional check for ChecksumResultSet

1. Adding docs and formatting the code. 2. Adding additional methods for enum and message which accepts descriptors.

generated-files-bot · 2022-11-09T06:48:51Z

Warning: This pull request is touching the following templated files:

proto-google-cloud-spanner-v1/src/main/java/com/google/spanner/v1/Type.java
proto-google-cloud-spanner-v1/src/main/java/com/google/spanner/v1/TypeCode.java
proto-google-cloud-spanner-v1/src/main/java/com/google/spanner/v1/TypeOrBuilder.java
proto-google-cloud-spanner-v1/src/main/java/com/google/spanner/v1/TypeProto.java
proto-google-cloud-spanner-v1/src/main/proto/google/spanner/v1/type.proto

ansh0l

Left a few comments, this is a long PR, will review changes to main and tests for spanner directory seprately

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

ansh0l · 2022-11-28T17:45:40Z

proto-google-cloud-spanner-v1/src/main/proto/google/spanner/v1/type.proto

+  // If [code][] == [PROTO][TypeCode.PROTO] or [code][] ==
+  // [ENUM][TypeCode.ENUM], then `proto_type_fqn` is the fully qualified name of
+  // the proto type representing the proto/enum definition.
+  string proto_type_fqn = 5;


ansh0l · 2022-11-28T17:45:41Z

proto-google-cloud-spanner-v1/src/main/proto/google/spanner/v1/type.proto

@@ -145,6 +150,13 @@ enum TypeCode {
  //   preserved.
  // - JSON array elements will have their order preserved.
  JSON = 11;
+


...loud-spanner/src/test/java/com/google/cloud/spanner/connection/ReadWriteTransactionTest.java

...loud-spanner/src/test/java/com/google/cloud/spanner/connection/RandomResultSetGenerator.java

...ud-spanner/src/test/java/com/google/cloud/spanner/connection/DirectExecuteResultSetTest.java

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

olavloite · 2022-12-01T16:01:00Z

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

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


As far as I can tell, the check is no longer required, but it is still active. Which means that if you don't add this, the build of the PR will be marked red (and some of us have brains that are not able to handle that, and need a green checkmark to be able to sleep at night ;-) )

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

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

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

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

Shua1

Please ask Sri Harsha to take a look at the test coverage and see if the coverage is comparable to the go client implementation. Thanks!

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

Shua1 · 2022-12-12T04:39:04Z

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

+    protected <T extends ProtocolMessageEnum> List<T> getProtoEnumListInternal(
+        int columnIndex, Function<Integer, ProtocolMessageEnum> method) {
+      Preconditions.checkNotNull(
+          method, "Method may not be null. Use 'MyProtoEnum::forNumber' as a parameter value.");


Is it possible to call it for the user in Java?

@Shua1: I prefer getProtoEnumListInternal(idx, my_method) here. Adding Function<Integer, ProtocolMessageEnum> method as a parameter makes more sense to me, because implementation for forNumber will be specific to the ProtocolMessageEnum class.

With getProtoEnumListInternal(idx), the Java client will not have any way to convert Long from Spanner backend into corresponding enum.

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

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

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

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

-Modified Docs/Comments -Minor Refactoring

olavloite

LGTM, with a couple of minor nits.

Just to be certain: This cannot be merged as is. It needs to be rebased once the generated changes have been merged.

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

...ud-spanner/src/test/java/com/google/cloud/spanner/connection/DirectExecuteResultSetTest.java

...er/src/test/java/com/google/cloud/spanner/connection/ReplaceableForwardingResultSetTest.java

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

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

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

google-cloud-spanner/src/test/resources/com/google/cloud/spanner/singer.proto

-code review comments -adding function docs

gauravpurohit06 · 2022-12-27T11:45:02Z

Just to be certain: This cannot be merged as is. It needs to be rebased once the generated changes have been merged.

Yes @olavloite , for now the changes will be merged in proto-column-enhancement-alpha branch and will only be merged in main once the generated changes have been merged.

* feat: Support for Proto Messages & Enums (#2155) * feat: Importing Proto Changes Commit will be reverted, once PROTO changes are available publicly. * feat: Proto Message Implementation * feat: Adding support for enum * feat: Code refactoring Adding default implementation for newly added methods ByteArray compatability changes for Proto Messages * docs: Adding Java docs for all the newly added methods. * test: Sample Proto & Generated classes for unit test * feat: Adding bytes/proto & int64/enum compatability Adding Additional check for ChecksumResultSet * test: Adding unit tests * test: Adding unit tests for ValueBinder.java * feat: refactoring to add support for getValue & other minor changes * feat: Minor refactoring 1. Adding docs and formatting the code. 2. Adding additional methods for enum and message which accepts descriptors. * feat: Adding bytes/message & int64/enum compatability in Value * refactor: Minor refactoring * feat: Adding Proto Array Implementation * test: Implementing unit tests for array of protos and enums * refactor: adding clirr ignores * feat: Adding support for enum as Primary Key * feat: Code Review Changes, minor refactoring and adding docs * feat: Addressing review comments -Modified Docs/Comments -Minor Refactoring * refactor: Using Column instead of column to avoid test failures * feat: Minor refactoring -code review comments -adding function docs * samples: Adding samples for updating & querying Proto messages & enums (#2211) * samples: Adding samples for updating & querying Proto messages & enums * style: linting * style: linting * docs: Adding function and class doc * test: Proto Column Integration tests (#2212) * test: Adding Integration tests for Proto Messages & Enums * test: Adding additional test for Parameterized Queries, Primary Keys & Invalid Wire type errors. * style: Formatting * style: Formatting * test: Updating instance and db name * test: Adding inter compatability check while writing data * Configured jitpack.yml to use OpenJDK 11 (#2218) Co-authored-by: Pavol Juhos <pjuhos@google.com> * feat: add support for Proto Columns DDL (#2277) * feat: add code changes and tests for Proto columns DDL support * feat: add auto generated code * feat: code changes and tests for Proto columns DDL support * feat: add descriptors file * feat: code refactoring * feat: Integration tests and code refactoring * feat: code refactoring * feat: unit tests and clirr differences * feat: lint changes * feat: code refactor * feat: code refactoring * feat: code refactoring * feat: code refactoring * feat: add java docs to new methods * feat: lint formatting * feat: lint formatting changes * feat: lint formatting * feat: lint formatting * feat: test exception cases * feat: code refactoring * feat: add java docs and refactoring * feat: add java docs * feat: java docs refactor * feat: remove overload method setProtoDescriptors that accepts file path as input to avoid unexpected issues * feat: remove updateDdl method overload to update proto descriptor * teat: update pom file to run tests on cloud-devel region temporarily to validate main branch update * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * fix: revert host changes in pom.xml file * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * feat(spanner): revert autogenerated code * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * feat(spanner): remove samples * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * feat(spanner): remove clirr * feat(spanner): skip emulator test * feat(spanner): clirr * feat(spanner): fix javadoc * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * feat(spanner): fix javadoc * feat(spanner): fix javadoc * feat(spanner): fix javadoc * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * feat(spanner): fix emulator skip * build: ignore all changes in v1 package * feat(spanner): add optimizations to deserialize proto messages * feat(spanner): remove TODO * feat(spanner): remove TODO --------- Co-authored-by: Gaurav Purohit <gauravpurohit@google.com> Co-authored-by: Pavol Juhos <pjuhos@google.com> Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com> Co-authored-by: Knut Olav Løite <koloite@gmail.com>

* feat: Support for Proto Messages & Enums (#2155) * feat: Importing Proto Changes Commit will be reverted, once PROTO changes are available publicly. * feat: Proto Message Implementation * feat: Adding support for enum * feat: Code refactoring Adding default implementation for newly added methods ByteArray compatability changes for Proto Messages * docs: Adding Java docs for all the newly added methods. * test: Sample Proto & Generated classes for unit test * feat: Adding bytes/proto & int64/enum compatability Adding Additional check for ChecksumResultSet * test: Adding unit tests * test: Adding unit tests for ValueBinder.java * feat: refactoring to add support for getValue & other minor changes * feat: Minor refactoring 1. Adding docs and formatting the code. 2. Adding additional methods for enum and message which accepts descriptors. * feat: Adding bytes/message & int64/enum compatability in Value * refactor: Minor refactoring * feat: Adding Proto Array Implementation * test: Implementing unit tests for array of protos and enums * refactor: adding clirr ignores * feat: Adding support for enum as Primary Key * feat: Code Review Changes, minor refactoring and adding docs * feat: Addressing review comments -Modified Docs/Comments -Minor Refactoring * refactor: Using Column instead of column to avoid test failures * feat: Minor refactoring -code review comments -adding function docs * samples: Adding samples for updating & querying Proto messages & enums (#2211) * samples: Adding samples for updating & querying Proto messages & enums * style: linting * style: linting * docs: Adding function and class doc * test: Proto Column Integration tests (#2212) * test: Adding Integration tests for Proto Messages & Enums * test: Adding additional test for Parameterized Queries, Primary Keys & Invalid Wire type errors. * style: Formatting * style: Formatting * test: Updating instance and db name * test: Adding inter compatability check while writing data * Configured jitpack.yml to use OpenJDK 11 (#2218) Co-authored-by: Pavol Juhos <pjuhos@google.com> * feat: add support for Proto Columns DDL (#2277) * feat: add code changes and tests for Proto columns DDL support * feat: add auto generated code * feat: code changes and tests for Proto columns DDL support * feat: add descriptors file * feat: code refactoring * feat: Integration tests and code refactoring * feat: code refactoring * feat: unit tests and clirr differences * feat: lint changes * feat: code refactor * feat: code refactoring * feat: code refactoring * feat: code refactoring * feat: add java docs to new methods * feat: lint formatting * feat: lint formatting changes * feat: lint formatting * feat: lint formatting * feat: test exception cases * feat: code refactoring * feat: add java docs and refactoring * feat: add java docs * feat: java docs refactor * feat: remove overload method setProtoDescriptors that accepts file path as input to avoid unexpected issues * feat: remove updateDdl method overload to update proto descriptor * teat: update pom file to run tests on cloud-devel region temporarily to validate main branch update * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * fix: revert host changes in pom.xml file * feat: add support for Proto Columns to Connection API * feat: add client side statement support for proto descriptor * feat: update existing unit tests to include proto descriptorss argument * feat: code refactor for client side statements * feat: add unit tests * feat: code refactoring for file path client statement * test: add integration test for DDL * fix: comment refactoring * feat: move proto descriptor file read logic to ConnectionImpl file to fix autogenerated client side statement tests * feat: add autogenerated test for new proto columns client side statements * feat: add unit tests to verify proto descriptor set via filepath * feat: add review comments * feat: add client side statement to show proto descriptors file path * fix: remove proto descriptors file extension validation * feat: comment refactor * feat: address review comments * feat: update tests and revert autogenerated code * chore: revert autogenerated coee * chore: revert autogenerated code * chore: revert autogenerated code * chore: lint format * 🦉 Updates from OwlBot post-processor See https://github.com/googleapis/repo-automation-bots/blob/main/packages/owl-bot/README.md * chore: regenerate descriptors file * chore: update schema * chore: update base64 value for protodescriptor * chore: update copyright year --------- Co-authored-by: Gaurav Purohit <gauravpurohit@google.com> Co-authored-by: Pavol Juhos <pjuhos@google.com> Co-authored-by: Owl Bot <gcf-owl-bot[bot]@users.noreply.github.com>

gauravpurohit06 added 12 commits October 20, 2022 08:24

feat: Importing Proto Changes

b7be994

Commit will be reverted, once PROTO changes are available publicly.

feat: Proto Message Implementation

a59bebf

feat: Adding support for enum

ffec112

feat: Code refactoring

7ade421

Adding default implementation for newly added methods ByteArray compatability changes for Proto Messages

docs: Adding Java docs for all the newly added methods.

570c952

test: Sample Proto & Generated classes for unit test

0e804f5

feat: Adding bytes/proto & int64/enum compatability

73df736

Adding Additional check for ChecksumResultSet

test: Adding unit tests

82a5468

test: Adding unit tests for ValueBinder.java

b125a1f

feat: refactoring to add support for getValue & other minor changes

5c48acc

feat: Minor refactoring

6c16377

1. Adding docs and formatting the code. 2. Adding additional methods for enum and message which accepts descriptors.

feat: Adding bytes/message & int64/enum compatability in Value

be26f87

gauravpurohit06 requested review from a team as code owners November 9, 2022 06:48

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 9, 2022

gauravpurohit06 changed the title ~~Support for Proto Messages & Enums~~ feat: Support for Proto Messages & Enums Nov 9, 2022

gauravpurohit06 added kokoro:run Add this label to force Kokoro to re-run the tests. kokoro:force-run Add this label to force Kokoro to re-run the tests. labels Nov 9, 2022

refactor: Minor refactoring

e4eb26c

gauravpurohit06 requested review from olavloite, ansh0l and rajatbhatta November 9, 2022 07:26

ansh0l marked this pull request as draft November 9, 2022 09:02

ansh0l added the do not merge Indicates a pull request not ready for merge, due to either quality or timing. label Nov 9, 2022

gauravpurohit06 added 3 commits November 28, 2022 06:32

feat: Adding Proto Array Implementation

f5af48e

test: Implementing unit tests for array of protos and enums

65424da

refactor: adding clirr ignores

64b7bda

ansh0l marked this pull request as ready for review November 28, 2022 17:34

ansh0l suggested changes Nov 28, 2022

View reviewed changes

olavloite reviewed Dec 1, 2022

View reviewed changes

gauravpurohit06 added 3 commits December 7, 2022 07:40

feat: Adding support for enum as Primary Key

426fc31

Merge branch 'proto-column-enhancement-alpha' into proto-basic-support

003ceb7

feat: Code Review Changes, minor refactoring and adding docs

3be24f0

Shua1 reviewed Dec 12, 2022

View reviewed changes

rajatbhatta reviewed Dec 12, 2022

View reviewed changes

google-cloud-spanner/src/main/java/com/google/cloud/spanner/AbstractResultSet.java Outdated Show resolved Hide resolved

feat: Addressing review comments

dfc38fb

-Modified Docs/Comments -Minor Refactoring

gauravpurohit06 requested review from olavloite, Shua1 and rajatbhatta and removed request for Shua1 December 13, 2022 06:55

refactor: Using Column instead of column to avoid test failures

4c6cc39

olavloite approved these changes Dec 20, 2022

View reviewed changes

harshachinta requested changes Dec 25, 2022

View reviewed changes

feat: Minor refactoring

b205121

-code review comments -adding function docs

ansh0l requested review from harshachinta and ansh0l and removed request for Shua1 December 28, 2022 10:41

ansh0l approved these changes Dec 28, 2022

View reviewed changes

gauravpurohit06 merged commit cbabd6c into googleapis:proto-column-enhancement-alpha Dec 28, 2022

gauravpurohit06 mentioned this pull request Mar 3, 2023

Proto col support [DRAFT] [DO NOT MERGE] #2006

Closed

harshachinta mentioned this pull request Jan 24, 2024

feat: add support for Proto Columns #2779

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Support for Proto Messages & Enums #2155

feat: Support for Proto Messages & Enums #2155

gauravpurohit06 commented Nov 9, 2022 •

edited

Loading

generated-files-bot bot commented Nov 9, 2022

ansh0l left a comment

ansh0l Nov 28, 2022

ansh0l Nov 28, 2022

olavloite Dec 1, 2022

Shua1 left a comment

Shua1 Dec 12, 2022

rajatbhatta Dec 12, 2022

olavloite left a comment

gauravpurohit06 commented Dec 27, 2022 •

edited

Loading

feat: Support for Proto Messages & Enums #2155

feat: Support for Proto Messages & Enums #2155

Conversation

gauravpurohit06 commented Nov 9, 2022 • edited Loading

generated-files-bot bot commented Nov 9, 2022

ansh0l left a comment

Choose a reason for hiding this comment

ansh0l Nov 28, 2022

Choose a reason for hiding this comment

ansh0l Nov 28, 2022

Choose a reason for hiding this comment

olavloite Dec 1, 2022

Choose a reason for hiding this comment

Shua1 left a comment

Choose a reason for hiding this comment

Shua1 Dec 12, 2022

Choose a reason for hiding this comment

rajatbhatta Dec 12, 2022

Choose a reason for hiding this comment

olavloite left a comment

Choose a reason for hiding this comment

gauravpurohit06 commented Dec 27, 2022 • edited Loading

gauravpurohit06 commented Nov 9, 2022 •

edited

Loading

gauravpurohit06 commented Dec 27, 2022 •

edited

Loading