Skip to content

Commit 41ab370

Browse files
HexclesCursor Agent
andauthored
Bump sql-psi 0.5.2 → 0.7.3 and sqldelight 2.1.0 → 2.3.2 (#167)
<!-- CURSOR_AGENT_PR_BODY_BEGIN --> ## Summary Bumps the two core upstream dependencies together (they release as a pair): - `app.cash.sql-psi:core` **0.5.2 → 0.7.3** - `app.cash.sqldelight:*` **2.1.0 → 2.3.2** ## What changed 1. **`gradle/libs.versions.toml`** — version bumps only. 2. **`cockroachdb-dialect/.../grammar/CockroachDB.bnf`** — added an override for the new `enum_data_type` production that PostgreSQL added in sqldelight 2.2.x. Upstream's `enum_data_type ::= {identifier}` is an unrestricted wildcard that turned every identifier into a valid column type, which broke parsing of CockroachDB-specific table-level constructs like `FAMILY f1 (id, last_accessed)` (the parser greedily consumed `FAMILY f1` as a `column_def` instead of falling back to `table_family_constraint`). The override replaces the production with a never-matching placeholder. Trade-off documented in a code comment: this dialect doesn't currently parse `CREATE TYPE ... AS ENUM` either (it isn't part of cockroach's `extension_stmt` override), so removing the wildcard has no functional regression today. 3. **`cockroachdb-dialect/.../CockroachDBFixturesTest.kt`** — extended the existing `excludedAnsiFixtures` and `excludedPgSqlFixtures` lists for inherited test fixtures whose statements aren't currently parseable by this dialect, with explanatory comments grouped by reason: - SQLite-style `CREATE TRIGGER ... BEGIN ... END;` ANSI fixtures (the PostgreSQL trigger grammar is `EXECUTE FUNCTION fn();`). - `DROP TRIGGER IF EXISTS test;` without `ON tablename` (SQLite syntax). - PostgreSQL-only DDL added in sqldelight 2.2.x / 2.3.x: CREATE/ALTER/DROP POLICY, CREATE TYPE/ENUM, CREATE/DROP FUNCTION, COMMENT ON, DROP TRIGGER, CREATE OR REPLACE TRIGGER, ALTER TABLE … ROW LEVEL SECURITY, ALTER TABLE … SET (storage_param=…). All valid CockroachDB SQL but not yet wired into this dialect's `extension_stmt` / `alter_table_rules` overrides. ## Verification ``` ./gradlew :cockroachdb-dialect:test # 174 fixture tests, 0 failures ./gradlew :integration-testing:assemble :integration-testing:compileTestKotlin ./gradlew spotlessCheck ./gradlew check -x :integration-testing:test # all green locally ``` `./gradlew :integration-testing:test` was not run locally (requires Docker for testcontainers + CockroachDB image, not available in the dev VM). CI on this PR will exercise it. ## Out of scope / follow-ups Extending the cockroach grammar to actually parse policies, enums, functions, comment-on, row-level-security, set-storage-parameter, and the new trigger forms. CockroachDB does support these on the database side; the work is to add productions to `extension_stmt` / `alter_table_rules` and likely new mixins. That would also let us drop most of the new exclusions and replace the `enum_data_type` placeholder with the real production. <!-- CURSOR_AGENT_PR_BODY_END --> <div><a href="https://cursor.com/agents/bc-fd4519a4-4bc1-42cd-9d8d-c77c1421faff"><picture><source media="(prefers-color-scheme: dark)" srcset="https://cursor.com/assets/images/open-in-web-dark.png"><source media="(prefers-color-scheme: light)" srcset="https://cursor.com/assets/images/open-in-web-light.png"><img alt="Open in Web" width="114" height="28" src="https://cursor.com/assets/images/open-in-web-dark.png"></picture></a>&nbsp;<a href="https://cursor.com/background-agent?bcId=bc-fd4519a4-4bc1-42cd-9d8d-c77c1421faff"><picture><source media="(prefers-color-scheme: dark)" srcset="https://cursor.com/assets/images/open-in-cursor-dark.png"><source media="(prefers-color-scheme: light)" srcset="https://cursor.com/assets/images/open-in-cursor-light.png"><img alt="Open in Cursor" width="131" height="28" src="https://cursor.com/assets/images/open-in-cursor-dark.png"></picture></a>&nbsp;</div> --------- Co-authored-by: Cursor Agent <cursor-agent@cursor.sh>
1 parent 57b1031 commit 41ab370

4 files changed

Lines changed: 147 additions & 15 deletions

File tree

cockroachdb-dialect/src/main/kotlin/com/faire/sqldelight/dialects/cockroachdb/grammar/CockroachDB.bnf

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -100,6 +100,7 @@ overrides ::= table_constraint
100100
| string_data_type
101101
| date_data_type
102102
| blob_data_type
103+
| enum_data_type
103104
| generated_clause
104105
| alter_table_rules
105106
| extension_stmt
@@ -187,6 +188,15 @@ blob_data_type ::= 'BYTEA' | 'BLOB' | 'BYTES' {
187188
override = true
188189
}
189190

191+
// PostgreSQL added enum_data_type as an unrestricted identifier wildcard. That makes any token
192+
// (e.g. FAMILY in a CockroachDB family constraint) eligible as a column type, which prevents the
193+
// parser from falling back to table_constraint alternatives. CockroachDB supports CREATE TYPE AS
194+
// ENUM but this dialect does not currently parse it, so we override the production with a never-
195+
// matching placeholder token.
196+
enum_data_type ::= 'COCKROACHDB_ENUM_DATA_TYPE_DISABLED_PLACEHOLDER' {
197+
override = true
198+
}
199+
190200
create_index_stmt ::= CREATE [ UNIQUE ] INDEX [ 'CONCURRENTLY' ] [ IF NOT EXISTS ] [ [ ansi_database_name DOT ] ansi_index_name ] ON ansi_table_name (
191201
USING {index_method} LP ansi_indexed_column [ {operator_class_stmt} ] ( COMMA ansi_indexed_column [ {operator_class_stmt} ] ) * RP [ {with_storage_parameter} ] |
192202
LP ansi_indexed_column [ {operator_class_stmt} ] ( COMMA ansi_indexed_column [ {operator_class_stmt} ] ) * RP [ index_using_hash ] [ ('STORING' | 'COVERING' | 'INCLUDE') LP ansi_indexed_column ( COMMA ansi_indexed_column ) * RP ] [ WHERE <<expr '-1'>> ]

cockroachdb-dialect/src/test/kotlin/com/faire/sqldelight/dialects/cockroachdb/CockroachDBFixturesTest.kt

Lines changed: 134 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -23,16 +23,33 @@ import java.io.File
2323

2424
@RunWith(Parameterized::class)
2525
class CockroachDBFixturesTest(name: String, fixtureRoot: File) : FixturesTest(name, fixtureRoot) {
26+
// The upstream sql-psi ANSI fixtures are written for a SQLite-flavored dialect. These
27+
// text substitutions translate the most common SQLite-isms into their
28+
// CockroachDB / PostgreSQL equivalents before the fixture is parsed:
29+
//
30+
// - INTEGER NOT NULL PRIMARY KEY AUTOINCREMENT -> SERIAL NOT NULL PRIMARY KEY
31+
// SQLite's AUTOINCREMENT only applies to INTEGER PRIMARY KEY columns; CockroachDB uses
32+
// SERIAL (https://www.cockroachlabs.com/docs/stable/serial).
33+
// - AUTOINCREMENT -> "" (drop the keyword everywhere else; column types like SERIAL
34+
// auto-increment by default in CockroachDB).
35+
// - ?1 / ?2 -> ? CockroachDB/PostgreSQL bind parameters are positional ($1, $2, ...) or
36+
// unnumbered ?, not ?1/?2.
37+
// - BLOB -> TEXT CockroachDB has BYTEA/BYTES/BLOB, but the upstream BLOB-typed columns
38+
// are usually used in TEXT-comparison contexts; mapping to TEXT keeps the test green
39+
// without changing what's being asserted.
40+
// - "id TEXT GENERATED ALWAYS AS (2) ... NOT NULL" gets a STORED keyword inserted because
41+
// PostgreSQL/CockroachDB require GENERATED ALWAYS AS (...) STORED (no VIRTUAL form for
42+
// PostgreSQL; CockroachDB allows STORED or VIRTUAL).
43+
//
44+
// (The previously-present rule rewriting the multiple-column-where parser-error message
45+
// was dead code: that fixture is excluded entirely below.)
2646
override val replaceRules = arrayOf(
27-
// TODO: document why
2847
"INTEGER NOT NULL PRIMARY KEY AUTOINCREMENT" to "SERIAL NOT NULL PRIMARY KEY",
2948
"AUTOINCREMENT" to "",
3049
"?1" to "?",
3150
"?2" to "?",
3251
"BLOB" to "TEXT",
3352
"id TEXT GENERATED ALWAYS AS (2) UNIQUE NOT NULL" to "id TEXT GENERATED ALWAYS AS (2) STORED UNIQUE NOT NULL",
34-
"'(', ')', ',', '.', <binary like operator real>, BETWEEN or IN expected, got ','"
35-
to "'#-', '(', ')', ',', '.', <binary like operator real>, <jsona binary operator real>, <jsonb boolean operator real>, '@@', BETWEEN or IN expected, got ','",
3653
)
3754

3855
override fun setupDialect() {
@@ -41,23 +58,128 @@ class CockroachDBFixturesTest(name: String, fixtureRoot: File) : FixturesTest(na
4158

4259
companion object {
4360
private val excludedAnsiFixtures = listOf(
44-
// Excluded since we're not validating indices when dropping them.
61+
// The cockroach grammar's CreateIndexMixin and DropIndexMixin intentionally leave
62+
// modifySchema()/annotate() empty (see those files for rationale: CockroachDB allows
63+
// unnamed indexes, indexes declared inline in CREATE TABLE, and the same index name
64+
// across different tables, none of which the upstream sql-psi schema tracker handles).
65+
// The result is that we don't currently surface "duplicate index name" / "no index
66+
// found" diagnostics, which these fixtures assert.
67+
//
68+
// TODO(validation): CockroachDB does enforce index-name uniqueness within a single
69+
// table (see https://www.cockroachlabs.com/docs/stable/create-index). Once unnamed
70+
// indexes and inline-CREATE-TABLE indexes are tracked in the schema, these fixtures
71+
// could be re-enabled.
4572
"index-migration",
46-
// Excluded since we're not validating indices when creating them.
73+
"create-index-collision",
74+
// Mostly exercises CREATE TABLE foreign-key constraint validation (composite primary
75+
// key matching, "unique index on column" lookups, duplicate primary key clauses, etc.)
76+
// — all of which CockroachDB does enforce — but several assertions also depend on
77+
// index tracking (see above). Excluded because of the index-tracking gap.
78+
//
79+
// TODO(validation): split or re-enable once CockroachDB index tracking is implemented.
4780
"create-table-validation-failures",
48-
// Excluded since we're not validating indices when creating them.
81+
// Tests CREATE TABLE/VIEW/INDEX/TRIGGER + IF NOT EXISTS collision detection. The
82+
// CREATE INDEX collision lines hit the same index-tracking gap as above, and the
83+
// CREATE TRIGGER lines use SQLite-style `BEGIN ... END;` trigger bodies that neither
84+
// PostgreSQL nor CockroachDB parse (CockroachDB uses `EXECUTE FUNCTION fn()`).
4985
"create-if-not-exists",
50-
// Excluded since we're not validating indices when creating them.
51-
"create-index-collision",
52-
// Excluded since our error message is different;
53-
// we've copied the test case, but without the failure case, into `multiple-column-where-ansi`.
86+
// The fixture asserts a parser error message that is shorter than what cockroach
87+
// actually emits, because the cockroach/postgres expression grammar adds many extra
88+
// operator alternatives (`<jsona binary operator real>`, `<jsonb boolean operator
89+
// real>`, `'#-'`, `'@@'`, `<contains operator real>`, etc.) to the "expected" list.
90+
// We've copied the valid-syntax half of this fixture to `multiple-column-where-ansi`.
91+
// Not a CockroachDB feature gap.
5492
"multiple-column-where",
93+
// The following ANSI fixtures use SQLite-style `CREATE TRIGGER ... BEGIN ... END;`
94+
// syntax, which is not parseable by the PostgreSQL dialect (which we inherit from)
95+
// and is not the form CockroachDB accepts either. CockroachDB's CREATE TRIGGER uses
96+
// the PostgreSQL-style `EXECUTE FUNCTION fn()` form (since v24.3 LTS, see
97+
// https://www.cockroachlabs.com/docs/stable/create-trigger), so these SQLite-flavored
98+
// fixtures can't be supported as-written.
99+
"create-trigger-collision",
100+
"create-trigger-docid",
101+
"create-trigger-raise",
102+
"create-trigger-success",
103+
"create-trigger-validation-failures",
104+
"rowid-triggers",
105+
"timestamp-literals",
106+
"trigger-migration",
107+
"trigger-new-in-expression",
108+
"update-view-with-trigger",
109+
// `DROP TRIGGER IF EXISTS test3;` (no `ON tablename`) is SQLite syntax. The
110+
// PostgreSQL dialect (and CockroachDB, see
111+
// https://www.cockroachlabs.com/docs/stable/drop-trigger) require
112+
// `DROP TRIGGER ... ON tablename`. The CREATE TRIGGER block in this fixture is also
113+
// SQLite-flavored.
114+
"if-not-exists",
55115
)
56116

57117
private val excludedPgSqlFixtures = listOf(
58-
// Excluded since we're not validating indices when creating them;
59-
// we've copied the test case, but without error assertions, into `create-index-pgsql`.
118+
// The fixture asserts on PostgreSQL's index storage-parameter validators (fillfactor,
119+
// deduplicate_items, fastupdate, gin_pending_list_limit, autosummarize, pages_per_range
120+
// ranges/booleans). The cockroach `create_index_stmt` in CockroachDB.bnf includes
121+
// [ {with_storage_parameter} ] syntactically but does not run those validators (the
122+
// cockroach CreateIndexMixin overrides annotate() with a no-op). We've copied the
123+
// valid-syntax half of this fixture to `create-index-pgsql` (without error
124+
// assertions).
125+
//
126+
// TODO(validation): CockroachDB does support / reject storage parameters on indexes
127+
// (https://www.cockroachlabs.com/docs/stable/create-index), and the cockroach-specific
128+
// ones (e.g. `bucket_count` for hash-sharded indexes) ought to be validated here too.
129+
// Once index tracking is wired up (see excludedAnsiFixtures above) the storage-param
130+
// validation could be inherited from the postgres mixin.
60131
"create-index",
132+
// The following PostgreSQL fixtures exercise statements added to PostgreSQL's
133+
// `extension_stmt` / `alter_table_rules` rules in sqldelight 2.2.x / 2.3.x. CockroachDB's
134+
// grammar overrides those rules without these productions, so they currently fail to
135+
// parse. Each is supported by CockroachDB itself and would be a worthwhile follow-up.
136+
//
137+
// TODO(grammar): Add CREATE/ALTER/DROP POLICY to the cockroach `extension_stmt` rule.
138+
// CockroachDB supports row-level security policies as of v25.2 with PostgreSQL-compatible
139+
// syntax. https://www.cockroachlabs.com/docs/stable/create-policy
140+
"alter-policy",
141+
"create-policy",
142+
"drop-policy",
143+
// TODO(grammar): Add COMMENT ON to the cockroach `extension_stmt` rule. CockroachDB
144+
// supports `COMMENT ON DATABASE/SCHEMA/TYPE/TABLE/COLUMN/INDEX/CONSTRAINT` (note: VIEW is
145+
// not currently listed in the CockroachDB docs, so the upstream fixture's
146+
// `COMMENT ON VIEW VSomeTable IS '...'` line may need a dialect-specific variant).
147+
// https://www.cockroachlabs.com/docs/stable/comment-on
148+
"comment-on",
149+
// TODO(grammar): Add CREATE/ALTER/DROP TYPE (enum) to the cockroach `extension_stmt`
150+
// rule. CockroachDB supports `CREATE TYPE name AS ENUM (...)`, `ALTER TYPE`, and
151+
// `DROP TYPE` with PostgreSQL-compatible syntax.
152+
// https://www.cockroachlabs.com/docs/stable/create-type
153+
"create-enum",
154+
// TODO(grammar): Add CREATE/DROP FUNCTION to the cockroach `extension_stmt` rule.
155+
// CockroachDB has supported user-defined functions since v22.2.
156+
// https://www.cockroachlabs.com/docs/stable/create-function
157+
// https://www.cockroachlabs.com/docs/stable/drop-function
158+
"drop-function",
159+
// TODO(grammar): Add CREATE TRIGGER / CREATE OR REPLACE TRIGGER / DROP TRIGGER to the
160+
// cockroach grammar. CockroachDB supports triggers (since v24.3 LTS) with the
161+
// PostgreSQL-style `EXECUTE FUNCTION fn()` form, and `CREATE OR REPLACE TRIGGER` since
162+
// v26.2. The upstream fixtures should pass once these productions are inherited.
163+
// https://www.cockroachlabs.com/docs/stable/create-trigger
164+
// https://www.cockroachlabs.com/docs/stable/drop-trigger
165+
"create-or-replace-trigger",
166+
"drop-trigger",
167+
// TODO(grammar): Add `ALTER TABLE ... { ENABLE | DISABLE } ROW LEVEL SECURITY` to the
168+
// cockroach `alter_table_rules` rule. CockroachDB supports both subcommands as of v25.2.
169+
// The upstream fixture also exercises `FORCE ROW LEVEL SECURITY`, which is not currently
170+
// documented as supported by CockroachDB; that line may need a dialect-specific variant.
171+
// https://www.cockroachlabs.com/docs/stable/alter-table#enable-row-level-security
172+
"alter-table-row-level-security",
173+
// TODO(grammar): Add support for PostgreSQL's `ALTER TABLE ... SET (param = value)` /
174+
// `RESET (...)` storage-parameter form to the cockroach `alter_table_rules` rule.
175+
// CockroachDB does support a SET (storage parameter) form on ALTER TABLE, although the
176+
// set of accepted parameters differs from PostgreSQL (CockroachDB-specific TTL/stats
177+
// parameters are real, while several PostgreSQL parameters such as `fillfactor` and
178+
// `autovacuum_vacuum_scale_factor` are parsed as no-ops or unsupported). Cockroach's
179+
// current `alter_table_set_storage_options` rule covers a different cockroach-only
180+
// `WITH (k=v)` form.
181+
// https://www.cockroachlabs.com/docs/stable/alter-table
182+
"alter-table-set-storage-parameter",
61183
)
62184

63185
// Used by Parameterized JUnit runner reflectively.

gradle.properties

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -2,4 +2,4 @@ org.gradle.parallel=true
22

33
GROUP=com.faire
44
POM_ARTIFACT_ID=sqldelight-cockroachdb-dialect
5-
VERSION_NAME=0.7.0
5+
VERSION_NAME=0.8.0

gradle/libs.versions.toml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,8 @@
22
idea = "231.9392.1"
33
kotlin = "2.3.20"
44
spotless = "8.4.0"
5-
sql-psi = "0.5.2"
6-
sqldelight = "2.1.0"
5+
sql-psi = "0.7.3"
6+
sqldelight = "2.3.2"
77

88
[libraries]
99
assertj-core = { module = "org.assertj:assertj-core", version = "3.27.7" }

0 commit comments

Comments
 (0)