public inbox for [email protected]
help / color / mirror / Atom feedFrom: Amit Langote <[email protected]>
To: Kirill Reshke <[email protected]>
Cc: Tom Lane <[email protected]>
Cc: Tender Wang <[email protected]>
Cc: jian he <[email protected]>
Cc: [email protected]
Cc: [email protected]
Subject: Re: BUG #19099: Conditional DELETE from partitioned table with non-updatable partition raises internal error
Date: Thu, 30 Oct 2025 17:29:37 +0900
Message-ID: <CA+HiwqFcejrmS_H8YB-AMB7sujB7wdJXFPdAVfDC6-19FXUjgg@mail.gmail.com> (raw)
In-Reply-To: <CALdSSPi9n2KGzKQn2Egqz3H8Nx0cgnZ8UeB5gk-KVdE3uBCj6Q@mail.gmail.com>
References: <[email protected]>
<CACJufxF9FcuYe8XOuWLgWK77HCUHpOc6+7+NkktFFNmzw15jKg@mail.gmail.com>
<CAHewXN=vF5d9O4R3+iUwLqEaP7pb8iYAN_e3vEE_p5sJHofn7w@mail.gmail.com>
<[email protected]>
<CALdSSPi7udsgQg3PUG=Z4+-9pRg8wT3HkDvTgYvtg30xNWQ9OA@mail.gmail.com>
<CALdSSPi9n2KGzKQn2Egqz3H8Nx0cgnZ8UeB5gk-KVdE3uBCj6Q@mail.gmail.com>
Hi,
On Thu, Oct 30, 2025 at 3:44 PM Kirill Reshke <[email protected]> wrote:
> On Thu, 30 Oct 2025 at 10:31, I wrote:
> >
> > I mean, we I believe we need to execute
> > CheckValidResultRel against all partitions in ExecInitModifyTable, at
> > least when no partition pruning has been performed
> >
>
> So, the problem is that we managed to exclude all child relations, and
> only have a single (dummy) root relation as a result of the
> modifyTable plan. Maybe we should populate its target list with
> pseudo-junk columns in create_modifytable_plan ?
>
> For instance, they query does not error-out if we have at least one
> another non-file-fdw partition:
>
> create table p2 partition of pt for values in ( 2) ;
>
> this is because we have this in create_modifytable_plan
>
> ```
> /* Transfer resname/resjunk labeling, too, to keep executor happy */
> apply_tlist_labeling(subplan->targetlist, root->processed_tlist);
> ```
>
> and we successfully found a junk column in the p2 partition.
>
> The problem is, it works iff root->processed_tlist has at least one
> relation which can give us junk columns. Should we add handling for
> corner case here?
> Another option is to remove this 'Transfer resname/resjunk labeling'
> completely and rework planner-executer contracts somehow.
I am not really sure if we should play with the planner code.
I suspect the real issue is that we’re assuming partitioned tables
always need a ctid, which wasn’t true before MERGE started using the
root ResultRelInfo. In fact, the old code already looked wrong -- it’s
been requiring a ctid even for partitioned tables where that was never
necessary. We can fix this by only requiring the junk ctid when we
actually operate through the root partitioned table, that is, for
MERGE. Like the attached.
--
Thanks, Amit Langote
Attachments:
[application/octet-stream] partitioned-table-ctid-check.diff (1.6K, 2-partitioned-table-ctid-check.diff)
download | inline diff:
diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
index 4c5647ac38a..78394be67ea 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -4348,8 +4348,13 @@ ExecModifyTable(PlanState *pstate)
relkind == RELKIND_MATVIEW ||
relkind == RELKIND_PARTITIONED_TABLE)
{
- /* ri_RowIdAttNo refers to a ctid attribute */
- Assert(AttributeNumberIsValid(resultRelInfo->ri_RowIdAttNo));
+ /*
+ * ri_RowIdAttNo refers to a ctid attribute. It may be missing
+ * only for non-MERGE operations on partitioned tables.
+ */
+ Assert(AttributeNumberIsValid(resultRelInfo->ri_RowIdAttNo) ||
+ (relkind == RELKIND_PARTITIONED_TABLE &&
+ operation != CMD_MERGE));
datum = ExecGetJunkAttribute(slot,
resultRelInfo->ri_RowIdAttNo,
&isNull);
@@ -4863,7 +4868,14 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
{
resultRelInfo->ri_RowIdAttNo =
ExecFindJunkAttributeInTlist(subplan->targetlist, "ctid");
- if (!AttributeNumberIsValid(resultRelInfo->ri_RowIdAttNo))
+
+ /*
+ * For heap relations, a ctid junk attribute must be present.
+ * For partitioned tables, require it only for MERGE; allow it to be
+ * missing for non-MERGE operations.
+ */
+ if (!AttributeNumberIsValid(resultRelInfo->ri_RowIdAttNo) &&
+ (relkind != RELKIND_PARTITIONED_TABLE || operation == CMD_MERGE))
elog(ERROR, "could not find junk ctid column");
}
else if (relkind == RELKIND_FOREIGN_TABLE)
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: BUG #19099: Conditional DELETE from partitioned table with non-updatable partition raises internal error
In-Reply-To: <CA+HiwqFcejrmS_H8YB-AMB7sujB7wdJXFPdAVfDC6-19FXUjgg@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox