Skip to content

fix(deparser): fix EXCLUDE WHERE, TypeName quoting, VALUES formatting; fix CI + pg-proto-parser#286

Merged
pyramation merged 16 commits intomainfrom
devin/1773537015-exclude-where-clause
Mar 15, 2026
Merged

fix(deparser): fix EXCLUDE WHERE, TypeName quoting, VALUES formatting; fix CI + pg-proto-parser#286
pyramation merged 16 commits intomainfrom
devin/1773537015-exclude-where-clause

Conversation

@pyramation
Copy link
Collaborator

@pyramation pyramation commented Mar 15, 2026

fix(deparser): fix EXCLUDE WHERE clause, TypeName quoting, VALUES formatting; fix CI + pg-proto-parser

Summary

1. Deparser bug fixes (5 bugs, 30 previously-failing tests)

Fixes five bugs in packages/deparser/src/deparser.ts that caused 30 test failures across 13 test suites. All were pre-existing on main but hidden because deparser tests never ran in CI (see CI fix below).

Bug 1 — EXCLUDE WHERE clause dropped:
The CONSTR_EXCLUSION handler never output node.where_clause. Given:

EXCLUDE USING btree (database_id WITH =) WHERE (status = 'pending')

The deparser silently dropped WHERE (status = 'pending'), changing constraint semantics.

Fix: Added where_clause output after the exclusions block (~line 3065).

Bug 2 — TypeName single-name quoting missing:
Single-name types like "AlertLevel", "any", "C" were emitted unquoted, producing invalid or semantically different SQL. For example, "any" (reserved pseudo-type) became any (syntax error), and "AlertLevel" became AlertLevel (case-insensitive, wrong semantics).

Fix: Applied QuoteUtils.quoteIdentifierTypeName() to single-name types in the TypeName handler (~line 1890). This also fixes the CREATE TYPE RANGE collation quoting issue (collation="C"collation = "C").

Bug 3 — Pretty VALUES tuple formatting:
Pretty-printed VALUES tuples were expanded to multi-line format even for single/short values:

-- Was producing:
VALUES (
  'John Doe'
)
-- Should produce:
VALUES
  ('John Doe')

Fix: Reverted to inline tuple formatting using context.parens(values.join(', ')) (~line 564-566).

Related issue: #287

2. Fix CI test workflow

  • Broken filter names: pnpm --filter matches package.json name fields, but the matrix used directory names. 7 of 9 packages silently skipped all tests. Only plpgsql-deparser and plpgsql-parser were ever tested.
  • Duplicate workflow runs: Both push and pull_request events fired for every PR commit. Fixed by restricting push to main and adding a concurrency group.
  • Added fail-fast: false so one package failure doesn't cancel other jobs.
  • Added pg-proto-parser back to the matrix (has 9 test files; was previously removed).

Final CI matrix (7 packages):

Package Filter name
pgsql-deparser pgsql-deparser
pgsql-parser pgsql-parser
plpgsql-deparser plpgsql-deparser
plpgsql-parser plpgsql-parser
pg-proto-parser pg-proto-parser
@pgsql/quotes @pgsql/quotes
@pgsql/transform @pgsql/transform

3. Fix pg-proto-parser test failures

  • Outdated snapshot header: meta.test.ts.snap had an old goo.gl/fbAQLP link that Jest 30 rejects. Updated to https://jestjs.io/docs/snapshot-testing.
  • Missing @pgsql/types devDependency: The test file imports SelectStmt from @pgsql/types but the package didn't declare it as a dependency. Added @pgsql/types@^17.6.2 to devDependencies.

All 9 pg-proto-parser test suites now pass (42 tests, 36 snapshots).

4. Test fixture additions

Added failing test case for EXCLUDE WHERE bug (misc/issues-18.sql) and regenerated test scaffolding.

Review & Testing Checklist for Human

  • Verify TypeName quoting doesn't over-quote — The fix applies quoteIdentifierTypeName() to ALL single-name types. This should only quote RESERVED_KEYWORD and lexically-unsafe identifiers (uppercase, special chars), but verify common types like text, integer, uuid, json remain unquoted in deparsed output.
  • Confirm VALUES formatting revert is desired — The code went from multi-line expansion back to inline tuples. This matches existing snapshots, but verify this is the intended pretty-print format (not a regression).
  • Spot-check deparser output — Run a few manual parse → deparse cycles on SQL with EXCLUDE WHERE, quoted type names, and VALUES clauses to verify correctness.

Notes

  • All 7 CI checks pass (7/7 green): pgsql-deparser, pgsql-parser, plpgsql-deparser, plpgsql-parser, pg-proto-parser, @pgsql/quotes, @pgsql/transform
  • All 290 deparser test suites now pass (710/710 tests, 424/424 snapshots). The 30 previously-failing tests are now fixed.
  • The CI changes surface tests that were never running before. pg-proto-parser had a pre-existing failure that is now fixed.
  • Discovered while working on constructive-db's database_transfer table, which uses EXCLUDE USING btree (database_id WITH =) WHERE (status = 'pending'). The pgpm package bundled SQL was missing the WHERE clause.

Link to Devin run: https://app.devin.ai/sessions/3f7a5f172eff49b388c44b148691911c
Requested by: @pyramation

The deparser drops the WHERE clause from EXCLUDE USING ... WHERE (...) constraints.
The parser correctly preserves where_clause in the AST, but the CONSTR_EXCLUSION
handler in the deparser never outputs it.

This causes partial exclusion constraints to lose their predicate during
parse -> deparse cycles, which silently changes the constraint semantics.
@devin-ai-integration
Copy link
Contributor

🤖 Devin AI Engineer

I'll be helping with this pull request! Here's what you should know:

✅ I will automatically:

  • Address comments on this PR. Add '(aside)' to your comment to have me ignore it.
  • Look at CI failures and help fix them

Note: I can only respond to comments from users who have write access to this repository.

⚙️ Control Options:

  • Disable automatic comment and CI monitoring

The pnpm --filter values must match the 'name' field in each
package's package.json, not the directory name. Most packages
were silently skipped because the filter matched nothing.

Fixes:
- deparser -> pgsql-deparser
- parser -> pgsql-parser
- pgsql-cli -> @pgsql/cli
- proto-parser -> pg-proto-parser
- transform -> @pgsql/transform
- traverse -> @pgsql/traverse
- utils -> @pgsql/utils
@devin-ai-integration devin-ai-integration bot changed the title test: add failing test for EXCLUDE constraint WHERE clause being dropped test: add failing test for EXCLUDE WHERE clause bug; fix CI test filters Mar 15, 2026
- Remove packages without test files: @pgsql/cli, @pgsql/traverse, @pgsql/utils
- Add packages with tests that were missing: @pgsql/quotes
- Add concurrency group to prevent push/pull_request runs from cancelling each other
- All filter names now match package.json 'name' fields
…xisting failure)

- push trigger now only fires on main branch (avoids duplicate runs on PR branches)
- Removed pg-proto-parser from matrix (meta.test.ts has pre-existing failure)
- pull_request + workflow_dispatch still trigger for all branches
- EXCLUDE WHERE clause: add where_clause output to CONSTR_EXCLUSION handler
- TypeName quoting: apply quoteIdentifierTypeName for single-name types (fixes "any", "AlertLevel", "C")
- Pretty VALUES formatting: keep tuples inline instead of expanding each value to its own line
- CREATE TYPE RANGE collation: fixed via TypeName quoting ("C" now properly quoted)
- CREATE AGGREGATE "any": fixed via TypeName quoting (reserved pseudo-type now quoted)
@devin-ai-integration devin-ai-integration bot changed the title test: add failing test for EXCLUDE WHERE clause bug; fix CI test filters fix(deparser): fix EXCLUDE WHERE clause, TypeName quoting, VALUES formatting; fix CI Mar 15, 2026
@devin-ai-integration devin-ai-integration bot changed the title fix(deparser): fix EXCLUDE WHERE clause, TypeName quoting, VALUES formatting; fix CI fix(deparser): fix EXCLUDE WHERE, TypeName quoting, VALUES formatting; fix CI + pg-proto-parser Mar 15, 2026
@pyramation pyramation merged commit 68b1381 into main Mar 15, 2026
7 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