Fix invalid SQL for column-level CHECK constraints using range/BETWEEN expressions - #1371
Open
vjymisal0 wants to merge 1 commit into
Open
Fix invalid SQL for column-level CHECK constraints using range/BETWEEN expressions#1371vjymisal0 wants to merge 1 commit into
vjymisal0 wants to merge 1 commit into
Conversation
The `~=` operator overloads used for Range/ClosedRange/PartialRange
pattern matching (e.g. `0...26 ~= column`) build their SQL fragment
without wrapping it in parentheses, unlike every other comparison
operator (`>`, `<`, `==`, etc.), which all self-wrap via
`Operator.infix(...)`.
This is harmless standalone (e.g. inside `.filter(...)`), but it
produces invalid SQL when used as a column-level CHECK constraint via
`table.create { t in t.column(col, check: 0...26 ~= col) }`, because
Schema.swift's per-column `definition()` relies on the check
expression already being parenthesized (it just does
`"CHECK" + " " + check`, same as the table-level `check()` builder
relies on `.prefix` to add its own parens). Since BETWEEN/range
expressions weren't self-wrapped, the generated SQL came out as:
CREATE TABLE "t" ("type" INTEGER NOT NULL CHECK "type" BETWEEN 0 AND 26)
which SQLite rejects with a syntax error near the CHECK condition,
exactly as reported in stephencelis#1056.
Fixes stephencelis#1056.
Fix: wrap the `~=` operators' generated SQL in parens, matching the
convention already used by every other comparison operator. Updated
the existing BETWEEN/range operator tests to expect the parenthesized
output, and added a regression test in SchemaTests that reproduces the
exact column-level CHECK scenario from the issue and asserts the
generated CREATE TABLE statement is now well-formed.
Verified by tracing the SQL-generation code path end-to-end (Operator
`infix`/`wrap` helpers, the `~=` overloads, and Schema.swift's
`definition()`/`check()`) since a full Swift toolchain wasn't
available in this environment to run `swift test` directly; the
updated/added unit tests exercise this exact path and should be run
by CI.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
Fixes #1056.
Column-level
CHECKconstraints built from a range pattern (e.g.t.column(type, check: 0...26 ~= type)) generate invalid SQL and SQLite rejects theCREATE TABLEstatement with a syntax error, exactly as reported in #1056:Root cause
Every comparison operator (
>,<,==,>=, etc.) self-wraps its generated SQL in parentheses viaOperator.infix(...)(seeSources/SQLite/Typed/Operators.swift), e.g.column > 0compiles to("column" > 0).The
~=operator overloads used forRange/ClosedRange/PartialRange...pattern matching (Sources/SQLite/Typed/Operators.swift:560-599) are the one exception — they build their BETWEEN/comparison SQL fragment directly without wrapping it in parens.This is harmless when the expression is used standalone (e.g.
.filter(0...26 ~= type)), but it breaks when used as a column-levelCHECKconstraint.Schema.swift's per-columndefinition()builds the CHECK clause as"CHECK" + " " + check(Sources/SQLite/Typed/Schema.swift:578), relying on the passed-in expression already being parenthesized — which holds for every other comparator, but not for the un-wrapped BETWEEN/range output. (The table-levelcheck()builder inSchema.swift:438doesn't hit this because it uses"CHECK".prefix(condition), which unconditionally adds its own parens — that's why the issue reporter found "table checks" worked around the bug.)Fix
Wrap the SQL generated by all 10
~=overloads (ClosedRange,Range,PartialRangeThrough,PartialRangeUpTo,PartialRangeFrom, each with an optional-column variant) in parentheses, matching the convention every other comparison operator already follows. This makes range-based conditions self-contained/composable like the rest of the operator library, and fixes the column-level CHECK bug without touchingSchema.swiftat all.Testing
Tests/SQLiteTests/Typed/OperatorsTests.swiftto expect the now-parenthesized output (purely a string-format change; the underlying SQL semantics/bindings are unchanged since parens don't alter meaning in these positions).test_column_withRangeCheck_compilesValidCheckConstraintinTests/SQLiteTests/Schema/SchemaTests.swift, which reproduces the exact column-level CHECK scenario from Column check constraints generate a syntax error #1056 and asserts the generatedCREATE TABLEstatement is now well-formed:CREATE TABLE "table" ("int64" INTEGER NOT NULL CHECK ("int64" BETWEEN 0 AND 26)).swift testdirectly, so I traced the fix end-to-end through theOperator.infix/.wrap/.prefixhelpers, the~=overloads, andSchema.swift'sdefinition()/check()to confirm the generated string exactly matches the new test expectations (including that non-optional/optional variants and the standalone table-levelcheck()path are unaffected). Would appreciate CI running the updated suite.