Support EXCEPT for ALL SEQUENCES publications

First seen: 2026-04-10 06:08:44+00:00 · Messages: 83 · Participants: 7

Latest Update

2026-06-04 · claude-opus-4-6

Round Update: Peter Smith Reviews v8 Patches; Nisha Posts v9; Zsolt Parragi Finds Two Bugs in Schema EXCEPT Validation

Key Technical Activity

This round contains Peter Smith's review of Nisha's v8-0003/0004 and Shlok's v8-0002, Nisha's responses and v9 posting, the decision to drop the tab-completion top-up patch, and two new bugs discovered by Zsolt Parragi in schema-level EXCEPT validation.

Bug Discovery: `pg_get_publication_tables()` Ignores EXCEPT (Zsolt Parragi)

Zsolt demonstrated that pg_get_publication_tables() does not respect EXCEPT exclusions:

CREATE PUBLICATION p FOR TABLES IN SCHEMA s EXCEPT (TABLE s.b);
SELECT * FROM pg_get_publication_tables(ARRAY['p'], 's.b'::regclass);
-- Returns one row for s.b (should return empty)

This is a significant correctness issue — the system function used to inspect publication membership does not honor EXCEPT filtering.

Bug Discovery: CURRENT_SCHEMA Bypasses EXCEPT Validation (Zsolt Parragi)

Zsolt found that the validation checking whether an EXCEPT table belongs to the specified schema is not applied when CURRENT_SCHEMA is used:

SET search_path = s1, public;
CREATE PUBLICATION pc FOR TABLES IN SCHEMA CURRENT_SCHEMA EXCEPT (TABLE public.x);
-- Should error (public.x not in s1) but succeeds
CREATE PUBLICATION pc4 FOR TABLES IN SCHEMA CURRENT_SCHEMA EXCEPT (TABLE onlypub);
-- Should error (onlypub resolves to public.onlypub, not in s1) but succeeds

The explicit schema name path correctly rejects cross-schema EXCEPT entries, but the CURRENT_SCHEMA resolution path skips this check. This is a validation gap in the schema-matching logic.

Tab-Completion Patch (v8-0002) Dropped

Peter expressed doubt about the value of the tab-completion improvements in patch 0002, noting that many of the broken scenarios are pre-existing HEAD behavior. Nisha confirmed via testing that only the EXCEPT (TABLE suggestion is new; all other tab-completion limitations already exist. She agreed to drop patch 0002 entirely, simplifying the patch set.

Peter Smith's Review of Shlok's v8-0002

Key points:

Documentation example ordering: The "Replace EXCEPT clause" example should come after the "SET ALL SEQUENCES" example, since a reader hasn't seen sequences being published yet
SGML markup: EXCEPT, ALL SEQUENCES etc. should use <literal> tags in example descriptions
GetIncludedPublicationRelations comment inconsistency: The function comment says "only for FOR TABLE publications" but it's intentionally called for ALL SEQUENCES publications too — if no Assert is added, the comment must be fixed
get_delete_rels comment rewrite: "Recreate" is misleading; the function returns the list of relations to be dropped during ALTER SET by comparing desired state against current state
AlterPublicationRelations comment simplification: Suggested more concise wording

Peter Smith's Review of Nisha's v8-0003 and v8-0004

Relatively minor issues:

Comment wording for DROP EXCEPT semantics: Peter's suggested rewording clarifies that "DROP schema with EXCEPT" is not supported (rather than the original phrasing which he found backwards)
pg_dump test technique: Suggested using /xms modifier with combined regex for multi-statement verification instead of separate tests
Code style: Unnecessary block braces, continue vs positive condition check preference
Test query fix: SELECT count(*) instead of SELECT a to guarantee single-row results for TAP test comparisons

Nisha's v9 Response

Nisha addressed all of Peter's v8-0003/0004 comments and posted v9. The patch set is now reduced to fewer patches (tab-completion top-up dropped). She also fixed a similar continue vs positive-condition pattern in the existing PublicationDropSchemas() function that she noticed while fixing the reported instance.

History (2 prior analyses)

2026-06-01 · claude-opus-4-6

Monthly Summary: Support EXCEPT for ALL SEQUENCES / TABLES IN SCHEMA Publications — May 2026

Overview

This thread tracks two parallel extensions to PostgreSQL's logical replication publication EXCEPT mechanism:

Shlok Kyal: FOR ALL SEQUENCES EXCEPT (SEQUENCE s1, s2, ...)
Nisha Moond: FOR TABLES IN SCHEMA ... EXCEPT (TABLE t1, t2, ...)

May 2026 saw the consolidation of key architectural decisions from prior months, followed by intensive review cycles (primarily from Peter Smith) that surfaced correctness issues, naming inconsistencies, and test gaps. The month's trajectory was: two rebase-only updates from Shlok, then deep review engagement on both patch sets, culminating in v6 postings from both authors.

Key Architectural Decisions (Confirmed This Month)

1. Catalog Reuse: `pg_publication_rel` for Both Tables and Sequences

The decision to store sequence exclusions in the existing pg_publication_rel catalog (rather than creating pg_publication_seq) was reaffirmed. The prattrs/prquals columns will be NULL for RELKIND_SEQUENCE rows. Rationale: the catalog name refers to "relations" not "tables"; relkind checks are cache-backed and cheap; a parallel catalog would duplicate extensive infrastructure.

2. EXCEPT Syntax: Per-Schema Adjacency, Mandatory Keywords

EXCEPT (TABLE t1, t2) and EXCEPT (SEQUENCE s1, s2) — keywords mandatory for forward compatibility
EXCEPT binds to each schema individually (not trailing), avoiding resolution ambiguity with multi-schema publications

3. Patch Structure: Shlok Merges 0001/0002

Peter Smith argued that separating mechanical identifier renames (0001) from functional changes (0002) was counterproductive because the renames are only comprehensible in context of the functionality. Shlok accepted and merged into a single patch in v6.

Major Review Cycles

Peter Smith's Review of Nisha's v5-0001 (28 points)

Key issues:

Grammar validation strategy: EXCEPT inside table-continuation rules should be parsed always but validated semantically later (raising explicit errors), not silently ignored via grammar-branch placement
GetTopMostAncestorInPublication encapsulation: EXCEPT filtering should be inside this function rather than requiring every caller to post-filter
Cross-patch ordering: ALTER PUBLICATION logic and assertions placed in wrong patches
Tab-completion bugs: Offering all tables instead of schema-filtered tables after EXCEPT (
5 missing test scenarios identified (conflict with FOR TABLE, non-existing tables, partitions, multiple schemas, keyword omission)

Nisha's v6 Response

Notable technical clarifications:

Grammar design is intentional "parse everything, validate semantically" pattern — prevents silent data loss
list_free(schemaRels) safety depends on List cell ownership: EXCEPT path copies Oids (safe to free), non-EXCEPT path uses list_concat transferring cells (unsafe)
Uses quote_qualified_identifier() instead of RelationGetQualifiedRelationName() in grammar contexts where no relation is open

Peter Smith's Review of Nisha's v6-0001 and v6-0002

New concerns:

Performance: EXCEPT check should short-circuit before more expensive schema-publication lookup
Naming chaos: Inconsistent variable names across subsystems (except_pubids, exceptlist, exceptrelations, exceptrels, etc.)
ALTER SET + EXCEPT: Error message should indicate "not yet implemented" with errhint for workaround, not imply undefined semantics
Function naming: AlterPublicationExceptTables should reflect schema-specific scope
11 additional missing test cases across v6-0001 and v6-0002

Peter Smith's Review of Nisha's v6-0003

18 comments focusing on documentation redundancy, foreach_ptr() modernization, TAP test fragility (running totals vs. predicate-based verification), and the persistent tab-completion schema-filtering issue.

Peter Smith's Review of Shlok's v5-0002

Minor issues: doc wording, comment completeness, and asymmetric relation name qualification between EXCEPT and non-EXCEPT error paths (Shlok correctly scoped this as belonging to the parent thread).

Unresolved / Deferred Items

Tab-completion schema filtering: After EXCEPT (TABLE ..., completion should offer only tables from the relevant schema — flagged repeatedly but not yet fixed
Dedicated documentation chapter: "Excluding objects from the Publication" deferred until more EXCEPT variants ship
Future FOR SEQUENCE <name>: Explicitly anticipated but not implemented
Combined FOR TABLES, SEQUENCES IN SCHEMA: Requires broader grammar rework
FOR ALL TABLES EXCEPT documentation gap: Not mentioned in TABLES IN SCHEMA EXCEPT docs

Patch Status at Month End

Shlok: v6 posted (merged rename+functionality into single patch)
Nisha: v6 posted (3 patches: CREATE, ALTER ADD/DROP, ALTER SET), undergoing continued review

2026-06-01 · claude-opus-4-6

Round Update: Peter Smith Reviews Shlok's v6-0002 and Nisha's v7; Zsolt Parragi Joins; Bug Found in DROP Cascade

This round contains substantive technical activity across both patch threads, including a new reviewer, a potential bug discovery, and continued refinement of naming conventions.

New Reviewer: Zsolt Parragi (Percona)

Zsolt provided three targeted observations on Shlok's patch:

Memory leak in footers array: The footers[2] assignment path in \dRp+ display code lacks a corresponding free() call — a genuine resource leak when publications have all three footer types (tables, sequences, except entries).
Missing pr.prexcept check in psql query: A NOT EXISTS subquery filtering publications that publish a sequence doesn't include pr.prexcept in its WHERE clause. Shlok's response is technically interesting — he argues the check is unnecessary because pg_publication_rel only contains entries for ALL TABLES/ALL SEQUENCES publications when they appear in an EXCEPT clause, so the NOT EXISTS logic already produces correct results without the explicit flag check. This is a subtle semantic argument about the catalog's invariants.
Removed Assert should be kept in weaker form: Zsolt suggested keeping a restricted assert (EXCEPT only valid for SET actions) rather than removing the assertion entirely. Shlok added the Assert in the v8 0002 patch.

Bug Discovery: DROP TABLES IN SCHEMA Not Cascading to EXCEPT Entries (v7-0002)

Peter Smith identified what appears to be a bug in Nisha's v7-0002 patch through careful test output analysis:

ALTER PUBLICATION testpub_alter_except DROP TABLES IN SCHEMA pub_test;
\dRp+ testpub_alter_except
-- Still shows: Except tables: "pub_test.testpub_tbl_s1"

After DROP TABLES IN SCHEMA, the EXCEPT entries for that schema are not being removed. This manifests in three consecutive test cases where stale EXCEPT entries persist and cause cascading failures (e.g., a subsequent ADD with EXCEPT fails because the old entry still exists).

Nisha's response: she acknowledged this is a test-placement issue — the DROP cascade logic lives in Patch-0003, so tests exercising it in Patch-0002 produce incorrect results. She restructured the tests to avoid depending on functionality from later patches. However, this raises the recurring cross-patch correctness issue: Patch-0002 alone leaves the system in a state where DROP doesn't clean up EXCEPT entries, which is a user-visible bug in the intermediate state.

Naming Convention Debate: `except_rel_names` Rejected

Peter challenged Nisha's chosen variable name except_rel_names (introduced in v7 per his own earlier suggestion for consistent except_ prefix). His argument: these are PublicationTable objects, not name strings. The name implies List * of char * when it's actually structured objects. Nisha accepted and renamed to except_pubtables.

`GetTopMostAncestorInPublication` Optimization (Nisha v7-0001)

Peter identified a loop-invariant redundancy: list_member_oid(except_pubids, puboid) is evaluated every iteration but neither value changes within the loop. Nisha moved it to a pre-loop boolean variable — a minor but correct optimization for partition hierarchies with many ancestors.

Tab-Completion Limitations Acknowledged

Both sides agreed that correct tab-completion after commas in EXCEPT lists is effectively unsolvable with the current tab-complete.in.c architecture (the relative word offset shifts unpredictably). Nisha's pragmatic solution: suppress suggestions entirely after a comma rather than showing incorrect ones. Peter agreed this is the right tradeoff.

Peter also noted that multi-schema tab-completion (FOR TABLES IN SCHEMA public, myschema <TAB>) doesn't work at all currently. Nisha created a separate top-up patch (v8-0002) to fix this as an independent improvement rather than mixing it into the EXCEPT feature.

Shlok's v7 and v8 Patch Progression

Shlok posted v7 addressing Peter's v6-0002 review, then v8 addressing Zsolt's comments. Key changes:

Added sanity Assert in get_publication_relations for valid pubrelkind values
Explained why GetIncludedPublicationRelations should NOT assert !allsequences — the function is called for ALL SEQUENCES publications (by pg_get_publication_tables, InvalidatePubRelSyncCache), it just returns an empty list. This was previously discussed and intentionally left as-is.
Added Assert that EXCEPT clause is only used with SET action in ALTER PUBLICATION
Fixed footers memory leak

Documentation Readability Concern Deferred

Peter flagged that the ALTER PUBLICATION documentation for SET ALL TABLES/SEQUENCES EXCEPT is becoming repetitive and hard to read, but explicitly scoped this as out-of-bounds — to be addressed holistically after both Shlok's and Nisha's patches are combined.