Repository navigation
fix(deparser): keep empty bit strings, collision-free dollar tags and case-sensitive option strings valid - #359
Merged
Merged
Conversation
… case-sensitive option strings valid
Contributor
|
I'll fix CI failures and address comments from users with write access. I'll skip comments containing "(aside)".
|
|
Review complete. No issues found — approved ✅. This PR hardens the SQL deparser against literal edge cases by adding a new kitchen-sink fixture (
The scanner wave produced no findings meeting the confidence threshold, so the review verdict is approve. Reviewed commit: e5373c5 |
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
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)
Empty hex bit string:
SELECT X''deparses asSELECT 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 isx''.Dollar-quote collisions:
getFunctionDelimitertried$$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 inbody + tagbeforebody.length(dollarTagConflicts). If both tags conflict, the deparser tries$EOFCODE1$,$EOFCODE2$, … in order and takes the first that is free, so output is deterministic.Option strings losing meaning:
formatOptionValueprinted any identifier-shapedStringbare.WITH (opt = 'Foo')becameFoo, which folds tofoo, and'int'/'select'became a type or a keyword. A value now prints bare only when it is lowercase[a-z_][a-z0-9_]*andkeywordKindOf(v) === 'NO_KEYWORD'. Anything else goes throughQuoteUtils.escape.Side effect: string values that are keywords (
'true','simple') now print quoted (security_barrier = 'true'). PG reads both forms as the sameStringarg, so behaviour is unchanged.Tests: new fixture
__fixtures__/kitchen-sink/misc/literal-edge-cases.sql.npm run kitchen-sinkproducedgenerated.jsonandmisc-literal-edge-cases.test.ts. That test fails onmainand 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