feat(spanner): add support for Proto Columns in Connection API #3123
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
* 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
googleapis#2211) * samples: Adding samples for updating & querying Proto messages & enums * style: linting * style: linting * docs: Adding function and class doc
* 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
Co-authored-by: Pavol Juhos <[email protected]>
* 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
…to validate main branch update
… fix autogenerated client side statement tests
|
Warning: This pull request is touching the following templated files:
|
product-auto-label
bot
added
the
api: spanner
harshachinta
added
the
owlbot:run
gcf-owl-bot
bot
removed
the
owlbot:run
harshachinta
changed the title
harshachinta
added
the
kokoro:force-run
yoshi-kokoro
removed
the
kokoro:force-run
olavloite
approved these changes
May 24, 2024
Comment on lines
+1944
to
+1946
| // reset proto descriptors after executing a DDL statement | ||
| this.protoDescriptors = null; | ||
| this.protoDescriptorsFilePath = null; |
There was a problem hiding this comment.
Would it make more sense to move this to setDefaultTransactionOptions()? It keeps the code cleaner, as we have all the reset code in one method, but it does mean that executing for example a query after setting the proto descriptors also clears the fields. One option to prevent the latter could be to add an input argument to setDefaultTransactionOptions() that indicates the type of statement that was executed, and only reset these fields if the last statement was a DDL statement.
There was a problem hiding this comment.
sure, will check if it makes sense to move that logic to setDefaultTransactionOptions() as a seperate PR. This PR is very big that the code review will be difficult. Will work on this immediately.
There was a problem hiding this comment.
This comment will be addressed in PR #3126
| private final String instanceId; | ||
| private final String databaseName; | ||
|
|
||
| static class Builder { | ||
| private DatabaseAdminClient dbAdminClient; | ||
| private String projectId; |
There was a problem hiding this comment.
nit: Instead of adding this field, could we change the individual fields projectId, instanceId, databaseName into one field of type DatabaseId?
There was a problem hiding this comment.
sure, will do this along with the previous comment as a seperate PR.
There was a problem hiding this comment.
This comment will be addressed in PR #3126
...spanner/src/test/java/com/google/cloud/spanner/connection/ProtoDescriptorsConverterTest.java
Outdated
Show resolved
Hide resolved
...ner/src/test/java/com/google/cloud/spanner/connection/ProtoDescriptorsFileConverterTest.java
Outdated
Show resolved
Hide resolved
harshachinta
added
the
owlbot:run
gcf-owl-bot
bot
removed
the
owlbot:run
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Adds support for Proto Columns feature in the Connection API.
This PR is taken from #2495.