public inbox for [email protected]
help / color / mirror / Atom feedFrom: Amit Langote <[email protected]>
To: Kirill Reshke <[email protected]>
Cc: David Rowley <[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: Fri, 7 Nov 2025 18:23:16 +0900
Message-ID: <CA+HiwqHe+qQ9BY3RSOqCN50bwWGve-QCoRJaDwhzpy_B91BC6A@mail.gmail.com> (raw)
In-Reply-To: <CALdSSPj_U4DAv4xZPPcUr8Gd9KGZh0g0Uf5TWjc=DuW2rjnSjw@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>
<CA+HiwqFcejrmS_H8YB-AMB7sujB7wdJXFPdAVfDC6-19FXUjgg@mail.gmail.com>
<CAHewXNmx+UXg46+WUrbPca91bmVipRTpe+SRm19GtxG6mArRhg@mail.gmail.com>
<CALdSSPi6xR1tG2kLvpwNLnAjG9e0wmaY62r2_MF81ZYg5in+qQ@mail.gmail.com>
<[email protected]>
<CAApHDvpYEqJ6h-3NWi_4S19RY9NARpJ3h8CRmWYbz5MJFqE-sg@mail.gmail.com>
<CA+HiwqEHHTG5_TKuNw1M0dCrgUd6SauJ5dcdicz7xozMJip0SA@mail.gmail.com>
<CA+HiwqE4y8So7zpDK-HDeSLtm_B7eH6ZdbWD=NYrvznEnkGeBA@mail.gmail.com>
<CALdSSPj_U4DAv4xZPPcUr8Gd9KGZh0g0Uf5TWjc=DuW2rjnSjw@mail.gmail.com>
Hi,
On Fri, Nov 7, 2025 at 6:05 PM Kirill Reshke <[email protected]> wrote:
> On Fri, 7 Nov 2025 at 11:02, Amit Langote <[email protected]> wrote:
> >
> > On Thu, Nov 6, 2025 at 7:00 PM Amit Langote <[email protected]> wrote:
> > > I looked at a few options, but none seem non-invasive enough for
> > > back-patching, apart from the first patch I posted. That one makes
> > > ExecInitModifyTable() not require a ctid to be present to set the root
> > > partitioned table’s ri_RowIdAttNo, except in the case of MERGE, which
> > > seems to need it. The corner case that triggers the internal error for
> > > UPDATE/DELETE doesn’t occur for MERGE now and likely won’t when
> > > foreign tables eventually gain MERGE support; don't mark my words
> > > though ;-).
> >
> > Well, OK, I just had not tried hard enough to see that the same error
> > happens for MERGE too.
> >
> > With my patch applied:
> > EXPLAIN VERBOSE MERGE INTO pt t USING (VALUES (1, 'x'::text)) AS s(a,
> > b) ON false WHEN MATCHED THEN UPDATE SET b = s.b;
> > ERROR: could not find junk ctid column
> >
> > I have another idea: we can simply recognize the corner condition that
> > throws this error in ExecInitModifyTable() by checking if
> > ModifyTable.resultRelations contains only the root partitioned table.
> > That can only happen for UPDATE, DELETE, or MERGE when all child
> > relations were excluded.
> >
> > Patch doing that attached. Added test cases to file_fdw's suite.
>
> HI!
>
> I think this is an OK option for backpatching. After v2 applied, I
> found the behavior of DELETE and EXPLAIN DELETE consistent.
Thanks for the comment.
> The only
> remaining issue is VERBOSE output difference with or without
> enable_partition_pruning (which is v19+ issue to worry about),
> correct?
Yes, iff we are to do anything at all about the difference.
> Also, should we add COSTS OFF to EXPLAIN in the regression test? I
> understand that costs should be always zero, but COSTS OFF is almost
> everywhere is tests
Yeah, a good call.
v3 attached.
--
Thanks, Amit Langote
Attachments:
[application/octet-stream] v3-0001-Fix-bogus-ctid-requirement-for-dummy-root-partiti.patch (7.8K, 2-v3-0001-Fix-bogus-ctid-requirement-for-dummy-root-partiti.patch)
download | inline diff:
From 42c14e3a448983cc3e52260fdfc48a428a71a67a Mon Sep 17 00:00:00 2001
From: Amit Langote <[email protected]>
Date: Fri, 7 Nov 2025 18:12:46 +0900
Subject: [PATCH v3] Fix bogus ctid requirement for dummy-root partitioned
targets
ExecInitModifyTable() unconditionally required a ctid junk column even
when the target was a partitioned table. This led to spurious "could
not find junk ctid column" errors when all children were excluded and
only the dummy root result relation remained.
Require ctid for heap relations as before. For partitioned tables,
require it only when at least one leaf result relation remains in the
plan. If the plan has only the dummy root, no rows can be produced and
ctid is thus not needed.
Bug: #19099
Reported-by: Alexander Lakhin <[email protected]>
Author: Amit Langote <[email protected]>
Reviewed-by: Tender Wang <[email protected]>
Reviewed-by: Kirill Reshke <[email protected]>
Discussion: https://postgr.es/m/19099-e05dcfa022fe553d%40postgresql.org
Backpatch-through: 14
---
contrib/file_fdw/expected/file_fdw.out | 75 ++++++++++++++++++++++++++
contrib/file_fdw/sql/file_fdw.sql | 34 ++++++++++++
src/backend/executor/nodeModifyTable.c | 11 +++-
3 files changed, 119 insertions(+), 1 deletion(-)
diff --git a/contrib/file_fdw/expected/file_fdw.out b/contrib/file_fdw/expected/file_fdw.out
index 5121e27dce5..6f7b9175735 100644
--- a/contrib/file_fdw/expected/file_fdw.out
+++ b/contrib/file_fdw/expected/file_fdw.out
@@ -457,6 +457,81 @@ SELECT tableoid::regclass, * FROM p2;
p2 | 2 | xyzzy
(3 rows)
+-- Verify that a dummy root partitioned-table result relation works without
+-- error when all child partitions are excluded from the plan (for example,
+-- by constraint exclusion or pruning). In this case, the executor accepts
+-- a missing ctid for the root result relation since no rows can be produced.
+-- When a foreign-table child is processed before exclusion, a tableoid junk
+-- column may still appear in the targetlist and also wholerow for update.
+-- Dummy-root cases where all children are excluded.
+-- With pruning off, the foreign child is processed first, then excluded
+-- by constraint exclusion. EXPLAIN shows tableoid (rewritten to NULL),
+-- and for UPDATE also wholerow as NULL::record. No ctid.
+DROP TABLE p2;
+SET enable_partition_pruning TO off;
+EXPLAIN (COSTS OFF, VERBOSE) DELETE FROM pt WHERE false;
+ QUERY PLAN
+--------------------------------
+ Delete on public.pt
+ -> Result
+ Output: NULL::oid
+ Replaces: Scan on pt
+ One-Time Filter: false
+(5 rows)
+
+-- also cover wholerow for UPDATE; expect NULL::oid and NULL::record
+EXPLAIN (COSTS OFF, VERBOSE) UPDATE pt SET b = 'x' WHERE false;
+ QUERY PLAN
+----------------------------------------------------
+ Update on public.pt
+ -> Result
+ Output: 'x'::text, NULL::oid, NULL::record
+ Replaces: Scan on pt
+ One-Time Filter: false
+(5 rows)
+
+-- MERGE behaves the same here; expect NULL::oid
+EXPLAIN (COSTS OFF, VERBOSE) MERGE INTO pt t USING (VALUES (1, 'x'::text)) AS s(a, b)
+ ON false WHEN MATCHED THEN UPDATE SET b = s.b;
+ QUERY PLAN
+--------------------------------
+ Merge on public.pt t
+ -> Result
+ Output: NULL::oid
+ Replaces: Scan on t
+ One-Time Filter: false
+(5 rows)
+
+-- With pruning on, the foreign child is pruned entirely. The plan has only
+-- the dummy root, and EXPLAIN shows ctid (and for UPDATE, ctid plus target).
+SET enable_partition_pruning TO on;
+EXPLAIN (COSTS OFF, VERBOSE) DELETE FROM pt WHERE false;
+ QUERY PLAN
+--------------------------------
+ Delete on public.pt
+ -> Result
+ Output: ctid
+ Replaces: Scan on pt
+ One-Time Filter: false
+(5 rows)
+
+EXPLAIN (COSTS OFF, VERBOSE) UPDATE pt SET b = 'x' WHERE false;
+ QUERY PLAN
+---------------------------------
+ Update on public.pt
+ -> Result
+ Output: 'x'::text, ctid
+ Replaces: Scan on pt
+ One-Time Filter: false
+(5 rows)
+
+-- Foreign child not pruned and it does not support DELETE: error.
+EXPLAIN (COSTS OFF, VERBOSE) DELETE FROM pt WHERE a = 1;
+ERROR: cannot delete from foreign table "p1"
+-- Runtime pruning includes the foreign child in the plan; executor errors
+-- since the foreign child does not support the command.
+EXPLAIN (COSTS OFF, VERBOSE) DELETE FROM pt WHERE (SELECT false);
+ERROR: cannot delete from foreign table "p1"
DROP TABLE pt;
-- generated column tests
\set filename :abs_srcdir '/data/list1.csv'
diff --git a/contrib/file_fdw/sql/file_fdw.sql b/contrib/file_fdw/sql/file_fdw.sql
index 1a397ad4bd1..25658b1f2dc 100644
--- a/contrib/file_fdw/sql/file_fdw.sql
+++ b/contrib/file_fdw/sql/file_fdw.sql
@@ -242,6 +242,40 @@ UPDATE pt set a = 1 where a = 2; -- ERROR
SELECT tableoid::regclass, * FROM pt;
SELECT tableoid::regclass, * FROM p1;
SELECT tableoid::regclass, * FROM p2;
+
+-- Verify that a dummy root partitioned-table result relation works without
+-- error when all child partitions are excluded from the plan (for example,
+-- by constraint exclusion or pruning). In this case, the executor accepts
+-- a missing ctid for the root result relation since no rows can be produced.
+-- When a foreign-table child is processed before exclusion, a tableoid junk
+-- column may still appear in the targetlist and also wholerow for update.
+
+-- Dummy-root cases where all children are excluded.
+-- With pruning off, the foreign child is processed first, then excluded
+-- by constraint exclusion. EXPLAIN shows tableoid (rewritten to NULL),
+-- and for UPDATE also wholerow as NULL::record. No ctid.
+DROP TABLE p2;
+SET enable_partition_pruning TO off;
+EXPLAIN (COSTS OFF, VERBOSE) DELETE FROM pt WHERE false;
+-- also cover wholerow for UPDATE; expect NULL::oid and NULL::record
+EXPLAIN (COSTS OFF, VERBOSE) UPDATE pt SET b = 'x' WHERE false;
+-- MERGE behaves the same here; expect NULL::oid
+EXPLAIN (COSTS OFF, VERBOSE) MERGE INTO pt t USING (VALUES (1, 'x'::text)) AS s(a, b)
+ ON false WHEN MATCHED THEN UPDATE SET b = s.b;
+
+-- With pruning on, the foreign child is pruned entirely. The plan has only
+-- the dummy root, and EXPLAIN shows ctid (and for UPDATE, ctid plus target).
+SET enable_partition_pruning TO on;
+EXPLAIN (COSTS OFF, VERBOSE) DELETE FROM pt WHERE false;
+EXPLAIN (COSTS OFF, VERBOSE) UPDATE pt SET b = 'x' WHERE false;
+
+-- Foreign child not pruned and it does not support DELETE: error.
+EXPLAIN (COSTS OFF, VERBOSE) DELETE FROM pt WHERE a = 1;
+
+-- Runtime pruning includes the foreign child in the plan; executor errors
+-- since the foreign child does not support the command.
+EXPLAIN (COSTS OFF, VERBOSE) DELETE FROM pt WHERE (SELECT false);
+
DROP TABLE pt;
-- generated column tests
diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
index 4c5647ac38a..9f4dce8668f 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -4863,7 +4863,16 @@ 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 when at least one leaf
+ * result relation remains in the plan. If the plan has only the
+ * dummy root (no leaves), no rows can be produced and ctid is not
+ * needed.
+ */
+ if (!AttributeNumberIsValid(resultRelInfo->ri_RowIdAttNo) &&
+ (relkind != RELKIND_PARTITIONED_TABLE || nrels > 1))
elog(ERROR, "could not find junk ctid column");
}
else if (relkind == RELKIND_FOREIGN_TABLE)
--
2.47.3
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], [email protected]
Subject: Re: BUG #19099: Conditional DELETE from partitioned table with non-updatable partition raises internal error
In-Reply-To: <CA+HiwqHe+qQ9BY3RSOqCN50bwWGve-QCoRJaDwhzpy_B91BC6A@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