Skip to content

fix(spanner/spansql): support ARRAY of PROTO types in DDL - #20463

Merged
rahul2393 merged 2 commits into
googleapis:mainfrom
machi-genggeng:spansql-proto-array
Sep 9, 2026
Merged

rahul2393 merged 2 commits into
googleapis:mainfrom
machi-genggeng:spansql-proto-array

Conversation

@machi-genggeng

@machi-genggeng machi-genggeng commented Sep 3, 2026 •

Copy link
Copy Markdown
Contributor

Fixes: #20462

In parseBaseOrParameterizedType, parsing protobuf type identifiers returned early with return t, nil, preventing the parser from consuming the closing > token when t.Array was true.

This change:

  1. Removes the early returns in parseBaseOrParameterizedType so the expect(">") check executes for arrays.
  2. Updates Type.SQL() to allow proto types to receive the ARRAY<...> wrapping when t.Array is true.
  3. Adds parser and SQL serialization test cases for array proto types.

@machi-genggeng
machi-genggeng requested review from a team as code owners September 3, 2026 00:01
@product-auto-label product-auto-label Bot added the api: spanner Issues related to the Spanner API. label Sep 3, 2026

@gemini-code-assist gemini-code-assist Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Code Review

This pull request fixes operator precedence and return logic in spanner/spansql to correctly handle and format arrays of protobuf and enum types, along with adding corresponding parser and SQL generation tests. The reviewer noted that the explicit break statements introduced in parser.go are redundant in Go's switch blocks and should be removed to keep the code idiomatic.

Comment thread spanner/spansql/parser.go Resolved
Comment thread spanner/spansql/parser.go Outdated
Comment on lines +3568 to +3569
t.Base = Proto
return t, nil
break

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

medium

In Go, switch cases do not fall through by default, so an explicit break statement at the end of a case is redundant. It can be safely omitted to keep the code clean and idiomatic.

Suggested change
t.Base = Proto
return t, nil
break
t.Base = Proto

@machi-genggeng
machi-genggeng force-pushed the spansql-proto-array branch 2 times, most recently from 8a77a02 to 9d29c51 Compare September 3, 2026 01:30
In `parseBaseOrParameterizedType`, parsing protobuf type identifiers
returned early with `return t, nil`, preventing the parser from
consuming the closing `>` token when `t.Array` was true.

This change:
1. Remove the early returns in `parseBaseOrParameterizedType` so the `expect(">")` check executes for arrays.
2. Updates `Type.SQL()` to allow proto types to receive the `ARRAY<...>`
   wrapping when `t.Array` is true.
3. Adds parser and SQL serialization test cases for array proto types.
@rahul2393 rahul2393 added the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Sep 9, 2026
@kokoro-team kokoro-team removed the kokoro:force-run Add this label to force Kokoro to re-run the tests. label Sep 9, 2026
@rahul2393
rahul2393 enabled auto-merge (squash) September 9, 2026 06:54
@rahul2393
rahul2393 merged commit 5fe6a28 into googleapis:main Sep 9, 2026
17 of 18 checks passed
dbolduc pushed a commit that referenced this pull request Sep 10, 2026
🤖 I have created a release *beep* *boop*
---


##
[1.95.1](spanner/v1.95.0...spanner/v1.95.1)
(2026-09-10)


### Bug Fixes

* **spanner/spansql:** Support ARRAY of PROTO types in DDL
([#20463](#20463))
([5fe6a28](5fe6a28))

---
This PR was generated with [Release
Please](https://github.com/googleapis/release-please). See
[documentation](https://github.com/googleapis/release-please#release-please).

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

spansql: parser error when parsing Array of PROTO

4 participants