fix(deparser): PG18 regress round-trip sweep fixes - #354
Conversation
Window frame bitmask decoding, publication/subscription stmts, CTE SEARCH/CYCLE, INSERT OVERRIDING, COPY options, GRANT/REVOKE role options + GRANTED BY, identity column alterations, STORAGE/COMPRESSION, PARTITION OF column options, hash MODULUS/REMAINDER 0, CTAS WITH placement, SQL/JSON constructors/aggregates, multi-column MERGE UPDATE, deferred EXCLUDE, DEFAULT b_expr parens, COMMENT ON CONSTRAINT ... ON DOMAIN.
🤖 Devin AI EngineerI'll be helping with this pull request! Here's what you should know: ✅ I will automatically:
Note: I can only respond to comments from users who have write access to this repository. ⚙️ Control Options:
|
|
Review complete. 🟠 1 high 💬 Inline comments (1)
🧹 Nitpicks (1) — 🟢 1 low
This PR expands kitchen-sink coverage to PostgreSQL 18 syntax and extends the deparser and type-inference metadata to support it. It adds nine new PG18 SQL fixtures (pg18-copy, cte-insert, grant-role, identity-storage, misc-ddl, partition-ctas, publication-subscription, sqljson, window-frames) with matching round-trip test files, updates the generated fixture JSON, and adds the corresponding deparser handlers (new SQL/JSON constructors and IS JSON predicates, GRANTED BY in GRANT/REVOKE, COPY options, CTE SEARCH/CYCLE, publication/subscription options) plus pgsql-types field-metadata tags.
Reviewed commit: 20649be |
![tenki-reviewer[bot]](https://avatars.githubusercontent.com/in/2900641?s=60&v=4){kind=small}
There was a problem hiding this comment.
Adds PG18 kitchen-sink fixtures (SQL/JSON, COPY, CTE INSERT, GRANT/REVOKE, publication/subscription, DDL) plus the deparser changes and pgsql-types metadata needed to round-trip them.
Key findings
- 🟠 GRANTED BY grantor deparses as PUBLIC — deparser.ts:8837
1 participant
Summary
Ran every statement from the PG18
src/test/regress/sqlsuite throughparse → deparse → reparseand fixed everything the TS deparser got wrong (~9.1k parseable statements: 762 failures → 0 hard failures; 7 remaining areCOPY ... FREEZEboolean-shape diffs that are semantically identical). Follow-up to #292/#346/#348/#349/#350 — same class of bugs, found by sweeping instead of waiting for reports.Fixes, grouped by root cause:
WindowDef.frameOptionsis a bitmask, not an enum (silent corruption — round-tripped but changed semantics)FRAMEOPTION_*flags; only emit a frame whenFRAMEOPTION_NONDEFAULTis set (previously default frames rendered asRANGE UNBOUNDED PRECEDING, andROWS BETWEEN 2 PRECEDING AND 1 FOLLOWINGlost its bounds/EXCLUDE).OVER w,OVER (w ORDER BY ...)andWINDOW w AS (...)preserved;FuncCall/JsonObjectAgg/JsonArrayAggshareformatOverClause.Protobuf zero-default omission
MODULUS n, REMAINDER 0→remainder ?? 0in bothCreateStmt.partboundandPartitionBoundSpec.Unhandled / mis-rendered statements (threw or produced unparseable SQL)
AlterPublicationStmt:ADD|DROP|SET TABLE ...,TABLES IN SCHEMA,SET (opts);PublicationObjSpecpredicates wrappedWHERE (expr);CreatePublicationStmtemitsFOR.AlterSubscriptionStmt: allkinds (CONNECTION,SET/ADD/DROP PUBLICATION ... WITH,REFRESH PUBLICATION,ENABLE/DISABLE,SKIP).DefElemargs of typeTypeName/Integer/Float/Booleanno longer stringified/quoted (publish_via_partition_root = 1,streaming = parallel).CommonTableExpr.search_clause/cycle_clause(SEARCH DEPTH|BREADTH FIRST BY ... SET,CYCLE ... SET ... TO x DEFAULT y USING ...).InsertStmt.override→OVERRIDING {USER|SYSTEM} VALUE(also in MERGE INSERT actions).AT_SetIdentity/AT_DropIdentity:SET GENERATED ALWAYS|BY DEFAULT,SET INCREMENT BY ..,RESTART [WITH n],DROP IDENTITY [IF EXISTS](wasSET generated = 97,DROP IDENTITY RESTRICT).ALTER SEQUENCE ... OWNED BY NONE(wasOWNED BY "none").ColumnDef.storage_name/.compression→STORAGE x/COMPRESSION y;AT_AlterColumnTypekeepscollClause.CREATE TABLE ... PARTITION OF parent (col WITH OPTIONS ...) FOR VALUES ...— column list now emitted afterPARTITION OF, not before.CREATE TABLE AS:WITH (...)moved beforeAS; reloption names quoted viaQuoteUtils.quoteIdentifier("Fillfactor").CopyStmt:TO STDOUTvsFROM STDIN(was alwaysSTDIN); options rendered asFORMAT csv,DELIMITER ',',FORCE_QUOTE (a, b), bareFREEZE/HEADER.GrantRoleStmtrewritten:WITH ADMIN|INHERIT|SET TRUE/FALSE,REVOKE ADMIN OPTION FOR,GRANTED BY,CASCADE;GrantStmt.grantor→GRANTED BY.JsonObjectConstructor,JsonArrayConstructor,JsonArrayQueryConstructor,JsonObjectAgg,JsonArrayAgg,JsonParseExpr,JsonScalarExpr,JsonSerializeExpr,JsonIsPredicate,JsonKeyValue,JsonValueExpr(withFORMAT JSON [ENCODING],RETURNING type,ABSENT|NULL ON NULL,WITH|WITHOUT UNIQUE KEYS).MergeWhenClauseUPDATE uses sharedformatUpdateAssignments→SET (b, c) = (SELECT ...)(wasb = (...)[1], c = (...)[2]). Assignments now joined with,instead of,(one plpgsql-deparser snapshot updated).CONSTR_EXCLUSIONdeferrability;INITIALLY DEFERREDwithoutDEFERRABLE.CONSTR_DEFAULTwraps non-b_exprexpressions in parens (DEFAULT (1 IN (1, 2)),DEFAULT (a AND b)).COMMENT ON CONSTRAINT c ON DOMAIN d.Tests
__fixtures__/kitchen-sink/misc/pg18-*.sql(~230 statements), regenerated vianpm run kitchen-sink.pnpm build,pnpm lint, and all package test suites pass locally (pnpm -r testfirst-fail abort was only the updated snapshot).Link to Devin session: https://app.devin.ai/sessions/9ac99c3b56114e9ca60e6d984ee9c11a
Open in Devin Desktop: https://app.devin.ai/desktop/session/9ac99c3b56114e9ca60e6d984ee9c11a?variant=devin
Requested by: @pyramation