Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix(spanner/spansql): PROTO BUNDLE and protobuf type parsing fixes #11279

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

dfinkel
Copy link
Contributor

@dfinkel dfinkel commented Dec 12, 2024

  • 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 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 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:

    CREATE PROTO BUNDLE (``)
    

Fixes: #11301

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 (``)
```
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api: spanner Issues related to the Spanner API.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

spanner/spansql: NOT NULL protobuf columns concatenate NOT NULL into protobuf type-name
1 participant