Skip to content

Commit 982666b

Browse files
h3n4lclaude
andauthored
feat(googlesql/parser-ddl-spanner): CHANGE STREAM / SEQUENCE / ROLE / LOCALITY GROUP / PROTO BUNDLE + role GRANT (#246)
Implements the Spanner-specific object DDL the legacy ANTLR grammar has no first-class rule for (it rides them on the generic-entity hook or rejects them): - CREATE/ALTER/DROP CHANGE STREAM (truth1 DDL-024/025/026) - CREATE/ALTER/DROP SEQUENCE (DDL-027/028/029) - CREATE/DROP ROLE (DDL-032/033) - role-based GRANT/REVOKE: `… TO/FROM ROLE r [, ...]` and `GRANT/REVOKE ROLE r [, ...] TO/FROM ROLE r [, ...]`, plus the Spanner comma-separated object list `ON TABLE t1, t2 TO ROLE r` (DDL-034-037) - CREATE/ALTER/DROP LOCALITY GROUP (DDL-041/042/043) - CREATE/ALTER PROTO BUNDLE (DDL-046/047) New AST nodes (parsenodes.go + nodetags.go + regenerated walk_generated.go) plus a GranteeRole grantee kind and Roles/Paths fields on the shared GrantStmt/ RevokeStmt so one union parser serves both the legacy ZetaSQL string-grantee dialect and the Spanner role dialect. Dispatch wired in create_table.go / alter_table.go / drop.go (the CHANGE/LOCALITY bare-identifier words and the SEQUENCE/PROTO keywords) and in grant_revoke.go (role-grant disambiguation + comma-object head). analysis/classify.go classifies the new nodes as DDL. Authoritative oracle: the live Cloud Spanner emulator (docs/migration/googlesql/ oracle.md). PROVE gate = spanner_ddl_oracle_test.go differential, both polarities (81 fixtures, omni == emulator). Spanner is authoritative for these forms. Owned divergences closed: - #112 INTERLEAVE IN PARENT ON DELETE made OPTIONAL (defaults to NO ACTION) in create_table.go — the original port over-rejected the documented default. - #8 inline column PRIMARY KEY already accepted (matches emulator) — guarded. Flagged (oracle non-authoritative / docs-backed): #128 column-level GRANT, and the central "Spanner object DDL is first-class, not generic-entity" dialect divergence. Cross-model (Codex) review caught 3 real bugs (now fixed + regression-tested): role-grant misroute of legacy `GRANT ROLE ON …`, over-permissive non-ROLE role-grant target, and over-rejected comma-separated object lists. Oracle-driven follow-ups: role names are single identifiers (CREATE ROLE rejects dotted; DROP ROLE accepts), comma-object lists require a type keyword + ROLE grantees, and PROTO BUNDLE lists allow a trailing comma (change-stream column lists do not). go test ./googlesql/{parser,ast,analysis}/ green; differential green; vet clean. Co-authored-by: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
1 parent 9ab73eb commit 982666b

15 files changed

Lines changed: 2441 additions & 64 deletions

googlesql/analysis/classify.go

Lines changed: 9 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -148,7 +148,15 @@ func Classify(node ast.Node, dialect Dialect) QueryType {
148148
*ast.CreateMaterializedViewStmt, *ast.CreateSnapshotStmt,
149149
*ast.SearchVectorIndexStmt, *ast.CreateRowAccessPolicyStmt,
150150
*ast.CreateEntityStmt, *ast.BQAlterStmt, *ast.BQDropStmt,
151-
*ast.DropAllRowAccessPoliciesStmt:
151+
*ast.DropAllRowAccessPoliciesStmt,
152+
// Spanner-only object DDL (parser-ddl-spanner node): CHANGE STREAM,
153+
// SEQUENCE, ROLE, LOCALITY GROUP, PROTO BUNDLE. Same DDL classification as
154+
// every other CREATE/ALTER/DROP form.
155+
*ast.CreateChangeStreamStmt, *ast.AlterChangeStreamStmt, *ast.DropChangeStreamStmt,
156+
*ast.CreateSequenceStmt, *ast.AlterSequenceStmt, *ast.DropSequenceStmt,
157+
*ast.CreateRoleStmt, *ast.DropRoleStmt,
158+
*ast.CreateLocalityGroupStmt, *ast.AlterLocalityGroupStmt, *ast.DropLocalityGroupStmt,
159+
*ast.CreateProtoBundleStmt, *ast.AlterProtoBundleStmt:
152160
return DDL
153161

154162
// DCL — GRANT / REVOKE are DDL in the legacy listener (its rule list groups

googlesql/analysis/classify_test.go

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -61,6 +61,25 @@ func TestClassifySQL(t *testing.T) {
6161
{"drop all row access policies", "DROP ALL ROW ACCESS POLICIES ON ds.t", DDL},
6262
{"drop capacity (generic entity)", "DROP CAPACITY `p.r.c`", DDL},
6363

64+
// --- DDL — Spanner-only objects (parser-ddl-spanner node). These now PARSE
65+
// (previously over-rejected), so classification must report DDL, not Unknown. ---
66+
{"create change stream", "CREATE CHANGE STREAM s FOR ALL", DDL},
67+
{"alter change stream", "ALTER CHANGE STREAM s SET FOR ALL", DDL},
68+
{"drop change stream", "DROP CHANGE STREAM s", DDL},
69+
{"create sequence", "CREATE SEQUENCE q", DDL},
70+
{"alter sequence", "ALTER SEQUENCE q SET OPTIONS (start_with_counter=1)", DDL},
71+
{"drop sequence", "DROP SEQUENCE q", DDL},
72+
{"create role", "CREATE ROLE analyst", DDL},
73+
{"drop role", "DROP ROLE analyst", DDL},
74+
{"create locality group", "CREATE LOCALITY GROUP g OPTIONS (storage='ssd')", DDL},
75+
{"alter locality group", "ALTER LOCALITY GROUP g SET OPTIONS (storage='hdd')", DDL},
76+
{"drop locality group", "DROP LOCALITY GROUP g", DDL},
77+
{"create proto bundle", "CREATE PROTO BUNDLE (`a.b.C`)", DDL},
78+
{"alter proto bundle", "ALTER PROTO BUNDLE INSERT (`a.b.C`)", DDL},
79+
{"grant to role", "GRANT SELECT ON TABLE t TO ROLE r", DDL},
80+
{"grant role to role", "GRANT ROLE a TO ROLE b", DDL},
81+
{"revoke role from role", "REVOKE ROLE a FROM ROLE b", DDL},
82+
6483
// --- Unknown / empty ---
6584
{"empty", "", Unknown},
6685
{"whitespace only", " \n ", Unknown},

googlesql/ast/nodetags.go

Lines changed: 49 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -142,6 +142,27 @@ const (
142142
T_BQDropStmt
143143
T_DropAllRowAccessPoliciesStmt
144144

145+
// Spanner-specific DDL (parser-ddl-spanner node): CHANGE STREAM, SEQUENCE,
146+
// ROLE, LOCALITY GROUP, and PROTO BUNDLE CREATE/ALTER/DROP forms. These have
147+
// no first-class rule in the legacy ANTLR grammar (it rides them on the
148+
// generic-entity hook or rejects them); the omni parser models them directly,
149+
// authoritatively verified against the live Cloud Spanner emulator. (Role-based
150+
// GRANT/REVOKE reuses T_GrantStmt/T_RevokeStmt with the GranteeRole kind.)
151+
T_ChangeStreamTrackedTable
152+
T_CreateChangeStreamStmt
153+
T_AlterChangeStreamStmt
154+
T_DropChangeStreamStmt
155+
T_CreateSequenceStmt
156+
T_AlterSequenceStmt
157+
T_DropSequenceStmt
158+
T_CreateRoleStmt
159+
T_DropRoleStmt
160+
T_CreateLocalityGroupStmt
161+
T_AlterLocalityGroupStmt
162+
T_DropLocalityGroupStmt
163+
T_CreateProtoBundleStmt
164+
T_AlterProtoBundleStmt
165+
145166
// DML — INSERT / UPDATE / DELETE / MERGE / TRUNCATE (parser-dml node).
146167
//
147168
// The data-manipulation family for the BigQuery + Spanner union, plus their
@@ -361,6 +382,34 @@ func (t NodeTag) String() string {
361382
return "BQDropStmt"
362383
case T_DropAllRowAccessPoliciesStmt:
363384
return "DropAllRowAccessPoliciesStmt"
385+
case T_ChangeStreamTrackedTable:
386+
return "ChangeStreamTrackedTable"
387+
case T_CreateChangeStreamStmt:
388+
return "CreateChangeStreamStmt"
389+
case T_AlterChangeStreamStmt:
390+
return "AlterChangeStreamStmt"
391+
case T_DropChangeStreamStmt:
392+
return "DropChangeStreamStmt"
393+
case T_CreateSequenceStmt:
394+
return "CreateSequenceStmt"
395+
case T_AlterSequenceStmt:
396+
return "AlterSequenceStmt"
397+
case T_DropSequenceStmt:
398+
return "DropSequenceStmt"
399+
case T_CreateRoleStmt:
400+
return "CreateRoleStmt"
401+
case T_DropRoleStmt:
402+
return "DropRoleStmt"
403+
case T_CreateLocalityGroupStmt:
404+
return "CreateLocalityGroupStmt"
405+
case T_AlterLocalityGroupStmt:
406+
return "AlterLocalityGroupStmt"
407+
case T_DropLocalityGroupStmt:
408+
return "DropLocalityGroupStmt"
409+
case T_CreateProtoBundleStmt:
410+
return "CreateProtoBundleStmt"
411+
case T_AlterProtoBundleStmt:
412+
return "AlterProtoBundleStmt"
364413
case T_DefaultExpr:
365414
return "DefaultExpr"
366415
case T_InsertStmt:

0 commit comments

Comments
 (0)