From 46ac7531201986d62f7dd6d84a304139ef6fd9d4 Mon Sep 17 00:00:00 2001 From: Amit Langote Date: Tue, 1 Oct 2024 15:03:02 +0900 Subject: [PATCH v2] Fix expression list handling in ATExecAttachPartition() MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit This commit resolves two issues related to the manipulation of 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 alter it. Second, there is a logical error when constructing the constraint expression used to validate against the default partition. The current approach mistakenly adds a negated version of the parent table’s partition constraint. This is incorrect, as the default partition’s constraint must include the parent table’s partition constraint without negation. Although this does not result in a live bug (since the negated constraint always evaluates to false and becomes redundant), it is logically wrong. To address 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 change preserves memory safety and prevents the unnecessary inclusion of redundant constraints. Nitin Jadhav also posted a patch to address the issue memory safety issue, but I decided to implement the solution suggested by Alvaro Herrera in the first discussion because it allows to address the second issue as well. Reported-by: Andres Freund Reported-by: Nitin Jadhav Reviewed-by: Junwang Zhao 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 | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 87232e0137..0e9b87d499 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -18719,8 +18719,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) -- 2.43.0