Skip to content

fix(deparser): keep empty bit strings, collision-free dollar tags and case-sensitive option strings valid - #359

Merged
pyramation merged 1 commit into
mainfrom
fix/edge-case-literals
Oct 13, 2026
Merged

pyramation merged 1 commit into
mainfrom
fix/edge-case-literals

Conversation

@pyramation

Copy link
Copy Markdown
Collaborator

Summary

The TS deparser produces SQL that either fails to parse or means something different for three edge cases (constructive-io/constructive-planning#2175). This PR fixes all three.

Real bugs (what's broken today → what changes)

  1. Empty hex bit string: SELECT X'' deparses as SELECT b'x', which PG rejects ("x" is not a valid binary digit). The hex regex ^x[0-9A-Fa-f]+$ now uses *, so the output is x''.

  2. Dollar-quote collisions: getFunctionDelimiter tried $$ and then $EOFCODE$, and used $EOFCODE$ even when the body contained both. It also missed bodies that end with a prefix of the tag, such as ...--$ followed by $$. A tag now counts as a conflict when the tag appears in body + tag before body.length (dollarTagConflicts). If both tags conflict, the deparser tries $EOFCODE1$, $EOFCODE2$, … in order and takes the first that is free, so output is deterministic.

  3. Option strings losing meaning: formatOptionValue printed any identifier-shaped String bare. WITH (opt = 'Foo') became Foo, which folds to foo, and 'int'/'select' became a type or a keyword. A value now prints bare only when it is lowercase [a-z_][a-z0-9_]* and keywordKindOf(v) === 'NO_KEYWORD'. Anything else goes through QuoteUtils.escape.

    Side effect: string values that are keywords ('true', 'simple') now print quoted (security_barrier = 'true'). PG reads both forms as the same String arg, so behaviour is unchanged.

Tests: new fixture __fixtures__/kitchen-sink/misc/literal-edge-cases.sql. npm run kitchen-sink produced generated.json and misc-literal-edge-cases.test.ts. That test fails on main and passes with this change. Full deparser suite: 312 suites / 748 tests pass.

constructive-db ports the same three fixes to its SQL deparser and syncs this fixture into its suite: constructive-io/constructive-db (feat/deparser-edge-literals).

Link to Devin session: https://app.devin.ai/sessions/a49105664e454ee19eb145330f00f61c
Open in Devin Desktop: https://app.devin.ai/desktop/session/a49105664e454ee19eb145330f00f61c?variant=devin
Requested by: @pyramation

@devin-ai-integration

Copy link
Copy Markdown
Contributor

I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".

  • Disable automatic comment, CI, and merge conflict monitoring

@tenki-reviewer

tenki-reviewer Bot commented Oct 10, 2026 •

Copy link
Copy Markdown

Review complete. No issues found — approved ✅.


This PR hardens the SQL deparser against literal edge cases by adding a new kitchen-sink fixture (literal-edge-cases.sql), its generated AST fixture, and a corresponding kitchen-sink test, plus targeted fixes in deparser.ts for literal and quoting behavior.

Files Change
packages/deparser/src/deparser.ts Fixes literal edge-case handling: empty hex/bit-string literals, dollar-tag conflict detection, and option-value quoting
__fixtures__/kitchen-sink/misc/literal-edge-cases.sql New SQL fixture covering 21 literal edge cases
__fixtures__/generated/generated.json Regenerated AST fixture snapshot for the new literals fixture
packages/deparser/__tests__/kitchen-sink/misc-literal-edge-cases.test.ts New kitchen-sink test round-tripping the new fixtures

The scanner wave produced no findings meeting the confidence threshold, so the review verdict is approve.

Reviewed commit: e5373c5

@pyramation
pyramation merged commit cf81016 into main Oct 13, 2026
15 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant