Skip to content

fix: strip double-quotes from extracted sequence name in SERIAL ownership detection#469

Merged
tianzhou merged 4 commits into
pgplex:mainfrom
ayusssmaan:fix/serial-owned-sequence-detection-quoted-identifiers
Jun 10, 2026
Merged

fix: strip double-quotes from extracted sequence name in SERIAL ownership detection#469
tianzhou merged 4 commits into
pgplex:mainfrom
ayusssmaan:fix/serial-owned-sequence-detection-quoted-identifiers

Conversation

@ayusssmaan

Copy link
Copy Markdown
Contributor

Problem

When a sequence has a mixed-case quoted name (e.g. "MyTable_myCol_seq"), the nextval() column default stores it as:

nextval('schema."MyTable_myCol_seq"'::regclass)

The existing two-pass REGEXP_REPLACE in getSequencesForSchema extracted the name with double-quotes still attached — "MyTable_myCol_seq" — and the col_table JOIN compared this against MyTable_myCol_seq (as stored unquoted in pg_sequences). The join always failed for such sequences, leaving OwnedByTable empty.

With OwnedByTable empty, the skip condition in diff.go never fired, so pgschema emitted a standalone CREATE SEQUENCE for sequences already handled by SERIAL, causing PostgreSQL to create a duplicate with a _seq1 suffix on every fresh database deploy.

Fix

Add a third REGEXP_REPLACE pass that strips leading/trailing double-quotes from the extracted sequence name before the JOIN. This makes the JOIN match correctly, OwnedByTable is populated, the skip condition fires, and no standalone CREATE SEQUENCE is emitted for SERIAL-backed sequences.

Testing

Verified on a schema with ~90 mixed-case SERIAL sequences across multiple tables:

  • Before: 93 spurious DROP SEQUENCE operations on every second apply
  • After: 0 sequence drops; fresh deploy is fully idempotent (second apply shows no changes)

…ship detection

When a sequence has a mixed-case quoted name, the nextval() default expression
stores it as nextval('schema."SeqName"'::regclass). The existing two-pass
REGEXP_REPLACE extracted 'SeqName' with double-quotes still present, causing
the col_table JOIN to compare '"SeqName"' against 'SeqName' (as stored in
pg_sequences) — the join always failed for such sequences.

Add a third REGEXP_REPLACE pass that strips leading/trailing double-quotes
from the extracted sequence name so the col_table JOIN matches correctly.

With this fix, OwnedByTable is correctly populated for quoted SERIAL sequences,
the skip condition fires, and pgschema no longer emits a standalone
CREATE SEQUENCE alongside the SERIAL-generated one — eliminating the _seq1
drift on fresh database deploys.
@greptile-apps

greptile-apps Bot commented Jun 10, 2026

Copy link
Copy Markdown

Greptile Summary

This PR updates sequence ownership detection for quoted SERIAL sequence defaults. The main changes are:

  • Adds another REGEXP_REPLACE pass to strip outer double quotes from extracted sequence names.
  • Applies the same SQL change to the generated Go query constant.
  • Keeps the existing fallback join from column defaults to pg_sequences.

Confidence Score: 3/5

This should be fixed before merging.

  • Some valid quoted identifiers still fail ownership detection.
  • Quoted schemas with dots are still parsed incorrectly.
  • Populated mixed-case ownership can produce invalid OWNED BY SQL in an existing-column path.

ir/queries/queries.sql.go, ir/queries/queries.sql, and internal/diff/sequence.go need follow-up.

Important Files Changed

Filename Overview
ir/queries/queries.sql Adds the source SQL quote-stripping pass for fallback sequence ownership detection.
ir/queries/queries.sql.go Carries the runtime generated query change used by inspection.

Reviews (1): Last reviewed commit: "fix: strip double-quotes from extracted ..." | Re-trigger Greptile

Comment thread ir/queries/queries.sql.go Outdated
Comment thread ir/queries/queries.sql.go Outdated
After stripping outer quotes, a sequence name like My"Seq is stored in
column_default as My""Seq (PostgreSQL SQL identifier escaping). Without
unescaping, the col_table JOIN still fails for such names.

Add REPLACE(..., '""', '"') as a final pass to unescape doubled quotes,
as suggested in code review.
The previous regex '^[^.]*\.' splits at the first dot, which breaks for
quoted schemas containing a dot (e.g. "foo.bar"). Replace with a
quote-aware alternative that matches either a full quoted identifier
followed by a dot, or an unquoted prefix followed by a dot.
@tianzhou

Copy link
Copy Markdown
Contributor

Pls add a test case to validate this fix

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Fixes SERIAL sequence ownership detection for mixed-case / quoted sequence names so pgschema doesn’t emit redundant standalone CREATE SEQUENCE statements (which can lead to _seq1 duplicates on fresh deploys).

Changes:

  • Updates GetSequencesForSchema’s nextval(...) parsing to strip schema prefixes that may be quoted and to remove surrounding double-quotes from the extracted sequence name.
  • Normalizes escaped quotes (""") to match the unquoted pg_sequences.sequencename join key.
  • Applies the same SQL change to both the source SQL and the sqlc-generated Go query constant.

Reviewed changes

Copilot reviewed 1 out of 2 changed files in this pull request and generated no comments.

File Description
ir/queries/queries.sql Adjusts the sequence-name extraction logic from column_default to correctly join mixed-case quoted sequence names to pg_sequences.
ir/queries/queries.sql.go Regenerates/updates the sqlc output so the runtime query matches the updated SQL source.
Files not reviewed (1)
  • ir/queries/queries.sql.go: Language not supported

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

…tection

Adds a query-level test that directly calls GetSequencesForSchema and
asserts OwnedByTable/OwnedByColumn are populated for a SERIAL column
with a mixed-case name ("orderId"). The pg_depend ownership edge is
removed via OWNED BY NONE to force the column_default parsing fallback
— the path that was broken before this fix.

Without the fix, the test fails with:
  OwnedByTable = "", want "orders"

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
@tianzhou

Copy link
Copy Markdown
Contributor

LGTM

@tianzhou tianzhou merged commit 8b7a248 into pgplex:main Jun 10, 2026
1 check 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.

3 participants