Fix expression list handling in ATExecAttachPartition()
authorAmit Langote <amitlan@postgresql.org>
Thu, 3 Oct 2024 02:59:09 +0000 (11:59 +0900)
committerAmit Langote <amitlan@postgresql.org>
Thu, 3 Oct 2024 02:59:09 +0000 (11:59 +0900)
This commit addresses two issues related to the manipulation of the
partition constraint expression list in ATExecAttachPartition().

First, the current use of list_concat() to combine the partition's
constraint (retrieved via get_qual_from_partbound()) with the parent
table’s partition constraint can lead to memory safety issues. After
calling list_concat(), the original constraint (partBoundConstraint)
might no longer be safe to access, as list_concat() may free or modify
it.

Second, there's a logical error in constructing the constraint for
validating against the default partition. The current approach
incorrectly includes a negated version of the parent table's partition
constraint, which is redundant, as it always evaluates to false for
rows in the default partition.

To resolve these issues, list_concat() is replaced with
list_concat_copy(), ensuring that partBoundConstraint remains unchanged
and can be safely reused when constructing the validation constraint
for the default partition.

This fix is not applied to back-branches, as there is no live bug and
the issue has not caused any reported problems in practice.

Nitin Jadhav posted a patch to address the memory safety issue, but I
decided to follow Alvaro Herrera's suggestion from the initial
discussion, as it allows us to fix both the memory safety and logical
issues.

Reported-by: Andres Freund <andres@anarazel.de>
Reported-by: Nitin Jadhav <nitinjadhavpostgres@gmail.com>
Reviewed-by: Junwang Zhao <zhjwpku@gmail.com>
Discussion: https://postgr.es/m/20231115165737.zeulb575cgrbqo74@awork3.anarazel.de
Discussion: https://postgr.es/m/CAMm1aWbmYHM3bqtjyMQ-a+4Ub=dgsb_2E3_up2cn=UGdHNrGTg@mail.gmail.com

src/backend/commands/tablecmds.c

index 16c86ffcc0f9b7948edb5c38973783dadd49ac2f..34f8bc801affeb87193c1123172d7e8add2ade32 100644 (file)
@@ -18688,8 +18688,14 @@ ATExecAttachPartition(List **wqueue, Relation rel, PartitionCmd *cmd,
     * constraint as well.
     */
    partBoundConstraint = get_qual_from_partbound(rel, cmd->bound);
-   partConstraint = list_concat(partBoundConstraint,
-                                RelationGetPartitionQual(rel));
+
+   /*
+    * Use list_concat_copy() to avoid modifying partBoundConstraint in place,
+    * since it’s needed later to construct the constraint expression for
+    * validating against the default partition, if any.
+    */
+   partConstraint = list_concat_copy(partBoundConstraint,
+                                     RelationGetPartitionQual(rel));
 
    /* Skip validation if there are no constraints to validate. */
    if (partConstraint)