Re: [PATCH] Fix pg_dump --clean with inherited partition constraints

Hi Jian,

Thanks for the additional reproducer. I'll keep the related fixes in
this thread.

In your CHECK example, the child constraint has both a local definition
and an inherited definition. Although conislocal is true, coninhcount is
nonzero, so it cannot be dropped independently. Dropping the parent
constraint first removes the inherited contribution, but leaves the
child's local definition behind. This means that simply omitting the
child's DROP is not sufficient for every selective restore.

For your reproducer, the child's local constraint can be removed by the
subsequent table DROP. The additional dependency planning below
addresses selective restores that recreate the constraints without
dropping their tables.

I have split the current work into five patches. The first three address
specific bugs; the last two explore a broader approach to cleanup
ordering and selection, for which I would appreciate feedback on the
design.

1. Avoid dropping constraints backed by attached partition indexes

As in the original patch, dumpConstraint() checks the backing index's
parentidx before emitting DROP CONSTRAINT. Constraints backed by
attached indexes are removed by a parent constraint or table DROP;
unattached local constraints retain their DROP statements. Constraint
creation and index attachment are unchanged.

2. Avoid dropping partitioned indexes with constraint-backed descendants

This addresses the other case mentioned in my original email: a plain
partitioned index can have a descendant index owned by a local
PRIMARY KEY or UNIQUE constraint. Dropping the root index then fails
without CASCADE, even if the individual child DROPs have been
omitted.

The patch checks the full index hierarchy, including descendants on
tables excluded from the dump. A plain root index with such
descendants has its independent DROP omitted and relies on table
cleanup.

Taken on their own, these first two patches still leave limitations
for selective restores that omit the covering table or parent
operation.

3. Fix enforcement and validation of merged inherited CHECK constraints

While testing restoration of the CHECK case, I found a separate
backend issue. Adding a local enforced CHECK can merge with an
existing inherited NOT ENFORCED CHECK. The merge previously set
convalidated to true unconditionally and was omitted from the
constraints returned to the ALTER TABLE caller. Consequently,
existing rows could escape validation, and existing descendants could
remain NOT ENFORCED.

For NOT VALID, the incorrect validated state can also allow
constraint exclusion to hide violating old rows.

The patch returns this merge to the ALTER TABLE caller, preserves the
requested validity state, and arranges validation and enforcement
throughout the existing descendants. NOT VALID enables enforcement
without scanning old rows. Already enforced and validated descendants
retain their state, and inheritance counts are unchanged.

4. Represent cleanup rules separately from creation dependencies

Reversing creation order cannot express both objects that must
disappear with another object and local inherited constraints that
require parent cleanup first. Also, a table being present in the
archive does not guarantee that its DROP will be selected by
pg_restore.

This patch records whether an entry can be dropped directly, must be
removed by a covering operation, or requires other cleanup operations
first. It distinguishes alternatives, where any one covering DROP is
sufficient, from prerequisites, where every required DROP must be
included in the cleanup plan.

This replaces the dump-time DROP suppression in the first two patches
with cleanup metadata. The DROP text remains in the archive, but the
planner determines whether an independent DROP is allowed and whether
a covering action is selected.

After restore selection, the planner checks these rules and orders
the cleanup operations. Covering actions include the object's table
and droppable ancestor indexes, constraints, or tables. A selected
database DROP in --create mode also provides coverage.

Missing coverage, missing prerequisites, and cycles are rejected
before connecting to the destination or emitting restoration SQL. The
planner does not add CASCADE or expand the selection. Creation
dependencies and creation order, including that specified by
--use-list, are unchanged.

5. Order cleanup of locally defined inherited constraints

This supplies cleanup prerequisites for separately dumped local CHECK
constraints and NOT VALID NOT NULL constraints with inherited
definitions. pg_dump follows the inheritance graph to the local
parent definitions that must be removed first, traversing purely
inherited intermediates as needed. CHECKs are matched by constraint
name; NOT NULL constraints are matched by column name because names
and attribute numbers can differ between tables.

The traversal excludes NO INHERIT definitions, checks inheritance
counts, and visits shared ancestors once without C recursion.

If the constraint's table is selected for cleanup, its standalone
DROP is omitted. Otherwise, the required parent definitions are
cleaned before the remaining local definition. This fixes the cleanup
failure in your reproducer and orders cleanup for selective
constraint restores when all required parent cleanup actions are
selected.

The last two patches have compatibility and behavioral implications that
I would like to discuss before treating the design as settled.

The implementation raises the archive format to 1.17 for all new custom,
directory, and tar archives, even without --clean or inherited objects.
Readers supporting only earlier archive versions reject them. New
readers retain the existing behavior for older archives; they do not
reconstruct missing cleanup rules or repair the saved DROP statements.
Plain SQL dumps have no archive-version restriction. I would consider
the last two patches for master, rather than an ordinary backpatch.

Also, the checks use source metadata rather than the destination
catalogs. A selection missing required cleanup coverage or prerequisites
is rejected even with --if-exists, without --exit-on-error, or against
an empty destination. If a local inherited constraint needs an
independent DROP, unresolved parent prerequisites cause rejection; this
includes inline parent definitions with no separate archive entry.
Selecting the child's table DROP avoids the need for that independent
constraint DROP. The planner does not account for additional
dependencies in the destination or repair creation-order problems. These
behaviors are documented.

The pg_dump TAP suite and backend regression suite pass locally. Added
coverage includes repeated restores, selective restores, transaction
modes, multiple inheritance, and preservation of violating old rows
under NOT VALID. Backend tests cover validation failure and rollback,
descendant enforcement, differing attribute numbers, and unchanged
inheritance counts. Archive tests cover malformed cleanup metadata,
dependency cycles, and repeated restoration of synthetic directory
archives using the 1.16 layout. Rejected restores are checked for
unchanged object OIDs and data, and the metadata tests also verify that
no SQL is executed.

Would it be preferable to pursue a narrower cleanup fix that retains the
existing selective-restore limitations, or is recording separate cleanup
rules in the archive a reasonable direction? The first three fixes can
be reviewed independently of that design decision.

--
Best Regards,
Teletele