Hi hackers,

Was trying to understand that negated operators thread and found what I
think is a bug in ATTACH PARTITION constraint validation.

Reproducer (master at 6a2edf2a1e6, also verified on 11-19):

    CREATE TABLE lp (a int) PARTITION BY LIST (a);
    CREATE TABLE

    CREATE TABLE lp_def PARTITION OF lp DEFAULT;
    CREATE TABLE

    INSERT INTO lp VALUES (1);
    INSERT 0 1

    CREATE TABLE lp_1 (a int);
    CREATE TABLE

    -- This is correctly rejected
    ALTER TABLE lp ATTACH PARTITION lp_1 FOR VALUES IN (1);
    psql:test3.sql:6: ERROR:  updated partition constraint for default
partition "lp_def" would be violated by some row

    -- This should also be rejected
    ALTER TABLE lp ATTACH PARTITION lp_1 FOR VALUES IN (1, NULL);
    ALTER TABLE

    SELECT tableoid::regclass, a FROM lp;
    tableoid | a
    ----------+---
    lp_def   | 1
    (1 row)

    -- a=1 shows up
    SELECT * FROM lp;
    a
    ---
    1
    (1 row)

    -- a=1 missing
    SELECT * FROM lp WHERE a = 1;
    a
    ---
    (0 rows)

    SELECT pg_get_partition_constraintdef('lp_def'::regclass);
    pg_get_partition_constraintdef
    --------------------------------
    (NOT ((a IS NULL) OR (a = 1)))
    (1 row)

QueuePartitionConstraintValidation() in tablecmds.c stores only the first
element of the implicit-AND list it is handed.  ATExecAttachPartition()
works around that for the partition being attached by wrapping its
constraint in a one-element list, but it passes the default partition's
proposed constraint through as is.

When the new bound is a LIST bound that includes NULL, the negation of
the bound has two conjuncts, so only the IS NOT NULL test is ever
checked.  The ATTACH then succeeds with rows left in the default
partition that violate its updated constraint.  Pruning trusts that
constraint, so any query whose qual lets it exclude the default
partition skips those rows.

The original workaround carries an "XXX this sure looks wrong"
comment, which turns out to have been prescient.

The attached patch stores make_ands_explicit() of the list instead and
drops the now unnecessary wrapper.  Also includes a regression test.

Regards,
-- Sehrope Sarkuni
Founder & CEO | JackDB, Inc. | https://www.jackdb.com/
From a32a376bfe2a68233dbe5769dbfe525e595289c4 Mon Sep 17 00:00:00 2001
From: Sehrope Sarkuni <[email protected]>
Date: Mon, 5 Oct 2026 20:23:07 +0000
Subject: [PATCH v1] Validate the whole default partition constraint on ATTACH
 PARTITION

QueuePartitionConstraintValidation() stored only the first element of
the implicit-AND list passed to it.  ATExecAttachPartition() avoided
that for the partition being attached by wrapping its constraint in a
one-element list, but passed the default partition's proposed
constraint as is.  When the new bound is a LIST bound that includes
NULL, the negated constraint is AND(key IS NOT NULL, key <> ALL(...)),
so only the IS NOT NULL test was checked and the ATTACH succeeded with
rows left in the default partition that violate its constraint and
that partition pruning no longer returns.

Store make_ands_explicit() of the list instead, and drop the wrapper.
---
 src/backend/commands/tablecmds.c          | 5 +----
 src/test/regress/expected/alter_table.out | 3 +++
 src/test/regress/sql/alter_table.sql      | 2 ++
 3 files changed, 6 insertions(+), 4 deletions(-)

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 0274d892f2e..6419c7f6934 100644
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -20922,7 +20922,7 @@ QueuePartitionConstraintValidation(List **wqueue, Relation scanrel,
 		/* Grab a work queue entry. */
 		tab = ATGetQueueEntry(wqueue, scanrel);
 		Assert(tab->partition_constraint == NULL);
-		tab->partition_constraint = (Expr *) linitial(partConstraint);
+		tab->partition_constraint = make_ands_explicit(partConstraint);
 		tab->validate_default = validate_default;
 	}
 	else if (scanrel->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
@@ -21240,9 +21240,6 @@ ATExecAttachPartition(List **wqueue, Relation rel, PartitionCmd *cmd,
 			(List *) eval_const_expressions(NULL,
 											(Node *) partConstraint);
 
-		/* XXX this sure looks wrong */
-		partConstraint = list_make1(make_ands_explicit(partConstraint));
-
 		/*
 		 * Adjust the generated constraint to match this partition's attribute
 		 * numbers.
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index e167a41ce79..9c92cd17af6 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -4208,6 +4208,9 @@ INSERT INTO list_parted2_def VALUES (11, 'z');
 CREATE TABLE part_3 (LIKE list_parted2);
 ALTER TABLE list_parted2 ATTACH PARTITION part_3 FOR VALUES IN (11);
 ERROR:  updated partition constraint for default partition "list_parted2_def" would be violated by some row
+-- same, with a bound whose negation has more than one conjunct
+ALTER TABLE list_parted2 ATTACH PARTITION part_3 FOR VALUES IN (11, NULL);
+ERROR:  updated partition constraint for default partition "list_parted2_def" would be violated by some row
 -- should be ok after deleting the bad row
 DELETE FROM list_parted2_def WHERE a = 11;
 ALTER TABLE list_parted2 ATTACH PARTITION part_3 FOR VALUES IN (11);
diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql
index 9f6c2a4bb08..73393da6304 100644
--- a/src/test/regress/sql/alter_table.sql
+++ b/src/test/regress/sql/alter_table.sql
@@ -2568,6 +2568,8 @@ CREATE TABLE list_parted2_def PARTITION OF list_parted2 DEFAULT;
 INSERT INTO list_parted2_def VALUES (11, 'z');
 CREATE TABLE part_3 (LIKE list_parted2);
 ALTER TABLE list_parted2 ATTACH PARTITION part_3 FOR VALUES IN (11);
+-- same, with a bound whose negation has more than one conjunct
+ALTER TABLE list_parted2 ATTACH PARTITION part_3 FOR VALUES IN (11, NULL);
 -- should be ok after deleting the bad row
 DELETE FROM list_parted2_def WHERE a = 11;
 ALTER TABLE list_parted2 ATTACH PARTITION part_3 FOR VALUES IN (11);
-- 
2.17.1

Reply via email to