public inbox for [email protected]  
help / color / mirror / Atom feed
From: Amit Langote <[email protected]>
To: Alexander Lakhin <[email protected]>
Cc: Tomas Vondra <[email protected]>
Cc: Robert Haas <[email protected]>
Cc: Alvaro Herrera <[email protected]>
Cc: Andres Freund <[email protected]>
Cc: Daniel Gustafsson <[email protected]>
Cc: David Rowley <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Cc: Thom Brown <[email protected]>
Cc: Tom Lane <[email protected]>
Subject: Re: generic plans and "initial" pruning
Date: Sat, 15 Feb 2025 16:51:09 +0900
Message-ID: <CA+HiwqG=gZZCmpR5VMnXYCm0JDxH3dOwiPngnMLCYTj1unWh0w@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <CA+HiwqFpZ80UJKr4tZus4Omgg7YESzFXKSwSHRW2Ap2=XSVyUA@mail.gmail.com>
	<CA+HiwqH45ZCQkWoLzjOyS6bQ9QsF7yDCKVwiEUtB_RwPFwLmQg@mail.gmail.com>
	<CA+HiwqHRRFQN6yZ54fBydOTM6ncqZBCmewZ6n519RjRdDsO44g@mail.gmail.com>
	<[email protected]>
	<CA+HiwqH8N-SxEB6SysEBsYNgV_KJs66k9Z2SNmqVzbBP-60yWg@mail.gmail.com>
	<[email protected]>
	<CA+HiwqEmG9YCQvG6uux7sO=jKFSAW6hA4Ea-ymfD+JhJAe4PWQ@mail.gmail.com>
	<CA+HiwqE2FfJfH=siLiR3kJ13tmXZORAGTWsZc2r52o1_5BDv+g@mail.gmail.com>
	<[email protected]>
	<CA+HiwqFhkpXHAA=4NY5SqYXX08uq=nYtXcSByNZF=2MAy1UA7A@mail.gmail.com>
	<CA+HiwqHCcSoYfpMjFshaU1bj6NjreiDvMSDpVSeBmqk-kbWrPw@mail.gmail.com>
	<CA+HiwqHOejJk0_qMuM5g38h70hY_JvHMAKwnH3k=urfTXauPQA@mail.gmail.com>
	<CA+HiwqFsGKM82oaMby3VWYXf_XFpDAMeT+6SXgj-45HpTrS1dA@mail.gmail.com>
	<CA+HiwqFA5hUWYktt3VMh4zQOYMxqH-MpdX8eemfM+o-9dY-zbQ@mail.gmail.com>
	<CA+HiwqEn7bbUXaXO=SmUujBjJSHfS31cwQroHRBwT0sR=66bgg@mail.gmail.com>
	<[email protected]>

Hi Alexander,

On Sat, Feb 15, 2025 at 6:00 AM Alexander Lakhin <[email protected]> wrote:
>
> Hello Amit,
>
> 06.02.2025 04:35, Amit Langote wrote:
>
> I plan to push 0001 tomorrow, barring any objections.
>
>
> Please try the following script:
> CREATE TABLE pt (a int, b int) PARTITION BY range (a);
> CREATE TABLE tp1 PARTITION OF pt FOR VALUES FROM (1) TO (2);
> CREATE TABLE tp2 PARTITION OF pt FOR VALUES FROM (2) TO (3);
>
> MERGE INTO pt
> USING (SELECT pg_backend_pid() AS pid) AS q JOIN tp1 ON (q.pid = tp1.a)
> ON pt.a = tp1.a
> WHEN MATCHED THEN DELETE;
>
> which fails for me with segfault:
> Program terminated with signal SIGSEGV, Segmentation fault.
> #0  ExecInitMerge (mtstate=0x5a9b9fbccae0, estate=0x5a9b9fbcbe20) at nodeModifyTable.c:3680
> 3680                    relationDesc = RelationGetDescr(resultRelInfo->ri_RelationDesc);
> (gdb) bt
> #0  ExecInitMerge (mtstate=0x5a9b9fbccae0, estate=0x5a9b9fbcbe20) at nodeModifyTable.c:3680
> #1  0x00005a9b67e6dfb5 in ExecInitModifyTable (node=0x5a9b9fbd5858, estate=0x5a9b9fbcbe20, eflags=0) at nodeModifyTable.c:4906
> #2  0x00005a9b67e273f7 in ExecInitNode (node=0x5a9b9fbd5858, estate=0x5a9b9fbcbe20, eflags=0) at execProcnode.c:177
> #3  0x00005a9b67e1b9d2 in InitPlan (queryDesc=0x5a9b9fbb9970, eflags=0) at execMain.c:1092
> #4  0x00005a9b67e1a524 in standard_ExecutorStart (queryDesc=0x5a9b9fbb9970, eflags=0) at execMain.c:268
> #5  0x00005a9b67e1a223 in ExecutorStart (queryDesc=0x5a9b9fbb9970, eflags=0) at execMain.c:142
> ...
>
> starting from cbc127917.
>
> (I've discovered this anomaly with SQLsmith.)

Thanks! It looks like I missed updating the MERGE-related lists in ModifyTable.

I've attached a fix with a test added based on your example. I plan to
push this on Monday.

-- 
Thanks, Amit Langote


Attachments:

  [application/octet-stream] 0001-Fix-an-oversight-in-cbc127917-for-MERGE-handling.patch (6.5K, 2-0001-Fix-an-oversight-in-cbc127917-for-MERGE-handling.patch)
  download | inline diff:
From 07784159aea4de7b5614fd7a39bb6eeafe07cb22 Mon Sep 17 00:00:00 2001
From: Amit Langote <[email protected]>
Date: Sat, 15 Feb 2025 16:39:54 +0900
Subject: [PATCH] Fix an oversight in cbc127917 for MERGE handling

ExecInitModifyTable() should also trim MERGE-related lists to exclude
result relations pruned during initial pruning.

Reported-by: Alexander Lakhin <[email protected]> (via sqlsmith)
Discussion: https://postgr.es/m/[email protected]
---
 src/backend/executor/nodeModifyTable.c        | 24 ++++++++++---
 src/include/nodes/execnodes.h                 |  7 ++--
 src/test/regress/expected/partition_prune.out | 34 +++++++++++++++++++
 src/test/regress/sql/partition_prune.sql      | 13 +++++++
 4 files changed, 72 insertions(+), 6 deletions(-)

diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
index a15e7863b0d..e0f859ba966 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -3667,14 +3667,14 @@ ExecInitMerge(ModifyTableState *mtstate, EState *estate)
 	 * anything here, do so there too.
 	 */
 	i = 0;
-	foreach(lc, node->mergeActionLists)
+	foreach(lc, mtstate->mt_mergeActionLists)
 	{
 		List	   *mergeActionList = lfirst(lc);
 		Node	   *joinCondition;
 		TupleDesc	relationDesc;
 		ListCell   *l;
 
-		joinCondition = (Node *) list_nth(node->mergeJoinConditions, i);
+		joinCondition = (Node *) list_nth(mtstate->mt_mergeJoinConditions, i);
 		resultRelInfo = mtstate->resultRelInfo + i;
 		i++;
 		relationDesc = RelationGetDescr(resultRelInfo->ri_RelationDesc);
@@ -4475,6 +4475,8 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
 	List	   *withCheckOptionLists = NIL;
 	List	   *returningLists = NIL;
 	List	   *updateColnosLists = NIL;
+	List	   *mergeActionLists = NIL;
+	List	   *mergeJoinConditions = NIL;
 	ResultRelInfo *resultRelInfo;
 	List	   *arowmarks;
 	ListCell   *l;
@@ -4518,6 +4520,18 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
 
 				updateColnosLists = lappend(updateColnosLists, updateColnosList);
 			}
+			if (node->mergeActionLists)
+			{
+				List	   *mergeActionList = list_nth(node->mergeActionLists, i);
+
+				mergeActionLists = lappend(mergeActionLists, mergeActionList);
+			}
+			if (node->mergeJoinConditions)
+			{
+				List	   *mergeJoinCondition = list_nth(node->mergeJoinConditions, i);
+
+				mergeJoinConditions = lappend(mergeJoinConditions, mergeJoinCondition);
+			}
 		}
 		i++;
 	}
@@ -4544,6 +4558,8 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
 	mtstate->mt_merge_updated = 0;
 	mtstate->mt_merge_deleted = 0;
 	mtstate->mt_updateColnosLists = updateColnosLists;
+	mtstate->mt_mergeActionLists = mergeActionLists;
+	mtstate->mt_mergeJoinConditions = mergeJoinConditions;
 
 	/*----------
 	 * Resolve the target relation. This is the same as:
@@ -4599,8 +4615,8 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
 		Index		resultRelation = lfirst_int(l);
 		List	   *mergeActions = NIL;
 
-		if (node->mergeActionLists)
-			mergeActions = list_nth(node->mergeActionLists, i);
+		if (mergeActionLists)
+			mergeActions = list_nth(mergeActionLists, i);
 
 		if (resultRelInfo != mtstate->rootResultRelInfo)
 		{
diff --git a/src/include/nodes/execnodes.h b/src/include/nodes/execnodes.h
index e2d1dc1e067..66fa6133343 100644
--- a/src/include/nodes/execnodes.h
+++ b/src/include/nodes/execnodes.h
@@ -1448,10 +1448,13 @@ typedef struct ModifyTableState
 	double		mt_merge_deleted;
 
 	/*
-	 * List of valid updateColnosLists.  Contains only those belonging to
-	 * unpruned relations from ModifyTable.updateColnosLists.
+	 * Lists of valid updateColnosListsm, mergeActionLists, and
+	 * mergeJoinConditions.  These contain only those belonging to unpruned
+	 * relations from the respective Lists in the ModifyTable.
 	 */
 	List	   *mt_updateColnosLists;
+	List	   *mt_mergeActionLists;
+	List	   *mt_mergeJoinConditions;
 } ModifyTableState;
 
 /* ----------------
diff --git a/src/test/regress/expected/partition_prune.out b/src/test/regress/expected/partition_prune.out
index e667503c961..3261da28219 100644
--- a/src/test/regress/expected/partition_prune.out
+++ b/src/test/regress/expected/partition_prune.out
@@ -4513,5 +4513,39 @@ execute update_part_abc_view (2, 'a');
 ERROR:  new row violates check option for view "part_abc_view"
 DETAIL:  Failing row contains (2, a, t).
 deallocate update_part_abc_view;
+-- Runtime pruning on MERGE using a stable function
+create function stable_one() returns int as $$ begin return 1; end; $$ language plpgsql stable;
+explain (costs off)
+merge into part_abc_view pt
+using (select stable_one() as pid) as q join part_abc_1 pt1 on (q.pid = pt1.a) on pt.a = pt1.a
+when matched then delete returning pt.a;
+                              QUERY PLAN                               
+-----------------------------------------------------------------------
+ Merge on part_abc
+   Merge on part_abc_1
+   ->  Nested Loop
+         ->  Append
+               Subplans Removed: 1
+               ->  Seq Scan on part_abc_1
+                     Filter: ((b <> 'a'::text) AND (a = stable_one()))
+         ->  Materialize
+               ->  Seq Scan on part_abc_1 pt1
+                     Filter: (a = stable_one())
+(10 rows)
+
+merge into part_abc_view pt
+using (select stable_one() as pid) as q join part_abc_1 pt1 on (q.pid = pt1.a) on pt.a = pt1.a
+when matched then delete returning pt.a;
+ a 
+---
+ 1
+(1 row)
+
+table part_abc_view;
+ a | b | c 
+---+---+---
+ 2 | c | t
+(1 row)
+
 drop view part_abc_view;
 drop table part_abc;
diff --git a/src/test/regress/sql/partition_prune.sql b/src/test/regress/sql/partition_prune.sql
index 730545e86a7..b27f3ace73c 100644
--- a/src/test/regress/sql/partition_prune.sql
+++ b/src/test/regress/sql/partition_prune.sql
@@ -1372,5 +1372,18 @@ execute update_part_abc_view (1, 'd');
 explain (costs off) execute update_part_abc_view (2, 'a');
 execute update_part_abc_view (2, 'a');
 deallocate update_part_abc_view;
+
+-- Runtime pruning on MERGE using a stable function
+create function stable_one() returns int as $$ begin return 1; end; $$ language plpgsql stable;
+explain (costs off)
+merge into part_abc_view pt
+using (select stable_one() as pid) as q join part_abc_1 pt1 on (q.pid = pt1.a) on pt.a = pt1.a
+when matched then delete returning pt.a;
+
+merge into part_abc_view pt
+using (select stable_one() as pid) as q join part_abc_1 pt1 on (q.pid = pt1.a) on pt.a = pt1.a
+when matched then delete returning pt.a;
+table part_abc_view;
+
 drop view part_abc_view;
 drop table part_abc;
-- 
2.43.0



view thread (66+ messages)  latest in thread

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], [email protected], [email protected]
  Subject: Re: generic plans and "initial" pruning
  In-Reply-To: <CA+HiwqG=gZZCmpR5VMnXYCm0JDxH3dOwiPngnMLCYTj1unWh0w@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