fix(spanner/spansql): PROTO BUNDLE and protobuf type parsing fixes#11279
Merged
harshachinta merged 6 commits intogoogleapis:mainfrom Jan 8, 2025
Merged
Conversation
The protobuf type-name parser was so greedy that it failed on NOT NULL columns. In particular, it wasn't aware that unquoted tokens should be separated by dots, and that quoted tokens shouldn't be concatenated with anything else. Fix this by adding a boolean to handle that alternation and only allowing quoted IDs if nothing else has been consumed. (then bailing immediately after a quoted ID so we don't try to consume anything else)
A misreading of the spanner docs lead to tests that indicated that casting `AS ENUM` or `AS PROTO` was valid syntax (despite not specifying _which_ protobuf enum or message type to cast to). Replace these cases with ones that validate casting to specific enum/message types. Thanks to @apstndb for calling this out on googleapis#10945.
Fix a bug in CreateProtoBundle.SQL() which unintentionally generated the DDL when there were no types listed: ``` CREATE PROTO BUNDLE (``) ```
This was referenced Dec 12, 2024
harshachinta
approved these changes
Jan 7, 2025
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
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.
fix: spansql: fix NOT NULL protobuf column type parsing
The protobuf type-name parser was so greedy that it failed on NOT NULL
columns. In particular, it wasn't aware that unquoted tokens should be
separated by dots, and that quoted tokens shouldn't be concatenated with
anything else.
Fix this by adding a boolean to handle that alternation and only
allowing quoted IDs if nothing else has been consumed. (then bailing
immediately after a quoted ID so we don't try to consume anything
else)
fix: spansql: fix invalid CAST tests
A misreading of the spanner docs lead to tests that indicated that
casting
AS ENUMorAS PROTOwas valid syntax (despite not specifyingwhich protobuf enum or message type to cast to). Replace these cases
with ones that validate casting to specific enum/message types.
Thanks to @apstndb for calling this out on feat(spanner/spansql): add support for protobuf column types & Proto bundles #10945.
fix spansql: CREATE PROTO BUNDLE SQL with 0 types
Fix a bug in CreateProtoBundle.SQL() which unintentionally generated the
DDL when there were no types listed:
Fixes: #11301