Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
28 changes: 28 additions & 0 deletions DEVIATIONS.md
Original file line number Diff line number Diff line change
Expand Up @@ -593,6 +593,34 @@ is registered in `testdata/upstream_extensions.jsonl` (`mysql-table-value`) with
`parser/parser.go`; regression tests `TestMySQLTableStatementIsSelectStar`, `TestMySQLTableStatementScope`,
`TestMySQLCommandLeadersNotDivertedInNestedStatement`.

### 1.16 MySQL `DROP INDEX <idx> ON <table>` accepts a db-qualified target

**What upstream does:** pinned sqlglot v30.12.0 structures the unqualified `DROP INDEX idx ON users` as
`Drop{kind:INDEX, cluster:OnProperty(this=Identifier(users))}`, but its shared `_parse_on_property`
(parser.py:3345) parses the ON target with a single `_parse_schema(_parse_id_var())`, so the
**db-qualified** `DROP INDEX idx ON db.users` parse-**errors** at the dot. Verified on the pinned
reference. (The port itself previously degraded even the unqualified form to a raw `Command`, because it
had no `exp.OnProperty` node — that gap is closed here too.)

**What sqlglot-go does:** `parseDrop` parses the `ON` target with `parseTableParts`, so the whole
statement is `Drop{kind:INDEX, this:Table(idx), cluster:OnProperty(this:Table(...))}` and the db
qualifier survives: `DROP INDEX idx ON db.users` → `cluster.OnProperty.this = Table(this:users,
schema:db)`, round-tripping unchanged. The target is a `Table` (not upstream's bare `Identifier`) so it
can carry the qualifier — an output-identical AST-shape difference for the unqualified form. This is
**scoped to DROP**: the shared `parseOnProperty` is left unchanged (returns nil for the ClickHouse
`ON CLUSTER` half, out of this port's dialect scope), so CREATE/ALTER are untouched. A missing target
(`DROP INDEX i ON`) fails closed via `parseTableParts`' own raise.

**Why we diverge (correctness):** MySQL's `DROP INDEX index_name ON tbl_name` permits a db-qualified
`tbl_name` — verified on MySQL 8.0.46 (`DROP INDEX idx_email ON zzq.users` executes). Upstream's
single-id parse is a bug vs the real engine, and for the downstream consumer it turned a legitimate
table-DDL statement into a parse-error/`Command` (over-denied). The unqualified form is a port
completion (aligns with upstream's structured `Drop`); the **qualified** form — which pinned upstream
parse-errors — is registered in `testdata/upstream_extensions.jsonl` (`mysql-drop-index-on-qualified`)
with a tripwire, per the "grammar beyond upstream" discipline. Implemented in `parser/stmt_drop.go` (the
`ON` branch) + the new `exp.OnProperty` node (`expressions/kinds.go`, `expressions/fidelity_properties.go`)
+ generator `onPropertySQL` (`generator/create_properties.go`); regression test `TestParseDropIndexOnTable`.

---

## Opt-in behavioral extensions beyond upstream
Expand Down
4 changes: 2 additions & 2 deletions corpus_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -251,7 +251,7 @@ func writeGaps(fails map[gapKey]string) error {
// SQLGLOT_CORPUS_UPDATE=1 run on the merged tree.
const (
minPassBase = 955
minPassMySQL = 424
minPassMySQL = 425
minPassPostgres = 468
)

Expand All @@ -265,7 +265,7 @@ const (
// legitimately grows; a count below any floor means the corpus shrank.
const (
minTotalBase = 955
minTotalMySQL = 424
minTotalMySQL = 425
minTotalPostgres = 468
)

Expand Down
1 change: 1 addition & 0 deletions expressions/fidelity_nodes_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -105,6 +105,7 @@ func TestFidelityPropertyMetadata(t *testing.T) {
{"MaterializedProperty", KindMaterializedProperty, MaterializedProperty, []string{"this"}, Args{}},
{"NoPrimaryIndexProperty", KindNoPrimaryIndexProperty, NoPrimaryIndexProperty, []string{}, Args{}},
{"OnCommitProperty", KindOnCommitProperty, OnCommitProperty, []string{"delete"}, Args{}},
{"OnProperty", KindOnProperty, OnProperty, []string{"this"}, Args{"this": fidelityScalar()}},
{"PartitionedByProperty", KindPartitionedByProperty, PartitionedByProperty, []string{"this"}, Args{"this": fidelityScalar()}},
{"PartitionByRangeProperty", KindPartitionByRangeProperty, PartitionByRangeProperty, []string{"partition_expressions", "create_expressions"}, Args{"partition_expressions": fidelityExpressions(), "create_expressions": fidelityExpressions()}},
{"PartitionByListProperty", KindPartitionByListProperty, PartitionByListProperty, []string{"partition_expressions", "create_expressions"}, Args{"partition_expressions": fidelityExpressions(), "create_expressions": fidelityExpressions()}},
Expand Down
1 change: 1 addition & 0 deletions expressions/fidelity_properties.go
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ func LockingProperty(args Args) Expression { return newNode(KindLockingPr
func MaterializedProperty(args Args) Expression { return newNode(KindMaterializedProperty, args) }
func NoPrimaryIndexProperty(args Args) Expression { return newNode(KindNoPrimaryIndexProperty, args) }
func OnCommitProperty(args Args) Expression { return newNode(KindOnCommitProperty, args) }
func OnProperty(args Args) Expression { return newNode(KindOnProperty, args) }
func PartitionedByProperty(args Args) Expression { return newNode(KindPartitionedByProperty, args) }
func PartitionByRangeProperty(args Args) Expression {
return newNode(KindPartitionByRangeProperty, args)
Expand Down
3 changes: 3 additions & 0 deletions expressions/kinds.go
Original file line number Diff line number Diff line change
Expand Up @@ -577,6 +577,7 @@ const (
KindMaterializedProperty
KindNoPrimaryIndexProperty
KindOnCommitProperty
KindOnProperty
KindPartitionedByProperty
KindPartitionByRangeProperty
KindPartitionByListProperty
Expand Down Expand Up @@ -1167,6 +1168,7 @@ var argTypes = map[Kind][]argSpec{
KindMaterializedProperty: {{"this", false}},
KindNoPrimaryIndexProperty: {},
KindOnCommitProperty: {{"delete", false}},
KindOnProperty: {{"this", true}},
KindPartitionedByProperty: {{"this", true}},
KindPartitionByRangeProperty: {{"partition_expressions", true}, {"create_expressions", true}},
KindPartitionByListProperty: {{"partition_expressions", true}, {"create_expressions", true}},
Expand Down Expand Up @@ -1905,6 +1907,7 @@ var className = map[Kind]string{
KindMaterializedProperty: "MaterializedProperty",
KindNoPrimaryIndexProperty: "NoPrimaryIndexProperty",
KindOnCommitProperty: "OnCommitProperty",
KindOnProperty: "OnProperty",
KindPartitionedByProperty: "PartitionedByProperty",
KindPartitionByRangeProperty: "PartitionByRangeProperty",
KindPartitionByListProperty: "PartitionByListProperty",
Expand Down
8 changes: 8 additions & 0 deletions generator/create_properties.go
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@ var propertyLocations = map[expressions.Kind]propertyLocation{
expressions.KindLockingProperty: propertyLocationPostAlias,
expressions.KindNoPrimaryIndexProperty: propertyLocationPostExpression,
expressions.KindOnCommitProperty: propertyLocationPostExpression,
expressions.KindOnProperty: propertyLocationPostSchema,
expressions.KindTriggerProperties: propertyLocationPostExpression,
expressions.KindWithDataProperty: propertyLocationPostExpression,
}
Expand Down Expand Up @@ -92,6 +93,7 @@ func init() {
dispatch[expressions.KindLikeProperty] = (*Generator).likePropertySQL
dispatch[expressions.KindNoPrimaryIndexProperty] = (*Generator).noPrimaryIndexPropertySQL
dispatch[expressions.KindOnCommitProperty] = (*Generator).onCommitPropertySQL
dispatch[expressions.KindOnProperty] = (*Generator).onPropertySQL
dispatch[expressions.KindSqlReadWriteProperty] = (*Generator).sqlReadWritePropertySQL
dispatch[expressions.KindLockingProperty] = (*Generator).lockingPropertySQL
dispatch[expressions.KindPartitionedByProperty] = (*Generator).partitionedByPropertySQL
Expand Down Expand Up @@ -257,6 +259,12 @@ func (g *Generator) onCommitPropertySQL(e expressions.Expression) string {
return "ON COMMIT " + rows + " ROWS"
}

// onPropertySQL ports OnProperty (generator.py:227): `ON <this>`, e.g. the `ON <table>` target of
// MySQL `DROP INDEX <idx> ON <table>` (carried in Drop.cluster) and the CREATE `ON <cluster>` property.
func (g *Generator) onPropertySQL(e expressions.Expression) string {
return "ON " + g.sqlKey(e, "this")
}

func (g *Generator) sqlReadWritePropertySQL(e expressions.Expression) string { return e.Name() }

func (g *Generator) lockingPropertySQL(e expressions.Expression) string {
Expand Down
1 change: 1 addition & 0 deletions generator/create_properties_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,7 @@ func TestCreatePropertyRenderers(t *testing.T) {
{"no primary index", nil, exp.NoPrimaryIndexProperty(exp.Args{}), "NO PRIMARY INDEX"},
{"on commit preserve", nil, exp.OnCommitProperty(exp.Args{"delete": false}), "ON COMMIT PRESERVE ROWS"},
{"on commit delete", nil, exp.OnCommitProperty(exp.Args{"delete": true}), "ON COMMIT DELETE ROWS"},
{"on property", nil, exp.OnProperty(exp.Args{"this": exp.Identifier(exp.Args{"this": "users", "quoted": false})}), "ON users"},
{"SQL read write", nil, exp.SqlReadWriteProperty(exp.Args{"this": "MODIFIES SQL DATA"}), "MODIFIES SQL DATA"},
{"locking", nil, exp.LockingProperty(exp.Args{"kind": "ROW", "for_or_in": "FOR", "lock_type": "ACCESS", "override": false}), "LOCKING ROW FOR ACCESS"},
{"partitioned by base", nil, exp.PartitionedByProperty(exp.Args{"this": exp.Anonymous(exp.Args{"this": "HASH", "expressions": []exp.Expression{createColumn("foo")}})}), "PARTITIONED_BY=HASH(foo)"},
Expand Down
4 changes: 4 additions & 0 deletions parser/parser_properties_fidelity.go
Original file line number Diff line number Diff line change
Expand Up @@ -351,6 +351,10 @@ func (p *Parser) parseOnProperty() exp.Expression {
if p.matchTextSeq("COMMIT", "DELETE", "ROWS") {
return exp.OnCommitProperty(exp.Args{"delete": true})
}
// The bare `ON <id>` → exp.OnProperty half of _parse_on_property (parser.py:3345) is the
// ClickHouse `ON CLUSTER` form for CREATE/ALTER, which is out of this port's dialect scope; the
// only supported OnProperty use is MySQL `DROP INDEX … ON <table>`, built directly in parseDrop
// (with a full table target so it accepts the db-qualified form). So this stays nil here.
return nil
}

Expand Down
19 changes: 7 additions & 12 deletions parser/stmt_drop.go
Original file line number Diff line number Diff line change
Expand Up @@ -41,18 +41,13 @@ func (p *Parser) parseDrop() exp.Expression {

var cluster exp.Expression
if p.match(tokens.ON) {
cluster = p.parseOnProperty()
if cluster == nil {
// `DROP INDEX <idx> ON <table>`: the ON-clause target is an exp.OnProperty (not
// ported; parseOnProperty matches ON but returns nil, leaving <table> unconsumed).
// parseDrop isn't wrapped in tryParse, so the leftover would otherwise surface as a
// hard "Unexpected token" error at the batch level. Degrade to a raw Command (guide:
// "leftover/unmatched -> parseAsCommand(start)"); parseAsCommand is source-position
// based, so it re-captures the whole statement and round-trips byte-identically.
// Scoped to the ON branch so parseDrop stays reusable as a CSV sub-parser inside
// ALTER ... DROP (e.g. `DROP COLUMN c, DROP PRIMARY KEY`), which never has an ON.
return p.parseAsCommand(start)
}
// `DROP INDEX <idx> ON <table>` (MySQL): the ON-clause target is an OnProperty carrying the
// table (upstream Drop.cluster, parser.py:2325). divergence: upstream's shared _parse_on_property
// parses only a single id (_parse_schema(_parse_id_var), parser.py:3345), so it rejects the
// db-qualified `ON db.tbl` that real MySQL 8.0.46 accepts. Parse the full table parts here —
// scoped to DROP so CREATE/ALTER keep upstream's ON handling — so the qualifier survives. The
// missing-name case fails closed via parseTableParts' own raiseError. See DEVIATIONS §1.
cluster = p.expression(exp.OnProperty(exp.Args{"this": p.parseTableParts(false, false, false, false)}), nil, nil)
}

var expressions []exp.Expression
Expand Down
32 changes: 32 additions & 0 deletions parser/stmt_drop_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -150,6 +150,38 @@ func TestParseDropPostgresConcurrently(t *testing.T) {
}
}

// TestParseDropIndexOnTable ports the MySQL `DROP INDEX <idx> ON <table>` form (parser.py:2325):
// the ON-clause target is an OnProperty carried in Drop.cluster. Previously the port had no
// OnProperty node, so it degraded to Command. The target is parsed as a full table so the
// db-qualified form real MySQL 8.0.46 accepts survives (upstream's single-id parse rejects it).
func TestParseDropIndexOnTable(t *testing.T) {
drop := parseOneDialect(t, "DROP INDEX idx_email ON users", "mysql")
if drop.Kind() != exp.KindDrop || drop.Arg("kind") != "INDEX" {
t.Fatalf("DROP INDEX ON: kind mismatch (want Drop/INDEX):\n%s", drop.ToS())
}
if exprArg(t, drop, "this").Name() != "idx_email" {
t.Fatalf("DROP INDEX ON: index name mismatch:\n%s", drop.ToS())
}
cluster := exprArg(t, drop, "cluster")
if cluster.Kind() != exp.KindOnProperty {
t.Fatalf("DROP INDEX ON: cluster should be OnProperty:\n%s", drop.ToS())
}
target := exprArg(t, cluster, "this")
if target.Kind() != exp.KindTable || target.Text("this") != "users" {
t.Fatalf("DROP INDEX ON: OnProperty target should be Table(users):\n%s", drop.ToS())
}

// db-qualified target: accepted (matches real MySQL), the qualifier preserved in the Table.
drop = parseOneDialect(t, "DROP INDEX idx ON db.users", "mysql")
target = exprArg(t, exprArg(t, drop, "cluster"), "this")
if target.Kind() != exp.KindTable || target.Text("schema") != "db" || target.Text("this") != "users" {
t.Fatalf("DROP INDEX ON db.users: qualifier not preserved:\n%s", drop.ToS())
}
if out, err := generateSQL(t, drop, "mysql"); err != nil || out != "DROP INDEX idx ON db.users" {
t.Fatalf("DROP INDEX ON db.users round-trip = %q, err=%v", out, err)
}
}

// TestParseDropDegradesToCommand covers DROP statements this port doesn't structurally
// model: an unrecognized creatable (out of the creatables set) and ICEBERG-qualified DROP
// on a non-TABLE kind (parser.py:2311-2315).
Expand Down
1 change: 1 addition & 0 deletions testdata/dialect_identity.jsonl
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,7 @@
{"dialect":"mysql","sql":"ALTER TABLE t CHANGE COLUMN c d VARCHAR(50) DEFAULT 'x'","want":"ALTER TABLE t CHANGE COLUMN c d VARCHAR(50) DEFAULT 'x'","pretty":false}
{"dialect":"mysql","sql":"ALTER TABLE t CHANGE a b BIGINT NOT NULL","want":"ALTER TABLE t CHANGE COLUMN a b BIGINT NOT NULL","pretty":false}
{"dialect":"mysql","sql":"ALTER TABLE t DROP COLUMN c, DROP PRIMARY KEY, DROP INDEX `i`","want":"ALTER TABLE t DROP COLUMN c, DROP PRIMARY KEY, DROP INDEX `i`","pretty":false}
{"dialect":"mysql","sql":"DROP INDEX idx_email ON users","want":"DROP INDEX idx_email ON users","pretty":false}
{"dialect":"mysql","sql":"ALTER TABLE t DROP PRIMARY KEY","want":"ALTER TABLE t DROP PRIMARY KEY","pretty":false}
{"dialect":"mysql","sql":"ALTER TABLE t MODIFY COLUMN c INT AFTER d","want":"ALTER TABLE t MODIFY COLUMN c INT AFTER d","pretty":false}
{"dialect":"mysql","sql":"ALTER TABLE t MODIFY COLUMN c INT COMMENT 'hi'","want":"ALTER TABLE t MODIFY COLUMN c INT COMMENT 'hi'","pretty":false}
Expand Down
1 change: 1 addition & 0 deletions testdata/upstream_extensions.jsonl
Original file line number Diff line number Diff line change
Expand Up @@ -40,3 +40,4 @@
{"id":"pg-set-multi-value","dialect":"postgres","sql":"SET search_path = a, b","upstream":"command","go_kind":"Set","reconcile":"If a future pin structures Postgres multi-value SET (SET var = v1, v2), adopt its representation, remove the Postgres value-list branch in parseSetItemAssignment (parser/stmt_set.go) + the multi-value fold in setItemSQL (generator/stmt_set.go), and delete this row. See DEVIATIONS 'Grammar extensions beyond upstream'."}
{"id":"pg-set-transaction-deferrable","dialect":"postgres","sql":"SET TRANSACTION DEFERRABLE","upstream":"parse_error","go_kind":"Set","reconcile":"The [NOT] DEFERRABLE transaction_mode lives in the Postgres-only pgTransactionCharacteristics (parser/sets_statements.go), which clone-extends the dialect-shared transactionCharacteristics (that omits DEFERRABLE, since MySQL/base reject it). Upstream raises 'Unknown option DEFERRABLE' for this SQL via its shared TRANSACTION_CHARACTERISTICS table; if a future pin adds DEFERRABLE there, this row's upstream behavior flips (parse_error -> structured) and the tripwire fires — then fold DEFERRABLE back into the shared path and drop pgTransactionCharacteristics + this row. See DEVIATIONS 'Grammar extensions beyond upstream'."}
{"id":"mysql-table-value","dialect":"mysql","sql":"TABLE db.users","upstream":"parse_error","go_kind":"Select","reconcile":"MySQL 8.0.19+ TABLE tbl_name is SELECT * FROM tbl_name. Pinned upstream mis-parses the UNqualified TABLE users as an Alias (tracked as the DEVIATIONS §1.15 correctness fix), but parse-errors the schema-qualified TABLE db.users at the dot — so the qualified spelling is the grammar-beyond-upstream case (cf. the §1.8 SAVEPOINT/RELEASE split). If a future pin adds a structural TABLE value constructor, adopt its node shape and remove parseMysqlTableStatement (parser/stmt_mysql_table.go) + the statementDepth==1 hook in parser/parser.go, then delete this row. Verified: pinned upstream parse-errors TABLE db.users; MySQL 8.0.46 returns the same rows as SELECT * FROM db.users."}
{"id":"mysql-drop-index-on-qualified","dialect":"mysql","sql":"DROP INDEX idx ON db.users","upstream":"parse_error","go_kind":"Drop","reconcile":"MySQL DROP INDEX index_name ON tbl_name permits a db-qualified tbl_name (verified MySQL 8.0.46). Pinned upstream's shared _parse_on_property parses only a single id (_parse_schema(_parse_id_var), parser.py:3345), so it parse-errors the qualified `ON db.users` at the dot; the unqualified `DROP INDEX idx ON users` is structured by upstream (a port completion here, not tracked). If a future pin parses a qualified DROP INDEX ON target, drop this row and re-align parseDrop's ON branch with upstream's node shape (upstream uses OnProperty(this=Identifier); the port uses a full Table target to carry the qualifier — decide whether to keep that). See DEVIATIONS §1.16."}