pgsql: Fix expression list handling in ATExecAttachPartition()

Started by Amit Langoteover 1 year ago5 messagescomitters
Jump to latest
#1Amit Langote
Langote_Amit_f8@lab.ntt.co.jp

Fix expression list handling in ATExecAttachPartition()

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: /messages/by-id/20231115165737.zeulb575cgrbqo74@awork3.anarazel.de
Discussion: /messages/by-id/CAMm1aWbmYHM3bqtjyMQ-a+4Ub=dgsb_2E3_up2cn=UGdHNrGTg@mail.gmail.com

Branch
------
master

Details
-------
https://git.postgresql.org/pg/commitdiff/babb3993dbe9c805c1d29fa275a5e8f4c2b40922

Modified Files
--------------
src/backend/commands/tablecmds.c | 10 ++++++++--
1 file changed, 8 insertions(+), 2 deletions(-)

#2Alexander Korotkov
aekorotkov@gmail.com
In reply to: Amit Langote (#1)
Re: pgsql: Fix expression list handling in ATExecAttachPartition()

Hi, Amit!

On Thu, Oct 3, 2024 at 6:01 AM Amit Langote <amitlan@postgresql.org> wrote:

Fix expression list handling in ATExecAttachPartition()

+ * since it’s needed later to construct the constraint expression for

I don't think we're good about using unicode apostrophes. Could you,
please, switch to ascii?

------
Regards,
Alexander Korotkov
Supabase

#3Amit Langote
Langote_Amit_f8@lab.ntt.co.jp
In reply to: Alexander Korotkov (#2)
Re: pgsql: Fix expression list handling in ATExecAttachPartition()

On Thu, Oct 3, 2024 at 18:57 Alexander Korotkov <aekorotkov@gmail.com>
wrote:

Hi, Amit!

On Thu, Oct 3, 2024 at 6:01 AM Amit Langote <amitlan@postgresql.org>
wrote:

Fix expression list handling in ATExecAttachPartition()

+ * since it’s needed later to construct the constraint expression for

I don't think we're good about using unicode apostrophes. Could you,
please, switch to ascii?

Oh, oops, will fix. Thanks for the heads up.

Show quoted text
#4Amit Langote
Langote_Amit_f8@lab.ntt.co.jp
In reply to: Amit Langote (#3)
Re: pgsql: Fix expression list handling in ATExecAttachPartition()

On Thu, Oct 3, 2024 at 7:12 PM Amit Langote <amitlangote09@gmail.com> wrote:

On Thu, Oct 3, 2024 at 18:57 Alexander Korotkov <aekorotkov@gmail.com> wrote:

Hi, Amit!

On Thu, Oct 3, 2024 at 6:01 AM Amit Langote <amitlan@postgresql.org> wrote:

Fix expression list handling in ATExecAttachPartition()

+ * since it’s needed later to construct the constraint expression for

I don't think we're good about using unicode apostrophes. Could you, please, switch to ascii?

Oh, oops, will fix. Thanks for the heads up.

Done.

--
Thanks, Amit Langote

#5Alexander Korotkov
aekorotkov@gmail.com
In reply to: Amit Langote (#4)
Re: pgsql: Fix expression list handling in ATExecAttachPartition()

On Thu, Oct 3, 2024 at 2:02 PM Amit Langote <amitlangote09@gmail.com> wrote:

On Thu, Oct 3, 2024 at 7:12 PM Amit Langote <amitlangote09@gmail.com>
wrote:

On Thu, Oct 3, 2024 at 18:57 Alexander Korotkov <aekorotkov@gmail.com>

wrote:

Hi, Amit!

On Thu, Oct 3, 2024 at 6:01 AM Amit Langote <amitlan@postgresql.org>

wrote:

Fix expression list handling in ATExecAttachPartition()

+ * since it’s needed later to construct the constraint expression

for

I don't think we're good about using unicode apostrophes. Could you,

please, switch to ascii?

Oh, oops, will fix. Thanks for the heads up.

Done.

Great, thank you!

------
Regards,
Alexander Korotkov
Supabase