pg.ddx.io pgsql-bugs@postgresql.org mailing list archive
help / color / mirror / Atom feedBUG #18830: ExecInitMerge Segfault on MERGE
32+ messages / 5 participants
[nested] [flat]
Commits from this thread:
2025-03-19 28317de723 Ensure first ModifyTable rel initialized if all are pruned
* BUG #18830: ExecInitMerge Segfault on MERGE
@ 2025-03-02 21:00 PG Bug reporting form <noreply@postgresql.org>
2025-03-03 15:46 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Tender Wang <tndrwang@gmail.com>
0 siblings, 1 reply; 32+ messages in thread
From: PG Bug reporting form @ 2025-03-02 21:00 UTC (permalink / raw)
To: pgsql-bugs@lists.postgresql.org; +Cc: tharakan@gmail.com
The following bug has been logged on the website:
Bug reference: 18830
Logged by: Robins Tharakan
Email address: tharakan@gmail.com
PostgreSQL version: Unsupported/Unknown
Operating system: Ubuntu
Description:
With this SQL (backtraces below), postgres starts to segfault starting at
cbc127917e.
SQL
===
CREATE AGGREGATE d(double precision ORDER BY anyelement) (
SFUNC = ordered_set_transition_multi,
STYPE = internal,
FINALFUNC = rank_final);
CREATE TABLE e (
a text,
b integer
) PARTITION BY LIST (a);
CREATE TABLE f (
a text,
b integer
);
CREATE TABLE g (
a text,
b integer
);
CREATE VIEW h AS
SELECT XMLSERIALIZE(DOCUMENT '<foo><bar>42</bar></foo>'AS text );
ALTER TABLE e ATTACH PARTITION f FOR VALUES IN ('a');
ALTER TABLE e ATTACH PARTITION g FOR VALUES IN ('b');
MERGE INTO e USING h ON a = xmlserialize WHEN NOT MATCHED THEN INSERT
VALUES (CAST(NULL AS text));
SQL Output
==========
$ psql -p 9999 postgres -f ../sqith/repro_final.sql
CREATE AGGREGATE
CREATE TABLE
CREATE TABLE
CREATE TABLE
CREATE VIEW
ALTER TABLE
ALTER TABLE
psql:../sqith/repro_final.sql:25: server closed the connection
unexpectedly
This probably means the server terminated abnormally
before or while processing the request.
psql:../sqith/repro_final.sql:25: error: connection to server was lost
Error Log
=========
2025-03-03 06:55:54.014 ACDT [137463] LOG: client backend (PID 137479) was
terminated by signal 11: Segmentation fault
2025-03-03 06:55:54.014 ACDT [137463] DETAIL: Failed process was running:
MERGE INTO e USING h ON a = xmlserialize WHEN NOT MATCHED THEN INSERT
VALUES (CAST(NULL AS text));
2025-03-03 06:55:54.014 ACDT [137463] LOG: terminating any other active
server processes
Commit
======
Checking (15a79c73111~0) - 15a79c7311 - fail
Checking (15a79c73111~10) - 424ededc58 - fail
Checking (15a79c73111~30) - 945a9e3832 - fail
Checking (15a79c73111~70) - a4e986ef5a - fail
Checking (15a79c73111~150) - 6a2275b895 - fail
Checking (15a79c73111~310) - 117f9f328e - Pass
Checking (15a79c73111~230) - c89525d57b - Pass
Checking (15a79c73111~190) - c366d2bdba - fail
Checking (15a79c73111~210) - fb056564ec - fail
Checking (15a79c73111~220) - 44ec095751 - Pass
Checking (15a79c73111~215) - cbc127917e - fail
Checking (15a79c73111~217) - 428fadb7e9 - Pass
Checking (15a79c73111~216) - 926c7fce03 - Pass
Offending Commit is cbc127917e - HEAD~215
BackTrace
=========
(gdb) bt
#0 ExecInitMerge (mtstate=0x599c6b260330, estate=0x599c6b260080) at
nodeModifyTable.c:3663
#1 0x0000599c2ca1ce11 in ExecInitModifyTable (node=0x599c6b282e08,
estate=0x599c6b260080, eflags=0) at nodeModifyTable.c:4889
#2 0x0000599c2c9d63a3 in ExecInitNode (node=0x599c6b282e08,
estate=0x599c6b260080, eflags=0) at execProcnode.c:177
#3 0x0000599c2c9cac66 in InitPlan (queryDesc=0x599c6b218ba0, eflags=0) at
execMain.c:985
#4 0x0000599c2c9c99f0 in standard_ExecutorStart (queryDesc=0x599c6b218ba0,
eflags=0) at execMain.c:259
#5 0x0000599c2c9c96fb in ExecutorStart (queryDesc=0x599c6b218ba0, eflags=0)
at execMain.c:135
#6 0x0000599c2ccb3925 in ProcessQuery (plan=0x599c6b284188,
sourceText=0x599c6b16c4e0 "MERGE INTO e USING h ON a = xmlserialize
WHEN NOT MATCHED THEN INSERT VALUES (CAST(NULL AS text));", params=0x0,
queryEnv=0x0, dest=0x599c6b284308,
qc=0x7ffc8bc1aaf0) at pquery.c:155
#7 0x0000599c2ccb546e in PortalRunMulti (portal=0x599c6b1ef0a0,
isTopLevel=true, setHoldSnapshot=false, dest=0x599c6b284308,
altdest=0x599c6b284308, qc=0x7ffc8bc1aaf0)
at pquery.c:1271
#8 0x0000599c2ccb497a in PortalRun (portal=0x599c6b1ef0a0,
count=9223372036854775807, isTopLevel=true, dest=0x599c6b284308,
altdest=0x599c6b284308, qc=0x7ffc8bc1aaf0) at pquery.c:787
#9 0x0000599c2ccad440 in exec_simple_query (query_string=0x599c6b16c4e0
"MERGE INTO e USING h ON a = xmlserialize WHEN NOT MATCHED THEN INSERT
VALUES (CAST(NULL AS text));")
at postgres.c:1271
#10 0x0000599c2ccb27ef in PostgresMain (dbname=0x599c6b1a6680 "smithreduce",
username=0x599c6b1a6668 "smith") at postgres.c:4691
BackTrace Full
==============
#0 ExecInitMerge (mtstate=0x599c6b260330, estate=0x599c6b260080) at
nodeModifyTable.c:3663
mergeActionList = 0x599c6b281b70
joinCondition = 0x0
relationDesc = 0x599c2c9df7cb <ExecTypeFromTL+33>
l = 0x100000000
lc__state = {l = 0x599c6b281bc0, i = 0}
node = 0x599c6b282e08
rootRelInfo = 0x599c6b260f98
resultRelInfo = 0x599c6b260f80
econtext = 0x599c6b2695f8
lc = 0x599c6b281bd8
i = 1
__func__ = "ExecInitMerge"
#1 0x0000599c2ca1ce11 in ExecInitModifyTable (node=0x599c6b282e08,
estate=0x599c6b260080, eflags=0) at nodeModifyTable.c:4889
mtstate = 0x599c6b260330
subplan = 0x599c6b282d78
operation = CMD_MERGE
nrels = 0
resultRelations = 0x0
withCheckOptionLists = 0x0
returningLists = 0x0
updateColnosLists = 0x0
resultRelInfo = 0x599c6b260f80
arowmarks = 0x0
l = 0x0
i = 0
rel = 0x7be2ba49e368
__func__ = "ExecInitModifyTable"
#2 0x0000599c2c9d63a3 in ExecInitNode (node=0x599c6b282e08,
estate=0x599c6b260080, eflags=0) at execProcnode.c:177
result = 0x0
subps = 0x599c6b282760
l = 0x599c6b2608c0
__func__ = "ExecInitNode"
#3 0x0000599c2c9cac66 in InitPlan (queryDesc=0x599c6b218ba0, eflags=0) at
execMain.c:985
operation = CMD_MERGE
plannedstmt = 0x599c6b284188
plan = 0x599c6b282e08
rangeTable = 0x599c6b283228
estate = 0x599c6b260080
planstate = 0x599c6b1ab310
tupType = 0x599c6b2011f0
l = 0x0
i = 1
__func__ = "InitPlan"
This (and bug #18828) found using SQLSmith / SQLReduce / creduce.
^ permalink raw reply [nested|flat] 32+ messages in thread
* Re: BUG #18830: ExecInitMerge Segfault on MERGE
2025-03-02 21:00 BUG #18830: ExecInitMerge Segfault on MERGE PG Bug reporting form <noreply@postgresql.org>
@ 2025-03-03 15:46 ` Tender Wang <tndrwang@gmail.com>
2025-03-04 06:58 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Tender Wang <tndrwang@gmail.com>
2025-03-04 11:51 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Amit Langote <amitlangote09@gmail.com>
0 siblings, 2 replies; 32+ messages in thread
From: Tender Wang @ 2025-03-03 15:46 UTC (permalink / raw)
To: tharakan@gmail.com; pgsql-bugs@lists.postgresql.org
PG Bug reporting form <noreply@postgresql.org> 于2025年3月3日周一 18:31写道：
> The following bug has been logged on the website:
>
> Bug reference: 18830
> Logged by: Robins Tharakan
> Email address: tharakan@gmail.com
> PostgreSQL version: Unsupported/Unknown
> Operating system: Ubuntu
> Description:
>
> With this SQL (backtraces below), postgres starts to segfault starting at
> cbc127917e.
>
>
> SQL
> ===
> CREATE AGGREGATE d(double precision ORDER BY anyelement) (
> SFUNC = ordered_set_transition_multi,
> STYPE = internal,
> FINALFUNC = rank_final);
> CREATE TABLE e (
> a text,
> b integer
> ) PARTITION BY LIST (a);
> CREATE TABLE f (
> a text,
> b integer
> );
> CREATE TABLE g (
> a text,
> b integer
> );
> CREATE VIEW h AS
> SELECT XMLSERIALIZE(DOCUMENT '<foo><bar>42</bar></foo>'AS text );
> ALTER TABLE e ATTACH PARTITION f FOR VALUES IN ('a');
> ALTER TABLE e ATTACH PARTITION g FOR VALUES IN ('b');
>
> MERGE INTO e USING h ON a = xmlserialize WHEN NOT MATCHED THEN INSERT
> VALUES (CAST(NULL AS text));
>
>
> SQL Output
> ==========
> $ psql -p 9999 postgres -f ../sqith/repro_final.sql
> CREATE AGGREGATE
> CREATE TABLE
> CREATE TABLE
> CREATE TABLE
> CREATE VIEW
> ALTER TABLE
> ALTER TABLE
> psql:../sqith/repro_final.sql:25: server closed the connection
> unexpectedly
> This probably means the server terminated abnormally
> before or while processing the request.
> psql:../sqith/repro_final.sql:25: error: connection to server was lost
>
>
>
> Error Log
> =========
> 2025-03-03 06:55:54.014 ACDT [137463] LOG: client backend (PID 137479) was
> terminated by signal 11: Segmentation fault
> 2025-03-03 06:55:54.014 ACDT [137463] DETAIL: Failed process was running:
> MERGE INTO e USING h ON a = xmlserialize WHEN NOT MATCHED THEN INSERT
> VALUES (CAST(NULL AS text));
> 2025-03-03 06:55:54.014 ACDT [137463] LOG: terminating any other active
> server processes
>
>
> Commit
> ======
> Checking (15a79c73111~0) - 15a79c7311 - fail
> Checking (15a79c73111~10) - 424ededc58 - fail
> Checking (15a79c73111~30) - 945a9e3832 - fail
> Checking (15a79c73111~70) - a4e986ef5a - fail
> Checking (15a79c73111~150) - 6a2275b895 - fail
> Checking (15a79c73111~310) - 117f9f328e - Pass
> Checking (15a79c73111~230) - c89525d57b - Pass
> Checking (15a79c73111~190) - c366d2bdba - fail
> Checking (15a79c73111~210) - fb056564ec - fail
> Checking (15a79c73111~220) - 44ec095751 - Pass
> Checking (15a79c73111~215) - cbc127917e - fail
> Checking (15a79c73111~217) - 428fadb7e9 - Pass
> Checking (15a79c73111~216) - 926c7fce03 - Pass
> Offending Commit is cbc127917e - HEAD~215
>
>
> BackTrace
> =========
> (gdb) bt
> #0 ExecInitMerge (mtstate=0x599c6b260330, estate=0x599c6b260080) at
> nodeModifyTable.c:3663
> #1 0x0000599c2ca1ce11 in ExecInitModifyTable (node=0x599c6b282e08,
> estate=0x599c6b260080, eflags=0) at nodeModifyTable.c:4889
> #2 0x0000599c2c9d63a3 in ExecInitNode (node=0x599c6b282e08,
> estate=0x599c6b260080, eflags=0) at execProcnode.c:177
> #3 0x0000599c2c9cac66 in InitPlan (queryDesc=0x599c6b218ba0, eflags=0) at
> execMain.c:985
> #4 0x0000599c2c9c99f0 in standard_ExecutorStart (queryDesc=0x599c6b218ba0,
> eflags=0) at execMain.c:259
> #5 0x0000599c2c9c96fb in ExecutorStart (queryDesc=0x599c6b218ba0,
> eflags=0)
> at execMain.c:135
> #6 0x0000599c2ccb3925 in ProcessQuery (plan=0x599c6b284188,
> sourceText=0x599c6b16c4e0 "MERGE INTO e USING h ON a = xmlserialize
> WHEN NOT MATCHED THEN INSERT VALUES (CAST(NULL AS text));", params=0x0,
> queryEnv=0x0, dest=0x599c6b284308,
> qc=0x7ffc8bc1aaf0) at pquery.c:155
> #7 0x0000599c2ccb546e in PortalRunMulti (portal=0x599c6b1ef0a0,
> isTopLevel=true, setHoldSnapshot=false, dest=0x599c6b284308,
> altdest=0x599c6b284308, qc=0x7ffc8bc1aaf0)
> at pquery.c:1271
> #8 0x0000599c2ccb497a in PortalRun (portal=0x599c6b1ef0a0,
> count=9223372036854775807, isTopLevel=true, dest=0x599c6b284308,
> altdest=0x599c6b284308, qc=0x7ffc8bc1aaf0) at pquery.c:787
> #9 0x0000599c2ccad440 in exec_simple_query (query_string=0x599c6b16c4e0
> "MERGE INTO e USING h ON a = xmlserialize WHEN NOT MATCHED THEN INSERT
> VALUES (CAST(NULL AS text));")
> at postgres.c:1271
> #10 0x0000599c2ccb27ef in PostgresMain (dbname=0x599c6b1a6680
> "smithreduce",
> username=0x599c6b1a6668 "smith") at postgres.c:4691
>
>
>
> BackTrace Full
> ==============
> #0 ExecInitMerge (mtstate=0x599c6b260330, estate=0x599c6b260080) at
> nodeModifyTable.c:3663
> mergeActionList = 0x599c6b281b70
> joinCondition = 0x0
> relationDesc = 0x599c2c9df7cb <ExecTypeFromTL+33>
> l = 0x100000000
> lc__state = {l = 0x599c6b281bc0, i = 0}
> node = 0x599c6b282e08
> rootRelInfo = 0x599c6b260f98
> resultRelInfo = 0x599c6b260f80
> econtext = 0x599c6b2695f8
> lc = 0x599c6b281bd8
> i = 1
> __func__ = "ExecInitMerge"
> #1 0x0000599c2ca1ce11 in ExecInitModifyTable (node=0x599c6b282e08,
> estate=0x599c6b260080, eflags=0) at nodeModifyTable.c:4889
> mtstate = 0x599c6b260330
> subplan = 0x599c6b282d78
> operation = CMD_MERGE
> nrels = 0
> resultRelations = 0x0
> withCheckOptionLists = 0x0
> returningLists = 0x0
> updateColnosLists = 0x0
> resultRelInfo = 0x599c6b260f80
> arowmarks = 0x0
> l = 0x0
> i = 0
> rel = 0x7be2ba49e368
> __func__ = "ExecInitModifyTable"
> #2 0x0000599c2c9d63a3 in ExecInitNode (node=0x599c6b282e08,
> estate=0x599c6b260080, eflags=0) at execProcnode.c:177
> result = 0x0
> subps = 0x599c6b282760
> l = 0x599c6b2608c0
> __func__ = "ExecInitNode"
> #3 0x0000599c2c9cac66 in InitPlan (queryDesc=0x599c6b218ba0, eflags=0) at
> execMain.c:985
> operation = CMD_MERGE
> plannedstmt = 0x599c6b284188
> plan = 0x599c6b282e08
> rangeTable = 0x599c6b283228
> estate = 0x599c6b260080
> planstate = 0x599c6b1ab310
> tupType = 0x599c6b2011f0
> l = 0x0
> i = 1
> __func__ = "InitPlan"
>
>
> This (and bug #18828) found using SQLSmith / SQLReduce / creduce.
>
>
Thanks for reporting.
I can reproduce this crash. On HEAD 3f1db99bfa, crash on a different
place due to the commit 75dfde1.
But cbc127917e is the root cause commit.
MERGE INTO e USING h ON a = xmlserialize WHEN NOT MATCHED THEN INSERT
VALUES (CAST(NULL AS text));
The plan of the above query looks as below:
QUERY PLAN
------------------------------------------------------------------
Merge on e (cost=0.00..58.29 rows=0 width=0)
-> Nested Loop Left Join (cost=0.00..58.29 rows=12 width=10)
-> Result (cost=0.00..0.01 rows=1 width=0)
-> Append (cost=0.00..58.16 rows=12 width=10)
Subplans Removed: 2
You can see that all partitions are pruned. After cbc127917e, we only
consider unpruned relations, then the list resultRelations is empty.
the estate->es_unpruned_relids contains (1,2), the node->resultRelations
contains (5,6).
nrels = list_length(resultRelations);
...
mtstate->resultRelInfo = (ResultRelInfo *)
palloc(nrels * sizeof(ResultRelInfo));
The memory of mtstate->resultRelInfo point to is undefined. When we access
its memory in ExecInitMerge(),
relationDesc = RelationGetDescr(resultRelInfo->ri_RelationDesc);
crash happened.
After 75dfde1, mergeActionLists list is empty, so in ExecInitMerge() we
don't enter the foreach(lc, mergeActionLists),
and the mtstate->ps.ps_ExprContext has no chance to initialize. So
in ExecMergeNotMatched(), econtext is NULL.
econtext->ecxt_scantuple = NULL;
The above statement can trigger a segment fault.
For Merge command NOT MATCH, should we need the logic that only consider
unpruned relations in ExecInitModifyTable()?
Merge command seems more complex than update and delete. Can we consider
the unpruned relations in ExecInitModifyTable()
according to the command type. For merge, we do the logic before
cbc127917e, for now?
--
Thanks,
Tender Wang
^ permalink raw reply [nested|flat] 32+ messages in thread
* Re: BUG #18830: ExecInitMerge Segfault on MERGE
2025-03-02 21:00 BUG #18830: ExecInitMerge Segfault on MERGE PG Bug reporting form <noreply@postgresql.org>
2025-03-03 15:46 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Tender Wang <tndrwang@gmail.com>
@ 2025-03-04 06:58 ` Tender Wang <tndrwang@gmail.com>
2025-03-04 09:29 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE David Rowley <dgrowleyml@gmail.com>
1 sibling, 1 reply; 32+ messages in thread
From: Tender Wang @ 2025-03-04 06:58 UTC (permalink / raw)
To: pgsql-bugs@lists.postgresql.org, Amit Langote <amitlangote09@gmail.com>; +Cc: tharakan@gmail.com
Tender Wang <tndrwang@gmail.com> 于2025年3月3日周一 23:46写道：
>
> Thanks for reporting.
> I can reproduce this crash. On HEAD 3f1db99bfa, crash on a different
> place due to the commit 75dfde1.
> But cbc127917e is the root cause commit.
>
> MERGE INTO e USING h ON a = xmlserialize WHEN NOT MATCHED THEN INSERT
> VALUES (CAST(NULL AS text));
> The plan of the above query looks as below:
> QUERY PLAN
> ------------------------------------------------------------------
> Merge on e (cost=0.00..58.29 rows=0 width=0)
> -> Nested Loop Left Join (cost=0.00..58.29 rows=12 width=10)
> -> Result (cost=0.00..0.01 rows=1 width=0)
> -> Append (cost=0.00..58.16 rows=12 width=10)
> Subplans Removed: 2
>
> You can see that all partitions are pruned. After cbc127917e, we only
> consider unpruned relations, then the list resultRelations is empty.
> the estate->es_unpruned_relids contains (1,2), the node->resultRelations
> contains (5,6).
>
> nrels = list_length(resultRelations);
> ...
> mtstate->resultRelInfo = (ResultRelInfo *)
> palloc(nrels * sizeof(ResultRelInfo));
>
> The memory of mtstate->resultRelInfo point to is undefined. When we access
> its memory in ExecInitMerge(),
>
> relationDesc = RelationGetDescr(resultRelInfo->ri_RelationDesc);
>
> crash happened.
>
> After 75dfde1, mergeActionLists list is empty, so in ExecInitMerge() we
> don't enter the foreach(lc, mergeActionLists),
> and the mtstate->ps.ps_ExprContext has no chance to initialize. So
> in ExecMergeNotMatched(), econtext is NULL.
> econtext->ecxt_scantuple = NULL;
> The above statement can trigger a segment fault.
>
> For Merge command NOT MATCH, should we need the logic that only consider
> unpruned relations in ExecInitModifyTable()?
>
> Merge command seems more complex than update and delete. Can we consider
> the unpruned relations in ExecInitModifyTable()
> according to the command type. For merge, we do the logic before
> cbc127917e, for now?
>
>
I found the partition_prune.sql does not cover merge into ... not match
case, and I found an easy reproduce step, seeing below:
postgres=# merge into part_abc_view pt
using (select stable_one() + 2 as pid) as q join part_abc_1 pt1 on (true)
on pt.a = stable_one() +2
when not matched then insert values(1, 'd', false);
server closed the connection unexpectedly
This probably means the server terminated abnormally
before or while processing the request.
The connection to the server was lost. Attempting reset: Succeeded.
--
Thanks,
Tender Wang
^ permalink raw reply [nested|flat] 32+ messages in thread
* Re: BUG #18830: ExecInitMerge Segfault on MERGE
2025-03-02 21:00 BUG #18830: ExecInitMerge Segfault on MERGE PG Bug reporting form <noreply@postgresql.org>
2025-03-03 15:46 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Tender Wang <tndrwang@gmail.com>
2025-03-04 06:58 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Tender Wang <tndrwang@gmail.com>
@ 2025-03-04 09:29 ` David Rowley <dgrowleyml@gmail.com>
2025-03-04 09:41 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Amit Langote <amitlangote09@gmail.com>
2025-03-04 09:45 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Tender Wang <tndrwang@gmail.com>
2025-03-04 18:56 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Dean Rasheed <dean.a.rasheed@gmail.com>
0 siblings, 3 replies; 32+ messages in thread
From: David Rowley @ 2025-03-04 09:29 UTC (permalink / raw)
To: Tender Wang <tndrwang@gmail.com>; +Cc: pgsql-bugs@lists.postgresql.org, Amit Langote <amitlangote09@gmail.com>; tharakan@gmail.com
On Tue, 4 Mar 2025 at 19:58, Tender Wang <tndrwang@gmail.com> wrote:
> I found the partition_prune.sql does not cover merge into ... not match case, and I found an easy reproduce step, seeing below:
>
> postgres=# merge into part_abc_view pt
> using (select stable_one() + 2 as pid) as q join part_abc_1 pt1 on (true) on pt.a = stable_one() +2
> when not matched then insert values(1, 'd', false);
> server closed the connection unexpectedly
> This probably means the server terminated abnormally
> before or while processing the request.
> The connection to the server was lost. Attempting reset: Succeeded.
It looks like this is happening because ExecInitModifyTable() skips
adding mergeActionsList items when bms_is_member(rti,
estate->es_unpruned_relids) is false for all resultRelations. This
results in an empty actions list. Because the MERGE is performing a
LEFT JOIN for the NOT MATCHED, ExecMerge() gets a row and runs
ExecMergeNotMatched(), which crashes on "econtext->ecxt_scantuple =
NULL;" because of a NULL econtext. econtext is NULL because
ExecInitMerge() skips calling ExecAssignExprContext() when
mergeActionLists is empty.
There are a couple of ways I can see to fix this, 1) would be to move
the ExecAssignExprContext() above the "if (mergeActionLists == NIL)"
in ExecInitMerge(), or 2) add code to return NULL in
ExecMergeNotMatched() if actionStates is NULL.
I think maybe #1 is the better option as #2 adds additional code that
executes on every ExecMergeNotMatched() call. The patch does #1. We
should probably add your test case too.
David
Attachments:
ExecInitMerge_fix.patch (application/octet-stream, 646B) download
diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
index b0fe50075ad..c519f366a8f 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -3651,13 +3651,14 @@ ExecInitMerge(ModifyTableState *mtstate, EState *estate)
ListCell *lc;
int i;
+ if (mtstate->ps.ps_ExprContext == NULL)
+ ExecAssignExprContext(estate, &mtstate->ps);
+
if (mergeActionLists == NIL)
return;
mtstate->mt_merge_subcommands = 0;
- if (mtstate->ps.ps_ExprContext == NULL)
- ExecAssignExprContext(estate, &mtstate->ps);
econtext = mtstate->ps.ps_ExprContext;
/*
* Re: BUG #18830: ExecInitMerge Segfault on MERGE
2025-03-02 21:00 BUG #18830: ExecInitMerge Segfault on MERGE PG Bug reporting form <noreply@postgresql.org>
2025-03-03 15:46 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Tender Wang <tndrwang@gmail.com>
2025-03-04 06:58 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Tender Wang <tndrwang@gmail.com>
2025-03-04 09:29 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE David Rowley <dgrowleyml@gmail.com>
@ 2025-03-04 09:41 ` Amit Langote <amitlangote09@gmail.com>
2 siblings, 0 replies; 32+ messages in thread
From: Amit Langote @ 2025-03-04 09:41 UTC (permalink / raw)
To: David Rowley <dgrowleyml@gmail.com>; +Cc: Tender Wang <tndrwang@gmail.com>; pgsql-bugs@lists.postgresql.org; tharakan@gmail.com
On Tue, Mar 4, 2025 at 3:00 PM David Rowley <dgrowleyml@gmail.com> wrote:
> On Tue, 4 Mar 2025 at 19:58, Tender Wang <tndrwang@gmail.com> wrote:
> > I found the partition_prune.sql does not cover merge into ... not match case, and I found an easy reproduce step, seeing below:
> >
> > postgres=# merge into part_abc_view pt
> > using (select stable_one() + 2 as pid) as q join part_abc_1 pt1 on (true) on pt.a = stable_one() +2
> > when not matched then insert values(1, 'd', false);
> > server closed the connection unexpectedly
> > This probably means the server terminated abnormally
> > before or while processing the request.
> > The connection to the server was lost. Attempting reset: Succeeded.
>
> It looks like this is happening because ExecInitModifyTable() skips
> adding mergeActionsList items when bms_is_member(rti,
> estate->es_unpruned_relids) is false for all resultRelations. This
> results in an empty actions list. Because the MERGE is performing a
> LEFT JOIN for the NOT MATCHED, ExecMerge() gets a row and runs
> ExecMergeNotMatched(), which crashes on "econtext->ecxt_scantuple =
> NULL;" because of a NULL econtext. econtext is NULL because
> ExecInitMerge() skips calling ExecAssignExprContext() when
> mergeActionLists is empty.
>
> There are a couple of ways I can see to fix this, 1) would be to move
> the ExecAssignExprContext() above the "if (mergeActionLists == NIL)"
> in ExecInitMerge(), or 2) add code to return NULL in
> ExecMergeNotMatched() if actionStates is NULL.
>
> I think maybe #1 is the better option as #2 adds additional code that
> executes on every ExecMergeNotMatched() call. The patch does #1. We
> should probably add your test case too.
Thanks for taking a look and the patch. I was thinking #1 too.
Though I was thinking of doing the ps_ExprContext initialization in
ExecInitModifyTable() before calling ExecInitMerge(), to be similar to
what we do for other ModifyTable properties that need one.
--
Thanks, Amit Langote
^ permalink raw reply [nested|flat] 32+ messages in thread
* Re: BUG #18830: ExecInitMerge Segfault on MERGE
2025-03-02 21:00 BUG #18830: ExecInitMerge Segfault on MERGE PG Bug reporting form <noreply@postgresql.org>
2025-03-03 15:46 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Tender Wang <tndrwang@gmail.com>
2025-03-04 06:58 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Tender Wang <tndrwang@gmail.com>
2025-03-04 09:29 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE David Rowley <dgrowleyml@gmail.com>
@ 2025-03-04 09:45 ` Tender Wang <tndrwang@gmail.com>
2025-03-04 10:09 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Amit Langote <amitlangote09@gmail.com>
2 siblings, 1 reply; 32+ messages in thread
From: Tender Wang @ 2025-03-04 09:45 UTC (permalink / raw)
To: David Rowley <dgrowleyml@gmail.com>; +Cc: pgsql-bugs@lists.postgresql.org, Amit Langote <amitlangote09@gmail.com>; tharakan@gmail.com
David Rowley <dgrowleyml@gmail.com> 于2025年3月4日周二 17:30写道：
> On Tue, 4 Mar 2025 at 19:58, Tender Wang <tndrwang@gmail.com> wrote:
> > I found the partition_prune.sql does not cover merge into ... not match
> case, and I found an easy reproduce step, seeing below:
> >
> > postgres=# merge into part_abc_view pt
> > using (select stable_one() + 2 as pid) as q join part_abc_1 pt1 on
> (true) on pt.a = stable_one() +2
> > when not matched then insert values(1, 'd', false);
> > server closed the connection unexpectedly
> > This probably means the server terminated abnormally
> > before or while processing the request.
> > The connection to the server was lost. Attempting reset: Succeeded.
>
> It looks like this is happening because ExecInitModifyTable() skips
> adding mergeActionsList items when bms_is_member(rti,
> estate->es_unpruned_relids) is false for all resultRelations. This
> results in an empty actions list. Because the MERGE is performing a
> LEFT JOIN for the NOT MATCHED, ExecMerge() gets a row and runs
> ExecMergeNotMatched(), which crashes on "econtext->ecxt_scantuple =
> NULL;" because of a NULL econtext. econtext is NULL because
> ExecInitMerge() skips calling ExecAssignExprContext() when
> mergeActionLists is empty.
>
> There are a couple of ways I can see to fix this, 1) would be to move
> the ExecAssignExprContext() above the "if (mergeActionLists == NIL)"
> in ExecInitMerge(), or 2) add code to return NULL in
> ExecMergeNotMatched() if actionStates is NULL.
>
> I think maybe #1 is the better option as #2 adds additional code that
> executes on every ExecMergeNotMatched() call. The patch does #1. We
> should probably add your test case too.
>
>
Hmm, apply your patch, I get different results when set
enable_partition_pruning = off, seeing below:
postgres=# merge into part_abc_view pt
using (select stable_one() + 2 as pid) as q join part_abc_1 pt1 on (true)
on pt.a = stable_one() +2
when not matched then insert values(1, 'd', false);
MERGE 0
postgres=# set enable_partition_pruning = off;
SET
postgres=# merge into part_abc_view pt
using (select stable_one() + 2 as pid) as q join part_abc_1 pt1 on (true)
on pt.a = stable_one() +2
when not matched then insert values(1, 'd', false);
MERGE 1
if all partitions are pruned, the resultRelation's mergeActions has no
chance to be set.
--
Thanks,
Tender Wang
^ permalink raw reply [nested|flat] 32+ messages in thread
* Re: BUG #18830: ExecInitMerge Segfault on MERGE
2025-03-02 21:00 BUG #18830: ExecInitMerge Segfault on MERGE PG Bug reporting form <noreply@postgresql.org>
2025-03-03 15:46 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Tender Wang <tndrwang@gmail.com>
2025-03-04 06:58 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Tender Wang <tndrwang@gmail.com>
2025-03-04 09:29 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE David Rowley <dgrowleyml@gmail.com>
2025-03-04 09:45 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Tender Wang <tndrwang@gmail.com>
@ 2025-03-04 10:09 ` Amit Langote <amitlangote09@gmail.com>
2025-03-04 10:19 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Tender Wang <tndrwang@gmail.com>
2025-03-04 10:27 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Tender Wang <tndrwang@gmail.com>
2025-03-04 11:06 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Tender Wang <tndrwang@gmail.com>
0 siblings, 3 replies; 32+ messages in thread
From: Amit Langote @ 2025-03-04 10:09 UTC (permalink / raw)
To: Tender Wang <tndrwang@gmail.com>; +Cc: David Rowley <dgrowleyml@gmail.com>; pgsql-bugs@lists.postgresql.org; tharakan@gmail.com
On Tue, Mar 4, 2025 at 3:16 PM Tender Wang <tndrwang@gmail.com> wrote:
> David Rowley <dgrowleyml@gmail.com> 于2025年3月4日周二 17:30写道：
>>
>> On Tue, 4 Mar 2025 at 19:58, Tender Wang <tndrwang@gmail.com> wrote:
>> > I found the partition_prune.sql does not cover merge into ... not match case, and I found an easy reproduce step, seeing below:
>> >
>> > postgres=# merge into part_abc_view pt
>> > using (select stable_one() + 2 as pid) as q join part_abc_1 pt1 on (true) on pt.a = stable_one() +2
>> > when not matched then insert values(1, 'd', false);
>> > server closed the connection unexpectedly
>> > This probably means the server terminated abnormally
>> > before or while processing the request.
>> > The connection to the server was lost. Attempting reset: Succeeded.
>>
>> It looks like this is happening because ExecInitModifyTable() skips
>> adding mergeActionsList items when bms_is_member(rti,
>> estate->es_unpruned_relids) is false for all resultRelations. This
>> results in an empty actions list. Because the MERGE is performing a
>> LEFT JOIN for the NOT MATCHED, ExecMerge() gets a row and runs
>> ExecMergeNotMatched(), which crashes on "econtext->ecxt_scantuple =
>> NULL;" because of a NULL econtext. econtext is NULL because
>> ExecInitMerge() skips calling ExecAssignExprContext() when
>> mergeActionLists is empty.
>>
>> There are a couple of ways I can see to fix this, 1) would be to move
>> the ExecAssignExprContext() above the "if (mergeActionLists == NIL)"
>> in ExecInitMerge(), or 2) add code to return NULL in
>> ExecMergeNotMatched() if actionStates is NULL.
>>
>> I think maybe #1 is the better option as #2 adds additional code that
>> executes on every ExecMergeNotMatched() call. The patch does #1. We
>> should probably add your test case too.
>>
>
> Hmm, apply your patch, I get different results when set enable_partition_pruning = off, seeing below:
> postgres=# merge into part_abc_view pt
> using (select stable_one() + 2 as pid) as q join part_abc_1 pt1 on (true)
> on pt.a = stable_one() +2
> when not matched then insert values(1, 'd', false);
> MERGE 0
> postgres=# set enable_partition_pruning = off;
> SET
> postgres=# merge into part_abc_view pt
> using (select stable_one() + 2 as pid) as q join part_abc_1 pt1 on (true)
> on pt.a = stable_one() +2
> when not matched then insert values(1, 'd', false);
> MERGE 1
Hmm, interesting. Can you share the full test case?
What's the behavior on v17 and older? Just want to be sure if we're
looking at another bug in the code committed in v18.
--
Thanks, Amit Langote
^ permalink raw reply [nested|flat] 32+ messages in thread
* Re: BUG #18830: ExecInitMerge Segfault on MERGE
2025-03-02 21:00 BUG #18830: ExecInitMerge Segfault on MERGE PG Bug reporting form <noreply@postgresql.org>
2025-03-03 15:46 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Tender Wang <tndrwang@gmail.com>
2025-03-04 06:58 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Tender Wang <tndrwang@gmail.com>
2025-03-04 09:29 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE David Rowley <dgrowleyml@gmail.com>
2025-03-04 09:45 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Tender Wang <tndrwang@gmail.com>
2025-03-04 10:09 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Amit Langote <amitlangote09@gmail.com>
@ 2025-03-04 10:19 ` Tender Wang <tndrwang@gmail.com>
2 siblings, 0 replies; 32+ messages in thread
From: Tender Wang @ 2025-03-04 10:19 UTC (permalink / raw)
To: Amit Langote <amitlangote09@gmail.com>; +Cc: David Rowley <dgrowleyml@gmail.com>; pgsql-bugs@lists.postgresql.org; tharakan@gmail.com
Amit Langote <amitlangote09@gmail.com> 于2025年3月4日周二 18:10写道：
> On Tue, Mar 4, 2025 at 3:16 PM Tender Wang <tndrwang@gmail.com> wrote:
> > David Rowley <dgrowleyml@gmail.com> 于2025年3月4日周二 17:30写道：
> >>
> >> On Tue, 4 Mar 2025 at 19:58, Tender Wang <tndrwang@gmail.com> wrote:
> >> > I found the partition_prune.sql does not cover merge into ... not
> match case, and I found an easy reproduce step, seeing below:
> >> >
> >> > postgres=# merge into part_abc_view pt
> >> > using (select stable_one() + 2 as pid) as q join part_abc_1 pt1 on
> (true) on pt.a = stable_one() +2
> >> > when not matched then insert values(1, 'd', false);
> >> > server closed the connection unexpectedly
> >> > This probably means the server terminated abnormally
> >> > before or while processing the request.
> >> > The connection to the server was lost. Attempting reset: Succeeded.
> >>
> >> It looks like this is happening because ExecInitModifyTable() skips
> >> adding mergeActionsList items when bms_is_member(rti,
> >> estate->es_unpruned_relids) is false for all resultRelations. This
> >> results in an empty actions list. Because the MERGE is performing a
> >> LEFT JOIN for the NOT MATCHED, ExecMerge() gets a row and runs
> >> ExecMergeNotMatched(), which crashes on "econtext->ecxt_scantuple =
> >> NULL;" because of a NULL econtext. econtext is NULL because
> >> ExecInitMerge() skips calling ExecAssignExprContext() when
> >> mergeActionLists is empty.
> >>
> >> There are a couple of ways I can see to fix this, 1) would be to move
> >> the ExecAssignExprContext() above the "if (mergeActionLists == NIL)"
> >> in ExecInitMerge(), or 2) add code to return NULL in
> >> ExecMergeNotMatched() if actionStates is NULL.
> >>
> >> I think maybe #1 is the better option as #2 adds additional code that
> >> executes on every ExecMergeNotMatched() call. The patch does #1. We
> >> should probably add your test case too.
> >>
> >
> > Hmm, apply your patch, I get different results when set
> enable_partition_pruning = off, seeing below:
> > postgres=# merge into part_abc_view pt
> > using (select stable_one() + 2 as pid) as q join part_abc_1 pt1 on (true)
> > on pt.a = stable_one() +2
> > when not matched then insert values(1, 'd', false);
> > MERGE 0
> > postgres=# set enable_partition_pruning = off;
> > SET
> > postgres=# merge into part_abc_view pt
> > using (select stable_one() + 2 as pid) as q join part_abc_1 pt1 on (true)
> > on pt.a = stable_one() +2
> > when not matched then insert values(1, 'd', false);
> > MERGE 1
>
> Hmm, interesting. Can you share the full test case?
>
> What's the behavior on v17 and older? Just want to be sure if we're
> looking at another bug in the code committed in v18.
>
> The test case comes from partition_prune.sql, but now we don't cover merge
.. not match.
create table part_abc (a int, b text, c bool) partition by list (a);
create table part_abc_1 (b text, a int, c bool);
create table part_abc_2 (a int, c bool, b text);
alter table part_abc attach partition part_abc_1 for values in (1);
alter table part_abc attach partition part_abc_2 for values in (2);
insert into part_abc values (1, 'b', true);
insert into part_abc values (2, 'c', true);
create view part_abc_view as select * from part_abc where b <> 'a' with
check option;
create function stable_one() returns int as $$ begin return 1; end; $$
language plpgsql stable;
merge into part_abc_view pt
using (select stable_one() + 2 as pid) as q join part_abc_1 pt1 on (true)
on pt.a = stable_one() +2
when not matched then insert values(1, 'd', false);
--
Thanks,
Tender Wang
^ permalink raw reply [nested|flat] 32+ messages in thread
* Re: BUG #18830: ExecInitMerge Segfault on MERGE
2025-03-02 21:00 BUG #18830: ExecInitMerge Segfault on MERGE PG Bug reporting form <noreply@postgresql.org>
2025-03-03 15:46 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Tender Wang <tndrwang@gmail.com>
2025-03-04 06:58 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Tender Wang <tndrwang@gmail.com>
2025-03-04 09:29 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE David Rowley <dgrowleyml@gmail.com>
2025-03-04 09:45 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Tender Wang <tndrwang@gmail.com>
2025-03-04 10:09 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Amit Langote <amitlangote09@gmail.com>
@ 2025-03-04 10:27 ` Tender Wang <tndrwang@gmail.com>
2 siblings, 0 replies; 32+ messages in thread
From: Tender Wang @ 2025-03-04 10:27 UTC (permalink / raw)
To: Amit Langote <amitlangote09@gmail.com>; +Cc: David Rowley <dgrowleyml@gmail.com>; pgsql-bugs@lists.postgresql.org; tharakan@gmail.com
Amit Langote <amitlangote09@gmail.com> 于2025年3月4日周二 18:10写道：
>
>
> Hmm, interesting. Can you share the full test case?
>
> What's the behavior on v17 and older? Just want to be sure if we're
> looking at another bug in the code committed in v18.
>
>
I do test on 17.1, the results are consistent. See below:
psql (17.1)
Type "help" for help.
postgres=# create table part_abc (a int, b text, c bool) partition by list
(a);
create table part_abc_1 (b text, a int, c bool);
create table part_abc_2 (a int, c bool, b text);
alter table part_abc attach partition part_abc_1 for values in (1);
alter table part_abc attach partition part_abc_2 for values in (2);
insert into part_abc values (1, 'b', true);
insert into part_abc values (2, 'c', true);
create view part_abc_view as select * from part_abc where b <> 'a' with
check option;
create function stable_one() returns int as $$ begin return 1; end; $$
language plpgsql stable;
merge into part_abc_view pt
using (select stable_one() + 2 as pid) as q join part_abc_1 pt1 on (true)
on pt.a = stable_one() +2
when not matched then insert values(1, 'd', false);
CREATE TABLE
CREATE TABLE
CREATE TABLE
ALTER TABLE
ALTER TABLE
INSERT 0 1
INSERT 0 1
CREATE VIEW
CREATE FUNCTION
MERGE 1
postgres=# show enable_partition_pruning ;
enable_partition_pruning
--------------------------
on
(1 row)
postgres=# create database test;
CREATE DATABASE
postgres=# \c test
You are now connected to database "test" as user "ecs-user".
test=# set enable_partition_pruning = off;
SET
test=# show enable_partition_pruning ;
enable_partition_pruning
--------------------------
off
(1 row)
test=# create table part_abc (a int, b text, c bool) partition by list (a);
create table part_abc_1 (b text, a int, c bool);
create table part_abc_2 (a int, c bool, b text);
alter table part_abc attach partition part_abc_1 for values in (1);
alter table part_abc attach partition part_abc_2 for values in (2);
insert into part_abc values (1, 'b', true);
insert into part_abc values (2, 'c', true);
create view part_abc_view as select * from part_abc where b <> 'a' with
check option;
create function stable_one() returns int as $$ begin return 1; end; $$
language plpgsql stable;
merge into part_abc_view pt
using (select stable_one() + 2 as pid) as q join part_abc_1 pt1 on (true)
on pt.a = stable_one() +2
when not matched then insert values(1, 'd', false);
CREATE TABLE
CREATE TABLE
CREATE TABLE
ALTER TABLE
ALTER TABLE
INSERT 0 1
INSERT 0 1
CREATE VIEW
CREATE FUNCTION
MERGE 1
--
Thanks,
Tender Wang
^ permalink raw reply [nested|flat] 32+ messages in thread
* Re: BUG #18830: ExecInitMerge Segfault on MERGE
2025-03-02 21:00 BUG #18830: ExecInitMerge Segfault on MERGE PG Bug reporting form <noreply@postgresql.org>
2025-03-03 15:46 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Tender Wang <tndrwang@gmail.com>
2025-03-04 06:58 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Tender Wang <tndrwang@gmail.com>
2025-03-04 09:29 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE David Rowley <dgrowleyml@gmail.com>
2025-03-04 09:45 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Tender Wang <tndrwang@gmail.com>
2025-03-04 10:09 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Amit Langote <amitlangote09@gmail.com>
@ 2025-03-04 11:06 ` Tender Wang <tndrwang@gmail.com>
2025-03-04 11:32 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Amit Langote <amitlangote09@gmail.com>
2 siblings, 1 reply; 32+ messages in thread
From: Tender Wang @ 2025-03-04 11:06 UTC (permalink / raw)
To: Amit Langote <amitlangote09@gmail.com>; +Cc: David Rowley <dgrowleyml@gmail.com>; pgsql-bugs@lists.postgresql.org; tharakan@gmail.com
Amit Langote <amitlangote09@gmail.com> 于2025年3月4日周二 18:10写道：
> On Tue, Mar 4, 2025 at 3:16 PM Tender Wang <tndrwang@gmail.com> wrote:
> > David Rowley <dgrowleyml@gmail.com> 于2025年3月4日周二 17:30写道：
> >>
> >> On Tue, 4 Mar 2025 at 19:58, Tender Wang <tndrwang@gmail.com> wrote:
> >> > I found the partition_prune.sql does not cover merge into ... not
> match case, and I found an easy reproduce step, seeing below:
> >> >
> >> > postgres=# merge into part_abc_view pt
> >> > using (select stable_one() + 2 as pid) as q join part_abc_1 pt1 on
> (true) on pt.a = stable_one() +2
> >> > when not matched then insert values(1, 'd', false);
> >> > server closed the connection unexpectedly
> >> > This probably means the server terminated abnormally
> >> > before or while processing the request.
> >> > The connection to the server was lost. Attempting reset: Succeeded.
> >>
> >> It looks like this is happening because ExecInitModifyTable() skips
> >> adding mergeActionsList items when bms_is_member(rti,
> >> estate->es_unpruned_relids) is false for all resultRelations. This
> >> results in an empty actions list. Because the MERGE is performing a
> >> LEFT JOIN for the NOT MATCHED, ExecMerge() gets a row and runs
> >> ExecMergeNotMatched(), which crashes on "econtext->ecxt_scantuple =
> >> NULL;" because of a NULL econtext. econtext is NULL because
> >> ExecInitMerge() skips calling ExecAssignExprContext() when
> >> mergeActionLists is empty.
> >>
> >> There are a couple of ways I can see to fix this, 1) would be to move
> >> the ExecAssignExprContext() above the "if (mergeActionLists == NIL)"
> >> in ExecInitMerge(), or 2) add code to return NULL in
> >> ExecMergeNotMatched() if actionStates is NULL.
> >>
> >> I think maybe #1 is the better option as #2 adds additional code that
> >> executes on every ExecMergeNotMatched() call. The patch does #1. We
> >> should probably add your test case too.
> >>
> >
> > Hmm, apply your patch, I get different results when set
> enable_partition_pruning = off, seeing below:
> > postgres=# merge into part_abc_view pt
> > using (select stable_one() + 2 as pid) as q join part_abc_1 pt1 on (true)
> > on pt.a = stable_one() +2
> > when not matched then insert values(1, 'd', false);
> > MERGE 0
> > postgres=# set enable_partition_pruning = off;
> > SET
> > postgres=# merge into part_abc_view pt
> > using (select stable_one() + 2 as pid) as q join part_abc_1 pt1 on (true)
> > on pt.a = stable_one() +2
> > when not matched then insert values(1, 'd', false);
> > MERGE 1
>
> Hmm, interesting. Can you share the full test case?
>
> What's the behavior on v17 and older? Just want to be sure if we're
> looking at another bug in the code committed in v18.
>
Because all partitions are pruned, so bms_is_member(rti,
estate->es_unpruned_relids) is false, then mergeActionLists is empty.
Even though we initialize econtext to not null, but
in ExecMergeNotMatched(),
because actionStates list is empty, so no merge operation happens.
actionStates is empty due to empty mergeActionLists.
--
Thanks,
Tender Wang
^ permalink raw reply [nested|flat] 32+ messages in thread
* Re: BUG #18830: ExecInitMerge Segfault on MERGE
2025-03-02 21:00 BUG #18830: ExecInitMerge Segfault on MERGE PG Bug reporting form <noreply@postgresql.org>
2025-03-03 15:46 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Tender Wang <tndrwang@gmail.com>
2025-03-04 06:58 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Tender Wang <tndrwang@gmail.com>
2025-03-04 09:29 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE David Rowley <dgrowleyml@gmail.com>
2025-03-04 09:45 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Tender Wang <tndrwang@gmail.com>
2025-03-04 10:09 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Amit Langote <amitlangote09@gmail.com>
2025-03-04 11:06 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Tender Wang <tndrwang@gmail.com>
@ 2025-03-04 11:32 ` Amit Langote <amitlangote09@gmail.com>
0 siblings, 0 replies; 32+ messages in thread
From: Amit Langote @ 2025-03-04 11:32 UTC (permalink / raw)
To: Tender Wang <tndrwang@gmail.com>; +Cc: David Rowley <dgrowleyml@gmail.com>; pgsql-bugs@lists.postgresql.org; tharakan@gmail.com
On Tue, Mar 4, 2025 at 4:36 PM Tender Wang <tndrwang@gmail.com> wrote:
> Amit Langote <amitlangote09@gmail.com> 于2025年3月4日周二 18:10写道：
>> On Tue, Mar 4, 2025 at 3:16 PM Tender Wang <tndrwang@gmail.com> wrote:
>> > David Rowley <dgrowleyml@gmail.com> 于2025年3月4日周二 17:30写道：
>> >> There are a couple of ways I can see to fix this, 1) would be to move
>> >> the ExecAssignExprContext() above the "if (mergeActionLists == NIL)"
>> >> in ExecInitMerge(), or 2) add code to return NULL in
>> >> ExecMergeNotMatched() if actionStates is NULL.
>> >>
>> >> I think maybe #1 is the better option as #2 adds additional code that
>> >> executes on every ExecMergeNotMatched() call. The patch does #1. We
>> >> should probably add your test case too.
>> >>
>> > Hmm, apply your patch, I get different results when set enable_partition_pruning = off, seeing below:
>> > postgres=# merge into part_abc_view pt
>> > using (select stable_one() + 2 as pid) as q join part_abc_1 pt1 on (true)
>> > on pt.a = stable_one() +2
>> > when not matched then insert values(1, 'd', false);
>> > MERGE 0
>> > postgres=# set enable_partition_pruning = off;
>> > SET
>> > postgres=# merge into part_abc_view pt
>> > using (select stable_one() + 2 as pid) as q join part_abc_1 pt1 on (true)
>> > on pt.a = stable_one() +2
>> > when not matched then insert values(1, 'd', false);
>> > MERGE 1
>>
>> Hmm, interesting. Can you share the full test case?
>>
>> What's the behavior on v17 and older? Just want to be sure if we're
>> looking at another bug in the code committed in v18.
>
>
> Because all partitions are pruned, so bms_is_member(rti,
> estate->es_unpruned_relids) is false, then mergeActionLists is empty.
> Even though we initialize econtext to not null, but in ExecMergeNotMatched(),
> because actionStates list is empty, so no merge operation happens.
> actionStates is empty due to empty mergeActionLists.
Yes, I get that. What I am wondering about is whether the case you
presented where disabling partition pruning gives a different result
for the same query has anything to do with this particular bug that's
causing the crash Robins reported. If there's a separate bug, we have
to check if it is present in v17 and older.
--
Thanks, Amit Langote
^ permalink raw reply [nested|flat] 32+ messages in thread
* Re: BUG #18830: ExecInitMerge Segfault on MERGE
2025-03-02 21:00 BUG #18830: ExecInitMerge Segfault on MERGE PG Bug reporting form <noreply@postgresql.org>
2025-03-03 15:46 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Tender Wang <tndrwang@gmail.com>
2025-03-04 06:58 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Tender Wang <tndrwang@gmail.com>
2025-03-04 09:29 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE David Rowley <dgrowleyml@gmail.com>
@ 2025-03-04 18:56 ` Dean Rasheed <dean.a.rasheed@gmail.com>
2025-03-05 01:26 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Tender Wang <tndrwang@gmail.com>
2025-03-12 12:24 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Amit Langote <amitlangote09@gmail.com>
2 siblings, 2 replies; 32+ messages in thread
From: Dean Rasheed @ 2025-03-04 18:56 UTC (permalink / raw)
To: David Rowley <dgrowleyml@gmail.com>; +Cc: Tender Wang <tndrwang@gmail.com>; pgsql-bugs@lists.postgresql.org, Amit Langote <amitlangote09@gmail.com>; tharakan@gmail.com
On Tue, 4 Mar 2025 at 09:30, David Rowley <dgrowleyml@gmail.com> wrote:
>
> It looks like this is happening because ExecInitModifyTable() skips
> adding mergeActionsList items when bms_is_member(rti,
> estate->es_unpruned_relids) is false for all resultRelations. This
> results in an empty actions list. Because the MERGE is performing a
> LEFT JOIN for the NOT MATCHED, ExecMerge() gets a row and runs
> ExecMergeNotMatched(), which crashes on "econtext->ecxt_scantuple =
> NULL;" because of a NULL econtext. econtext is NULL because
> ExecInitMerge() skips calling ExecAssignExprContext() when
> mergeActionLists is empty.
>
> There are a couple of ways I can see to fix this, 1) would be to move
> the ExecAssignExprContext() above the "if (mergeActionLists == NIL)"
> in ExecInitMerge(), or 2) add code to return NULL in
> ExecMergeNotMatched() if actionStates is NULL.
Hmm, I don't think that's right. I think this is just masking the problem.
I think the real problem is that, as things stand,
ExecInitModifyTable() must never be allowed to prune every leaf
partition, because there are multiple places that rely on there being
at least one resultRelInfo.
For example, ExecModifyTable() passes the first resultRelInfo to
ExecMerge() and ExecMergeNotMatched() in the NOT MATCHED case, and
those use the action lists in that resultRelInfo to work out what
action to execute. The fact that it didn't crash with this patch is
probably just luck, because it's passing a pointer to uninitialised
memory, but it's still doing the wrong thing by not invoking any merge
actions.
Similarly, if MERGE does an INSERT on a partitioned table, the code in
ExecInitPartitionInfo() from ExecFindPartition() assumes that
mtstate->resultRelInfo has at least one entry.
So I think the simplest solution is to arrange for
ExecInitModifyTable() to always include details of at least one result
relation, adding at least one entry to each of the lists. As an
alternative, it might be possible to make the executor cope with an
empty resultRelInfo array (by using the root resultRelInfo instead,
where one is needed), but I think that would be a bigger change.
Regards,
Dean
^ permalink raw reply [nested|flat] 32+ messages in thread
* Re: BUG #18830: ExecInitMerge Segfault on MERGE
2025-03-02 21:00 BUG #18830: ExecInitMerge Segfault on MERGE PG Bug reporting form <noreply@postgresql.org>
2025-03-03 15:46 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Tender Wang <tndrwang@gmail.com>
2025-03-04 06:58 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Tender Wang <tndrwang@gmail.com>
2025-03-04 09:29 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE David Rowley <dgrowleyml@gmail.com>
2025-03-04 18:56 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Dean Rasheed <dean.a.rasheed@gmail.com>
@ 2025-03-05 01:26 ` Tender Wang <tndrwang@gmail.com>
1 sibling, 0 replies; 32+ messages in thread
From: Tender Wang @ 2025-03-05 01:26 UTC (permalink / raw)
To: Dean Rasheed <dean.a.rasheed@gmail.com>; +Cc: David Rowley <dgrowleyml@gmail.com>; pgsql-bugs@lists.postgresql.org, Amit Langote <amitlangote09@gmail.com>; tharakan@gmail.com
Dean Rasheed <dean.a.rasheed@gmail.com> 于2025年3月5日周三 02:56写道：
> On Tue, 4 Mar 2025 at 09:30, David Rowley <dgrowleyml@gmail.com> wrote:
> >
> > It looks like this is happening because ExecInitModifyTable() skips
> > adding mergeActionsList items when bms_is_member(rti,
> > estate->es_unpruned_relids) is false for all resultRelations. This
> > results in an empty actions list. Because the MERGE is performing a
> > LEFT JOIN for the NOT MATCHED, ExecMerge() gets a row and runs
> > ExecMergeNotMatched(), which crashes on "econtext->ecxt_scantuple =
> > NULL;" because of a NULL econtext. econtext is NULL because
> > ExecInitMerge() skips calling ExecAssignExprContext() when
> > mergeActionLists is empty.
> >
> > There are a couple of ways I can see to fix this, 1) would be to move
> > the ExecAssignExprContext() above the "if (mergeActionLists == NIL)"
> > in ExecInitMerge(), or 2) add code to return NULL in
> > ExecMergeNotMatched() if actionStates is NULL.
>
> Hmm, I don't think that's right. I think this is just masking the problem.
>
> I think the real problem is that, as things stand,
> ExecInitModifyTable() must never be allowed to prune every leaf
> partition, because there are multiple places that rely on there being
> at least one resultRelInfo.
>
> For example, ExecModifyTable() passes the first resultRelInfo to
> ExecMerge() and ExecMergeNotMatched() in the NOT MATCHED case, and
> those use the action lists in that resultRelInfo to work out what
> action to execute. The fact that it didn't crash with this patch is
> probably just luck, because it's passing a pointer to uninitialised
> memory, but it's still doing the wrong thing by not invoking any merge
> actions.
>
Yeah, I agree with you. As I said before, this fix will get a wrong result
compared to
enable_partition_pruning = off, even though no crash happened again.
At least for NOT MATCH, we must have resultRelInfo entry.
> Similarly, if MERGE does an INSERT on a partitioned table, the code in
> ExecInitPartitionInfo() from ExecFindPartition() assumes that
> mtstate->resultRelInfo has at least one entry.
>
> So I think the simplest solution is to arrange for
> ExecInitModifyTable() to always include details of at least one result
> relation, adding at least one entry to each of the lists.
I have not tried this way. I'm not sure about this.
As an
> alternative, it might be possible to make the executor cope with an
> empty resultRelInfo array (by using the root resultRelInfo instead,
> where one is needed), but I think that would be a bigger change.
>
Yeah, this way changes a lot of codes.
--
Thanks,
Tender Wang
^ permalink raw reply [nested|flat] 32+ messages in thread
* Re: BUG #18830: ExecInitMerge Segfault on MERGE
2025-03-02 21:00 BUG #18830: ExecInitMerge Segfault on MERGE PG Bug reporting form <noreply@postgresql.org>
2025-03-03 15:46 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Tender Wang <tndrwang@gmail.com>
2025-03-04 06:58 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Tender Wang <tndrwang@gmail.com>
2025-03-04 09:29 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE David Rowley <dgrowleyml@gmail.com>
2025-03-04 18:56 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Dean Rasheed <dean.a.rasheed@gmail.com>
@ 2025-03-12 12:24 ` Amit Langote <amitlangote09@gmail.com>
2025-03-12 13:24 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Tender Wang <tndrwang@gmail.com>
2025-03-12 14:59 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Dean Rasheed <dean.a.rasheed@gmail.com>
1 sibling, 2 replies; 32+ messages in thread
From: Amit Langote @ 2025-03-12 12:24 UTC (permalink / raw)
To: Dean Rasheed <dean.a.rasheed@gmail.com>; +Cc: David Rowley <dgrowleyml@gmail.com>; Tender Wang <tndrwang@gmail.com>; pgsql-bugs@lists.postgresql.org; tharakan@gmail.com
On Wed, Mar 5, 2025 at 3:56 AM Dean Rasheed <dean.a.rasheed@gmail.com> wrote:
> On Tue, 4 Mar 2025 at 09:30, David Rowley <dgrowleyml@gmail.com> wrote:
> >
> > It looks like this is happening because ExecInitModifyTable() skips
> > adding mergeActionsList items when bms_is_member(rti,
> > estate->es_unpruned_relids) is false for all resultRelations. This
> > results in an empty actions list. Because the MERGE is performing a
> > LEFT JOIN for the NOT MATCHED, ExecMerge() gets a row and runs
> > ExecMergeNotMatched(), which crashes on "econtext->ecxt_scantuple =
> > NULL;" because of a NULL econtext. econtext is NULL because
> > ExecInitMerge() skips calling ExecAssignExprContext() when
> > mergeActionLists is empty.
> >
> > There are a couple of ways I can see to fix this, 1) would be to move
> > the ExecAssignExprContext() above the "if (mergeActionLists == NIL)"
> > in ExecInitMerge(), or 2) add code to return NULL in
> > ExecMergeNotMatched() if actionStates is NULL.
>
> Hmm, I don't think that's right. I think this is just masking the problem.
>
> I think the real problem is that, as things stand,
> ExecInitModifyTable() must never be allowed to prune every leaf
> partition, because there are multiple places that rely on there being
> at least one resultRelInfo.
>
> For example, ExecModifyTable() passes the first resultRelInfo to
> ExecMerge() and ExecMergeNotMatched() in the NOT MATCHED case, and
> those use the action lists in that resultRelInfo to work out what
> action to execute. The fact that it didn't crash with this patch is
> probably just luck, because it's passing a pointer to uninitialised
> memory, but it's still doing the wrong thing by not invoking any merge
> actions.
>
> Similarly, if MERGE does an INSERT on a partitioned table, the code in
> ExecInitPartitionInfo() from ExecFindPartition() assumes that
> mtstate->resultRelInfo has at least one entry.
>
> So I think the simplest solution is to arrange for
> ExecInitModifyTable() to always include details of at least one result
> relation, adding at least one entry to each of the lists.
Thanks -- I’ve studied the code and I agree with the conclusion: when
all result relations are pruned, we still need to lock and process the
first one to preserve executor invariants.
Your examples with ExecMerge() and ExecInitPartitionInfo() make the
consequences of missing resultRelInfo[0] pretty clear. Locking and
including the first result relation, even if pruned, seems like the
most straightforward way to maintain correctness without deeper
structural changes.
I've come up with the attached. I'll still need to add a test case.
--
Thanks, Amit Langote
Attachments:
v1-0001-Ensure-first-result-relation-is-included-even-if-.patch (application/octet-stream, 4.2K) download
From 20b1c4c6c7206e364f169cc30b3bdc1b0114eb1b Mon Sep 17 00:00:00 2001
From: Amit Langote <amitlan@postgresql.org>
Date: Wed, 12 Mar 2025 21:10:31 +0900
Subject: [PATCH v1] Ensure first result relation is included even if pruned
This started as an investigation into crashes and misbehavior in MERGE
statements when all target partitions were pruned. These issues became
possible after commit cbc127917e, which introduced tracking of
unpruned relids to avoid processing pruned relations.
Some executor code paths rely on ModifyTableState.resultRelInfo[0] being
present and initialized, even if no result relations remain after
pruning. For example, ExecMerge() and ExecMergeNotMatched() use the first
resultRelInfo to determine the appropriate action. Similarly,
ExecInitPartitionInfo() assumes at least one result relation exists.
To preserve these assumptions, ExecDoInitialPruning() now ensures the
first result relation is locked and marked as unpruned, and
ExecInitModifyTable() includes it in the initialized result relation list
even if it was pruned.
This avoids crashes and undefined behavior while allowing pruning of
other result relations to proceed as before.
Bug: #18830
Reported-by: Robins Tharakan <tharakan@gmail.com>
Diagnozed-by: Tender Wang <tndrwang@gmail.com>
Diagnozed-by: Dean Rasheed <dean.a.rasheed@gmail.com>
Suggested-by: Dean Rasheed <dean.a.rasheed@gmail.com>
Discussion: https://postgr.es/m/18830-1f31ea1dc930d444%40postgresql.org
---
src/backend/executor/execPartition.c | 23 +++++++++++++++++++++++
src/backend/executor/nodeModifyTable.c | 12 +++++++++++-
2 files changed, 34 insertions(+), 1 deletion(-)
diff --git a/src/backend/executor/execPartition.c b/src/backend/executor/execPartition.c
index 5cd5e2eeb80..c93c2afad03 100644
--- a/src/backend/executor/execPartition.c
+++ b/src/backend/executor/execPartition.c
@@ -1819,6 +1819,7 @@ adjust_partition_colnos_using_map(List *colnos, AttrMap *attrMap)
void
ExecDoInitialPruning(EState *estate)
{
+ PlannedStmt *stmt = estate->es_plannedstmt;
ListCell *lc;
List *locked_relids = NIL;
@@ -1867,6 +1868,28 @@ ExecDoInitialPruning(EState *estate)
validsubplans);
}
+ /*
+ * Lock the first result relation even if it was pruned. Some executor
+ * paths (e.g., in nodeModifyTable.c and execPartition.c) expect this
+ * relation to be locked regardless of pruning. Also add it to
+ * es_unpruned_relids so that ExecInitModifyTable() processes it.
+ */
+ if (stmt->resultRelations)
+ {
+ Index firstResultRel = linitial_int(stmt->resultRelations);
+
+ if (!bms_is_member(firstResultRel, estate->es_unpruned_relids))
+ {
+ RangeTblEntry *rte = exec_rt_fetch(firstResultRel, estate);
+
+ Assert(rte->rtekind == RTE_RELATION && rte->rellockmode != NoLock);
+ LockRelationOid(rte->relid, rte->rellockmode);
+ locked_relids = lappend_int(locked_relids, firstResultRel);
+ estate->es_unpruned_relids = bms_add_member(estate->es_unpruned_relids,
+ firstResultRel);
+ }
+ }
+
/*
* Release the useless locks if the plan won't be executed. This is the
* same as what CheckCachedPlan() in plancache.c does.
diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
index a9142796743..bcc80a79046 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -4495,7 +4495,16 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
{
Index rti = lfirst_int(l);
- if (bms_is_member(rti, estate->es_unpruned_relids))
+ /*
+ * The first result relation might have been pruned, but we still need
+ * to include it to support executor code paths that access
+ * ModifyTableState.resultRelInfo[0], e.g., for evaluating result
+ * relation expressions.
+ *
+ * ExecDoInitialPruning() already locked it and added it to
+ * es_unpruned_relids.
+ */
+ if (i == 0 || bms_is_member(rti, estate->es_unpruned_relids))
{
resultRelations = lappend_int(resultRelations, rti);
if (node->withCheckOptionLists)
@@ -4536,6 +4545,7 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
i++;
}
nrels = list_length(resultRelations);
+ Assert (nrels > 0);
/*
* create state structure
--
2.43.0
* Re: BUG #18830: ExecInitMerge Segfault on MERGE
2025-03-02 21:00 BUG #18830: ExecInitMerge Segfault on MERGE PG Bug reporting form <noreply@postgresql.org>
2025-03-03 15:46 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Tender Wang <tndrwang@gmail.com>
2025-03-04 06:58 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Tender Wang <tndrwang@gmail.com>
2025-03-04 09:29 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE David Rowley <dgrowleyml@gmail.com>
2025-03-04 18:56 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Dean Rasheed <dean.a.rasheed@gmail.com>
2025-03-12 12:24 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Amit Langote <amitlangote09@gmail.com>
@ 2025-03-12 13:24 ` Tender Wang <tndrwang@gmail.com>
1 sibling, 0 replies; 32+ messages in thread
From: Tender Wang @ 2025-03-12 13:24 UTC (permalink / raw)
To: Amit Langote <amitlangote09@gmail.com>; +Cc: Dean Rasheed <dean.a.rasheed@gmail.com>; David Rowley <dgrowleyml@gmail.com>; pgsql-bugs@lists.postgresql.org; tharakan@gmail.com
Amit Langote <amitlangote09@gmail.com> 于2025年3月12日周三 20:24写道：
> Thanks -- I’ve studied the code and I agree with the conclusion: when
> all result relations are pruned, we still need to lock and process the
> first one to preserve executor invariants.
>
> Your examples with ExecMerge() and ExecInitPartitionInfo() make the
> consequences of missing resultRelInfo[0] pretty clear. Locking and
> including the first result relation, even if pruned, seems like the
> most straightforward way to maintain correctness without deeper
> structural changes.
>
> I've come up with the attached. I'll still need to add a test case.
>
It looks good to me, on a quick read-through.
We now don't have merge into ... not match ... test case in
partition_prune.sql
In [1], I figured out a query that could trigger a crash. It may be used as
the test case.
create table part_abc (a int, b text, c bool) partition by list (a);
create table part_abc_1 (b text, a int, c bool);
create table part_abc_2 (a int, c bool, b text);
alter table part_abc attach partition part_abc_1 for values in (1);
alter table part_abc attach partition part_abc_2 for values in (2);
insert into part_abc values (1, 'b', true);
insert into part_abc values (2, 'c', true);
create view part_abc_view as select * from part_abc where b <> 'a' with
check option;
create function stable_one() returns int as $$ begin return 1; end; $$
language plpgsql stable;
Above SQLs are already in parition_prune.sql, only need the below SQLs.
explain (costs off)
merge into part_abc_view pt
using (select stable_one() + 2 as pid) as q join part_abc_1 pt1 on (true)
on pt.a = stable_one() +2
when not matched then insert values(1, 'd', false);
merge into part_abc_view pt
using (select stable_one() + 2 as pid) as q join part_abc_1 pt1 on (true)
on pt.a = stable_one() +2
when not matched then insert values(1, 'd', false);
[1]
https://www.postgresql.org/message-id/CAHewXN%3DS5nqVMqoYpFXe8OykqDC-r22vG7RvDN-VFamY7XcVTw%40mail.g...
--
Thanks,
Tender Wang
^ permalink raw reply [nested|flat] 32+ messages in thread
* Re: BUG #18830: ExecInitMerge Segfault on MERGE
2025-03-02 21:00 BUG #18830: ExecInitMerge Segfault on MERGE PG Bug reporting form <noreply@postgresql.org>
2025-03-03 15:46 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Tender Wang <tndrwang@gmail.com>
2025-03-04 06:58 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Tender Wang <tndrwang@gmail.com>
2025-03-04 09:29 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE David Rowley <dgrowleyml@gmail.com>
2025-03-04 18:56 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Dean Rasheed <dean.a.rasheed@gmail.com>
2025-03-12 12:24 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Amit Langote <amitlangote09@gmail.com>
@ 2025-03-12 14:59 ` Dean Rasheed <dean.a.rasheed@gmail.com>
2025-03-12 16:11 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Tender Wang <tndrwang@gmail.com>
1 sibling, 1 reply; 32+ messages in thread
From: Dean Rasheed @ 2025-03-12 14:59 UTC (permalink / raw)
To: Amit Langote <amitlangote09@gmail.com>; +Cc: David Rowley <dgrowleyml@gmail.com>; Tender Wang <tndrwang@gmail.com>; pgsql-bugs@lists.postgresql.org; tharakan@gmail.com
On Wed, 12 Mar 2025 at 12:24, Amit Langote <amitlangote09@gmail.com> wrote:
>
> Thanks -- I’ve studied the code and I agree with the conclusion: when
> all result relations are pruned, we still need to lock and process the
> first one to preserve executor invariants.
>
> Your examples with ExecMerge() and ExecInitPartitionInfo() make the
> consequences of missing resultRelInfo[0] pretty clear. Locking and
> including the first result relation, even if pruned, seems like the
> most straightforward way to maintain correctness without deeper
> structural changes.
>
> I've come up with the attached. I'll still need to add a test case.
Hmm, this isn't quite sufficient.
In ExecDoInitialPruning(), stmt->resultRelations contains all the
result relations plus the root relation (if set) of each ModifyTable
node in the planned stmt (see set_plan_refs()). Therefore, just adding
the first result relation in that list may not be sufficient to ensure
that every ModifyTable node ends up with at least one unpruned result
relation.
For example, consider:
create table foo (a int);
create table part_abc (a int, b text, c bool) partition by list (a);
create table part_abc_1 (b text, c bool, a int);
create table part_abc_2 (b text, a int, c bool);
alter table part_abc attach partition part_abc_1 for values in (1);
alter table part_abc attach partition part_abc_2 for values in (2);
insert into part_abc_1 values ('b', true, 1);
insert into part_abc_2 values ('c', 2, true);
create function stable_one() returns int as
$$ begin return 1; end; $$ language plpgsql stable;
with t as (
merge into part_abc pt using (values(1)) v(a) on pt.a = stable_one() + 2
when not matched then insert values (v.a, 'd', false)
returning pt.*
)
insert into foo select a from t;
ERROR: trying to open a pruned relation
The problem here is that "foo" is the first result relation in
stmt->resultRelations, so that's the one that it chooses not to prune,
which is of no use to the merge.
Regards,
Dean
^ permalink raw reply [nested|flat] 32+ messages in thread
* Re: BUG #18830: ExecInitMerge Segfault on MERGE
2025-03-02 21:00 BUG #18830: ExecInitMerge Segfault on MERGE PG Bug reporting form <noreply@postgresql.org>
2025-03-03 15:46 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Tender Wang <tndrwang@gmail.com>
2025-03-04 06:58 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Tender Wang <tndrwang@gmail.com>
2025-03-04 09:29 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE David Rowley <dgrowleyml@gmail.com>
2025-03-04 18:56 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Dean Rasheed <dean.a.rasheed@gmail.com>
2025-03-12 12:24 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Amit Langote <amitlangote09@gmail.com>
2025-03-12 14:59 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Dean Rasheed <dean.a.rasheed@gmail.com>
@ 2025-03-12 16:11 ` Tender Wang <tndrwang@gmail.com>
2025-03-12 19:23 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Dean Rasheed <dean.a.rasheed@gmail.com>
0 siblings, 1 reply; 32+ messages in thread
From: Tender Wang @ 2025-03-12 16:11 UTC (permalink / raw)
To: Dean Rasheed <dean.a.rasheed@gmail.com>; +Cc: Amit Langote <amitlangote09@gmail.com>; David Rowley <dgrowleyml@gmail.com>; pgsql-bugs@lists.postgresql.org; tharakan@gmail.com
Dean Rasheed <dean.a.rasheed@gmail.com> 于2025年3月12日周三 22:59写道：
> On Wed, 12 Mar 2025 at 12:24, Amit Langote <amitlangote09@gmail.com>
> wrote:
> >
> > Thanks -- I’ve studied the code and I agree with the conclusion: when
> > all result relations are pruned, we still need to lock and process the
> > first one to preserve executor invariants.
> >
> > Your examples with ExecMerge() and ExecInitPartitionInfo() make the
> > consequences of missing resultRelInfo[0] pretty clear. Locking and
> > including the first result relation, even if pruned, seems like the
> > most straightforward way to maintain correctness without deeper
> > structural changes.
> >
> > I've come up with the attached. I'll still need to add a test case.
>
> Hmm, this isn't quite sufficient.
>
> In ExecDoInitialPruning(), stmt->resultRelations contains all the
> result relations plus the root relation (if set) of each ModifyTable
> node in the planned stmt (see set_plan_refs()). Therefore, just adding
> the first result relation in that list may not be sufficient to ensure
> that every ModifyTable node ends up with at least one unpruned result
> relation.
>
> For example, consider:
>
> create table foo (a int);
> create table part_abc (a int, b text, c bool) partition by list (a);
> create table part_abc_1 (b text, c bool, a int);
> create table part_abc_2 (b text, a int, c bool);
> alter table part_abc attach partition part_abc_1 for values in (1);
> alter table part_abc attach partition part_abc_2 for values in (2);
> insert into part_abc_1 values ('b', true, 1);
> insert into part_abc_2 values ('c', 2, true);
> create function stable_one() returns int as
> $$ begin return 1; end; $$ language plpgsql stable;
>
> with t as (
> merge into part_abc pt using (values(1)) v(a) on pt.a = stable_one() + 2
> when not matched then insert values (v.a, 'd', false)
> returning pt.*
> )
> insert into foo select a from t;
>
> ERROR: trying to open a pruned relation
>
Nice catch. The above case can be another test case.
I can find another same error in the below query:
with t as (update part_abc set c = true where a = stable_one() +2
returning a) insert into foo select a from t;
--
Thanks,
Tender Wang
^ permalink raw reply [nested|flat] 32+ messages in thread
* Re: BUG #18830: ExecInitMerge Segfault on MERGE
2025-03-02 21:00 BUG #18830: ExecInitMerge Segfault on MERGE PG Bug reporting form <noreply@postgresql.org>
2025-03-03 15:46 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Tender Wang <tndrwang@gmail.com>
2025-03-04 06:58 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Tender Wang <tndrwang@gmail.com>
2025-03-04 09:29 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE David Rowley <dgrowleyml@gmail.com>
2025-03-04 18:56 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Dean Rasheed <dean.a.rasheed@gmail.com>
2025-03-12 12:24 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Amit Langote <amitlangote09@gmail.com>
2025-03-12 14:59 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Dean Rasheed <dean.a.rasheed@gmail.com>
2025-03-12 16:11 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Tender Wang <tndrwang@gmail.com>
@ 2025-03-12 19:23 ` Dean Rasheed <dean.a.rasheed@gmail.com>
2025-03-13 07:29 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Tender Wang <tndrwang@gmail.com>
2025-03-13 12:44 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Amit Langote <amitlangote09@gmail.com>
0 siblings, 2 replies; 32+ messages in thread
From: Dean Rasheed @ 2025-03-12 19:23 UTC (permalink / raw)
To: Tender Wang <tndrwang@gmail.com>; +Cc: Amit Langote <amitlangote09@gmail.com>; David Rowley <dgrowleyml@gmail.com>; pgsql-bugs@lists.postgresql.org; tharakan@gmail.com
On Wed, 12 Mar 2025 at 16:11, Tender Wang <tndrwang@gmail.com> wrote:
>
> I can find another same error in the below query:
>
> with t as (update part_abc set c = true where a = stable_one() +2 returning a) insert into foo select a from t;
>
Interesting example. That passes in HEAD, but fails with the v1 patch
because it is attempting to open a pruned result relation when it
doesn't need to.
Attached is a rough patch that attempts to solve these issues locally
in ExecInitModifyTable(), by not allowing all result relations to be
pruned for that node. That makes it easy to handle cases with multiple
ModifyTable nodes.
This also has the advantage that all these relations can continue to
be pruned from the subplans of the ModifyTable nodes, making those
scans more efficient, and it only keeps a pruned result relation if
all other result relations have been pruned.
It does mean that ExecGetRangeTableRelation() needs to allow pruned
relations to be opened, if called from ExecInitResultRelation(). I
think that's OK because the check for opening pruned relations still
applies to scan relations.
I also adjusted explain.c slightly, to avoid the dummy pruned result
relation that we might now keep from appearing in the EXPLAIN output
as a target relation. Allowing it to be shown looked a bit odd,
because it's not really the target relation in any real sense.
I also noticed a few Asserts in ExecInitModifyTable() that didn't
appear to be doing what they were originally intended to do.
Regards,
Dean
Attachments:
v2-do-not-prune-all-ModifyTable-result-relations.patch (text/x-patch, 12.7K) download
diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
new file mode 100644
index 19ffcc2..967a5f8
--- a/src/backend/commands/explain.c
+++ b/src/backend/commands/explain.c
@@ -4698,7 +4698,9 @@ show_modifytable_info(ModifyTableState *
/* Should we explicitly label target relations? */
labeltargets = (mtstate->mt_nrels > 1 ||
(mtstate->mt_nrels == 1 &&
- mtstate->resultRelInfo[0].ri_RangeTableIndex != node->nominalRelation));
+ mtstate->resultRelInfo[0].ri_RangeTableIndex != node->nominalRelation &&
+ bms_is_member(mtstate->resultRelInfo[0].ri_RangeTableIndex,
+ mtstate->ps.state->es_unpruned_relids)));
if (labeltargets)
ExplainOpenGroup("Target Tables", "Target Tables", false, es);
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
new file mode 100644
index 0493b7d..e9bd98c
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -1006,7 +1006,7 @@ InitPlan(QueryDesc *queryDesc, int eflag
case ROW_MARK_SHARE:
case ROW_MARK_KEYSHARE:
case ROW_MARK_REFERENCE:
- relation = ExecGetRangeTableRelation(estate, rc->rti);
+ relation = ExecGetRangeTableRelation(estate, rc->rti, false);
break;
case ROW_MARK_COPY:
/* no physical table access is required */
diff --git a/src/backend/executor/execPartition.c b/src/backend/executor/execPartition.c
new file mode 100644
index 5cd5e2e..6ac165a
--- a/src/backend/executor/execPartition.c
+++ b/src/backend/executor/execPartition.c
@@ -2076,7 +2076,7 @@ CreatePartitionPruneState(EState *estate
* because that entry will be held open and locked for the
* duration of this executor run.
*/
- partrel = ExecGetRangeTableRelation(estate, pinfo->rtindex);
+ partrel = ExecGetRangeTableRelation(estate, pinfo->rtindex, false);
/* Remember for InitExecPartitionPruneContext(). */
pprune->partrel = partrel;
diff --git a/src/backend/executor/execUtils.c b/src/backend/executor/execUtils.c
new file mode 100644
index 39d6f4d..e1521e2
--- a/src/backend/executor/execUtils.c
+++ b/src/backend/executor/execUtils.c
@@ -746,7 +746,7 @@ ExecOpenScanRelation(EState *estate, Ind
Relation rel;
/* Open the relation. */
- rel = ExecGetRangeTableRelation(estate, scanrelid);
+ rel = ExecGetRangeTableRelation(estate, scanrelid, false);
/*
* Complain if we're attempting a scan of an unscannable relation, except
@@ -815,19 +815,24 @@ ExecInitRangeTable(EState *estate, List
*
* The Relations will be closed in ExecEndPlan().
*
- * Note: The caller must ensure that 'rti' refers to an unpruned relation
- * (i.e., it is a member of estate->es_unpruned_relids) before calling this
- * function. Attempting to open a pruned relation will result in an error.
+ * If isResultRel is true, the relation is being used as a result relation.
+ * Such a relation might have been pruned, which is OK for result relations,
+ * but not for scan relations. If isResultRel is false, the caller must
+ * ensure that 'rti' refers to an unpruned relation (i.e., it is a member of
+ * estate->es_unpruned_relids) before calling this function. Attempting to
+ * open a pruned relation for scanning will result in an error.
*/
Relation
-ExecGetRangeTableRelation(EState *estate, Index rti)
+ExecGetRangeTableRelation(EState *estate, Index rti, bool isResultRel)
{
+ bool pruned;
Relation rel;
Assert(rti > 0 && rti <= estate->es_range_table_size);
- if (!bms_is_member(rti, estate->es_unpruned_relids))
- elog(ERROR, "trying to open a pruned relation");
+ pruned = !bms_is_member(rti, estate->es_unpruned_relids);
+ if (!isResultRel && pruned)
+ elog(ERROR, "trying to open a pruned relation for scanning");
rel = estate->es_relations[rti - 1];
if (rel == NULL)
@@ -837,7 +842,15 @@ ExecGetRangeTableRelation(EState *estate
Assert(rte->rtekind == RTE_RELATION);
- if (!IsParallelWorker())
+ if (isResultRel && pruned)
+ {
+ /*
+ * A pruned result relation might not have been locked yet, so we
+ * must lock it now.
+ */
+ rel = table_open(rte->relid, rte->rellockmode);
+ }
+ else if (!IsParallelWorker())
{
/*
* In a normal query, we should already have the appropriate lock,
@@ -880,7 +893,7 @@ ExecInitResultRelation(EState *estate, R
{
Relation resultRelationDesc;
- resultRelationDesc = ExecGetRangeTableRelation(estate, rti);
+ resultRelationDesc = ExecGetRangeTableRelation(estate, rti, true);
InitResultRelInfo(resultRelInfo,
resultRelationDesc,
rti,
diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
new file mode 100644
index b0fe500..f2cf999
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -4471,6 +4471,7 @@ ExecInitModifyTable(ModifyTable *node, E
ModifyTableState *mtstate;
Plan *subplan = outerPlan(node);
CmdType operation = node->operation;
+ int total_nrels = list_length(node->resultRelations);
int nrels;
List *resultRelations = NIL;
List *withCheckOptionLists = NIL;
@@ -4490,13 +4491,25 @@ ExecInitModifyTable(ModifyTable *node, E
/*
* Only consider unpruned relations for initializing their ResultRelInfo
* struct and other fields such as withCheckOptions, etc.
+ *
+ * Note, however, that we must avoid pruning every result relation. This
+ * is important for MERGE, since even if every result relation is pruned
+ * from the subplan, there might still be NOT MATCHED rows, for which
+ * there may be INSERT actions to perform. To allow these actions to be
+ * found, at least one result relation must be kept. In addition, when
+ * inserting into a partitioned table, ExecInitPartitionInfo() needs a
+ * ResultRelInfo struct as a reference for building the ResultRelInfo of
+ * the target partition. In either case, it doesn't matter which result
+ * relation is kept, so we just keep the last one, if all others have been
+ * pruned.
*/
i = 0;
foreach(l, node->resultRelations)
{
Index rti = lfirst_int(l);
- if (bms_is_member(rti, estate->es_unpruned_relids))
+ if (bms_is_member(rti, estate->es_unpruned_relids) ||
+ (i == total_nrels - 1 && resultRelations == NIL))
{
resultRelations = lappend_int(resultRelations, rti);
if (node->withCheckOptionLists)
@@ -4537,6 +4550,7 @@ ExecInitModifyTable(ModifyTable *node, E
i++;
}
nrels = list_length(resultRelations);
+ Assert(nrels > 0);
/*
* create state structure
@@ -4735,7 +4749,7 @@ ExecInitModifyTable(ModifyTable *node, E
*/
mtstate->mt_resultOidAttno =
ExecFindJunkAttributeInTlist(subplan->targetlist, "tableoid");
- Assert(AttributeNumberIsValid(mtstate->mt_resultOidAttno) || nrels == 1);
+ Assert(AttributeNumberIsValid(mtstate->mt_resultOidAttno) || total_nrels == 1);
mtstate->mt_lastResultOid = InvalidOid; /* force lookup at first tuple */
mtstate->mt_lastResultIndex = 0; /* must be zero if no such attr */
@@ -4832,7 +4846,7 @@ ExecInitModifyTable(ModifyTable *node, E
if (node->onConflictAction != ONCONFLICT_NONE)
{
/* insert may only have one relation, inheritance is not expanded */
- Assert(nrels == 1);
+ Assert(total_nrels == 1);
resultRelInfo->ri_onConflictArbiterIndexes = node->arbiterIndexes;
}
@@ -4979,7 +4993,7 @@ ExecInitModifyTable(ModifyTable *node, E
if (operation == CMD_INSERT)
{
/* insert may only have one relation, inheritance is not expanded */
- Assert(nrels == 1);
+ Assert(total_nrels == 1);
resultRelInfo = mtstate->resultRelInfo;
if (!resultRelInfo->ri_usesFdwDirectModify &&
resultRelInfo->ri_FdwRoutine != NULL &&
diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h
new file mode 100644
index 0d2ffab..0db5d18
--- a/src/include/executor/executor.h
+++ b/src/include/executor/executor.h
@@ -680,7 +680,8 @@ exec_rt_fetch(Index rti, EState *estate)
return (RangeTblEntry *) list_nth(estate->es_range_table, rti - 1);
}
-extern Relation ExecGetRangeTableRelation(EState *estate, Index rti);
+extern Relation ExecGetRangeTableRelation(EState *estate, Index rti,
+ bool isResultRel);
extern void ExecInitResultRelation(EState *estate, ResultRelInfo *resultRelInfo,
Index rti);
diff --git a/src/test/regress/expected/partition_prune.out b/src/test/regress/expected/partition_prune.out
new file mode 100644
index 8097f4e..0bf3526
--- a/src/test/regress/expected/partition_prune.out
+++ b/src/test/regress/expected/partition_prune.out
@@ -4662,6 +4662,88 @@ table part_abc_view;
2 | c | t
(1 row)
+-- MERGE ... INSERT when all pruned from MERGE source.
+begin;
+explain (costs off)
+merge into part_abc_view pt
+using (select stable_one() + 1 as pid) as q join part_abc_2 pt2 on (q.pid = pt2.a) on pt.a = stable_one() + 2
+when not matched then insert values (1, 'd', false) returning pt.a;
+ QUERY PLAN
+------------------------------------------------
+ Merge on part_abc
+ -> Nested Loop Left Join
+ -> Seq Scan on part_abc_2 pt2
+ Filter: ((stable_one() + 1) = a)
+ -> Materialize
+ -> Append
+ Subplans Removed: 2
+(7 rows)
+
+merge into part_abc_view pt
+using (select stable_one() + 1 as pid) as q join part_abc_2 pt2 on (q.pid = pt2.a) on pt.a = stable_one() + 2
+when not matched then insert values (1, 'd', false) returning pt.a;
+ a
+---
+ 1
+(1 row)
+
+table part_abc_view;
+ a | b | c
+---+---+---
+ 1 | d | f
+ 2 | c | t
+(2 rows)
+
+rollback;
+-- A case with multiple ModifyTable nodes.
+begin;
+create table part_abc_log (action text, a int, b text, c bool);
+explain (costs off)
+with t as (
+ merge into part_abc_view pt
+ using (select stable_one() + 1 as pid) as q join part_abc_2 pt2 on (q.pid = pt2.a) on pt.a = stable_one() + 2
+ when not matched then insert values (1, 'd', false) returning merge_action(), pt.*
+)
+insert into part_abc_log select * from t returning *;
+ QUERY PLAN
+--------------------------------------------------------
+ Insert on part_abc_log
+ CTE t
+ -> Merge on part_abc
+ -> Nested Loop Left Join
+ -> Seq Scan on part_abc_2 pt2
+ Filter: ((stable_one() + 1) = a)
+ -> Materialize
+ -> Append
+ Subplans Removed: 2
+ -> CTE Scan on t
+(10 rows)
+
+with t as (
+ merge into part_abc_view pt
+ using (select stable_one() + 1 as pid) as q join part_abc_2 pt2 on (q.pid = pt2.a) on pt.a = stable_one() + 2
+ when not matched then insert values (1, 'd', false) returning merge_action(), pt.*
+)
+insert into part_abc_log select * from t returning *;
+ action | a | b | c
+--------+---+---+---
+ INSERT | 1 | d | f
+(1 row)
+
+table part_abc_view;
+ a | b | c
+---+---+---
+ 1 | d | f
+ 2 | c | t
+(2 rows)
+
+table part_abc_log;
+ action | a | b | c
+--------+---+---+---
+ INSERT | 1 | d | f
+(1 row)
+
+rollback;
-- A case with nested MergeAppend with its own PartitionPruneInfo.
create index on part_abc (a);
alter table part_abc add d int;
diff --git a/src/test/regress/sql/partition_prune.sql b/src/test/regress/sql/partition_prune.sql
new file mode 100644
index 4a2c74b..f6db947
--- a/src/test/regress/sql/partition_prune.sql
+++ b/src/test/regress/sql/partition_prune.sql
@@ -1401,6 +1401,38 @@ using (select stable_one() + 2 as pid) a
when matched then delete returning pt.a;
table part_abc_view;
+-- MERGE ... INSERT when all pruned from MERGE source.
+begin;
+explain (costs off)
+merge into part_abc_view pt
+using (select stable_one() + 1 as pid) as q join part_abc_2 pt2 on (q.pid = pt2.a) on pt.a = stable_one() + 2
+when not matched then insert values (1, 'd', false) returning pt.a;
+merge into part_abc_view pt
+using (select stable_one() + 1 as pid) as q join part_abc_2 pt2 on (q.pid = pt2.a) on pt.a = stable_one() + 2
+when not matched then insert values (1, 'd', false) returning pt.a;
+table part_abc_view;
+rollback;
+
+-- A case with multiple ModifyTable nodes.
+begin;
+create table part_abc_log (action text, a int, b text, c bool);
+explain (costs off)
+with t as (
+ merge into part_abc_view pt
+ using (select stable_one() + 1 as pid) as q join part_abc_2 pt2 on (q.pid = pt2.a) on pt.a = stable_one() + 2
+ when not matched then insert values (1, 'd', false) returning merge_action(), pt.*
+)
+insert into part_abc_log select * from t returning *;
+with t as (
+ merge into part_abc_view pt
+ using (select stable_one() + 1 as pid) as q join part_abc_2 pt2 on (q.pid = pt2.a) on pt.a = stable_one() + 2
+ when not matched then insert values (1, 'd', false) returning merge_action(), pt.*
+)
+insert into part_abc_log select * from t returning *;
+table part_abc_view;
+table part_abc_log;
+rollback;
+
-- A case with nested MergeAppend with its own PartitionPruneInfo.
create index on part_abc (a);
alter table part_abc add d int;
* Re: BUG #18830: ExecInitMerge Segfault on MERGE
2025-03-02 21:00 BUG #18830: ExecInitMerge Segfault on MERGE PG Bug reporting form <noreply@postgresql.org>
2025-03-03 15:46 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Tender Wang <tndrwang@gmail.com>
2025-03-04 06:58 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Tender Wang <tndrwang@gmail.com>
2025-03-04 09:29 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE David Rowley <dgrowleyml@gmail.com>
2025-03-04 18:56 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Dean Rasheed <dean.a.rasheed@gmail.com>
2025-03-12 12:24 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Amit Langote <amitlangote09@gmail.com>
2025-03-12 14:59 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Dean Rasheed <dean.a.rasheed@gmail.com>
2025-03-12 16:11 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Tender Wang <tndrwang@gmail.com>
2025-03-12 19:23 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Dean Rasheed <dean.a.rasheed@gmail.com>
@ 2025-03-13 07:29 ` Tender Wang <tndrwang@gmail.com>
2025-03-13 12:44 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Amit Langote <amitlangote09@gmail.com>
1 sibling, 1 reply; 32+ messages in thread
From: Tender Wang @ 2025-03-13 07:29 UTC (permalink / raw)
To: Dean Rasheed <dean.a.rasheed@gmail.com>; +Cc: Amit Langote <amitlangote09@gmail.com>; David Rowley <dgrowleyml@gmail.com>; pgsql-bugs@lists.postgresql.org; tharakan@gmail.com
Dean Rasheed <dean.a.rasheed@gmail.com> 于2025年3月13日周四 03:23写道：
> On Wed, 12 Mar 2025 at 16:11, Tender Wang <tndrwang@gmail.com> wrote:
> >
> > I can find another same error in the below query:
> >
> > with t as (update part_abc set c = true where a = stable_one() +2
> returning a) insert into foo select a from t;
> >
>
> Interesting example. That passes in HEAD, but fails with the v1 patch
> because it is attempting to open a pruned result relation when it
> doesn't need to.
>
> Attached is a rough patch that attempts to solve these issues locally
> in ExecInitModifyTable(), by not allowing all result relations to be
> pruned for that node. That makes it easy to handle cases with multiple
> ModifyTable nodes.
>
> This also has the advantage that all these relations can continue to
> be pruned from the subplans of the ModifyTable nodes, making those
> scans more efficient, and it only keeps a pruned result relation if
> all other result relations have been pruned.
>
> It does mean that ExecGetRangeTableRelation() needs to allow pruned
> relations to be opened, if called from ExecInitResultRelation(). I
> think that's OK because the check for opening pruned relations still
> applies to scan relations.
>
> I also adjusted explain.c slightly, to avoid the dummy pruned result
> relation that we might now keep from appearing in the EXPLAIN output
> as a target relation. Allowing it to be shown looked a bit odd,
> because it's not really the target relation in any real sense.
>
> I also noticed a few Asserts in ExecInitModifyTable() that didn't
> appear to be doing what they were originally intended to do.
>
I tested and debugged the attached patch. I didn't find any other issue.
A comment in execPartition.h may need to be changed:
* partrel Partitioned table
Relation; obtained by
*
ExecGetRangeTableRelation(estate, rti), where
Because the func interface has changed, the above comments are better to be
changed "ExecGetRangeTableRelation(estate, rti, false)".
--
Thanks,
Tender Wang
^ permalink raw reply [nested|flat] 32+ messages in thread
* Re: BUG #18830: ExecInitMerge Segfault on MERGE
2025-03-02 21:00 BUG #18830: ExecInitMerge Segfault on MERGE PG Bug reporting form <noreply@postgresql.org>
2025-03-03 15:46 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Tender Wang <tndrwang@gmail.com>
2025-03-04 06:58 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Tender Wang <tndrwang@gmail.com>
2025-03-04 09:29 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE David Rowley <dgrowleyml@gmail.com>
2025-03-04 18:56 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Dean Rasheed <dean.a.rasheed@gmail.com>
2025-03-12 12:24 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Amit Langote <amitlangote09@gmail.com>
2025-03-12 14:59 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Dean Rasheed <dean.a.rasheed@gmail.com>
2025-03-12 16:11 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Tender Wang <tndrwang@gmail.com>
2025-03-12 19:23 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Dean Rasheed <dean.a.rasheed@gmail.com>
2025-03-13 07:29 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Tender Wang <tndrwang@gmail.com>
@ 2025-03-13 12:44 ` Amit Langote <amitlangote09@gmail.com>
0 siblings, 0 replies; 32+ messages in thread
From: Amit Langote @ 2025-03-13 12:44 UTC (permalink / raw)
To: Tender Wang <tndrwang@gmail.com>; +Cc: Dean Rasheed <dean.a.rasheed@gmail.com>; David Rowley <dgrowleyml@gmail.com>; pgsql-bugs@lists.postgresql.org; tharakan@gmail.com
On Thu, Mar 13, 2025 at 4:29 PM Tender Wang <tndrwang@gmail.com> wrote:
> Dean Rasheed <dean.a.rasheed@gmail.com> 于2025年3月13日周四 03:23写道：
>> It does mean that ExecGetRangeTableRelation() needs to allow pruned
>> relations to be opened, if called from ExecInitResultRelation(). I
>> think that's OK because the check for opening pruned relations still
>> applies to scan relations.
>
> A comment in execPartition.h may need to be changed:
> * partrel Partitioned table Relation; obtained by
> * ExecGetRangeTableRelation(estate, rti), where
>
> Because the func interface has changed, the above comments are better to be changed "ExecGetRangeTableRelation(estate, rti, false)".
Noted, thanks.
--
Thanks, Amit Langote
^ permalink raw reply [nested|flat] 32+ messages in thread
* Re: BUG #18830: ExecInitMerge Segfault on MERGE
2025-03-02 21:00 BUG #18830: ExecInitMerge Segfault on MERGE PG Bug reporting form <noreply@postgresql.org>
2025-03-03 15:46 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Tender Wang <tndrwang@gmail.com>
2025-03-04 06:58 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Tender Wang <tndrwang@gmail.com>
2025-03-04 09:29 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE David Rowley <dgrowleyml@gmail.com>
2025-03-04 18:56 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Dean Rasheed <dean.a.rasheed@gmail.com>
2025-03-12 12:24 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Amit Langote <amitlangote09@gmail.com>
2025-03-12 14:59 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Dean Rasheed <dean.a.rasheed@gmail.com>
2025-03-12 16:11 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Tender Wang <tndrwang@gmail.com>
2025-03-12 19:23 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Dean Rasheed <dean.a.rasheed@gmail.com>
@ 2025-03-13 12:44 ` Amit Langote <amitlangote09@gmail.com>
2025-03-13 14:42 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Dean Rasheed <dean.a.rasheed@gmail.com>
1 sibling, 1 reply; 32+ messages in thread
From: Amit Langote @ 2025-03-13 12:44 UTC (permalink / raw)
To: Dean Rasheed <dean.a.rasheed@gmail.com>; +Cc: Tender Wang <tndrwang@gmail.com>; David Rowley <dgrowleyml@gmail.com>; pgsql-bugs@lists.postgresql.org; tharakan@gmail.com
On Thu, Mar 13, 2025 at 4:23 AM Dean Rasheed <dean.a.rasheed@gmail.com> wrote:
> On Wed, 12 Mar 2025 at 16:11, Tender Wang <tndrwang@gmail.com> wrote:
> >
> > I can find another same error in the below query:
> >
> > with t as (update part_abc set c = true where a = stable_one() +2 returning a) insert into foo select a from t;
> >
Yes, I missed that PlannedStmt.resultRelations can include entries
from multiple ModifyTable nodes -- not the first time I’ve overlooked
that. :(
> Interesting example. That passes in HEAD, but fails with the v1 patch
> because it is attempting to open a pruned result relation when it
> doesn't need to.
>
> Attached is a rough patch that attempts to solve these issues locally
> in ExecInitModifyTable(), by not allowing all result relations to be
> pruned for that node. That makes it easy to handle cases with multiple
> ModifyTable nodes.
Thanks for the patch. I have some comments on the approach.
> This also has the advantage that all these relations can continue to
> be pruned from the subplans of the ModifyTable nodes, making those
> scans more efficient, and it only keeps a pruned result relation if
> all other result relations have been pruned.
>
> It does mean that ExecGetRangeTableRelation() needs to allow pruned
> relations to be opened, if called from ExecInitResultRelation(). I
> think that's OK because the check for opening pruned relations still
> applies to scan relations.
+ if (isResultRel && pruned)
+ {
+ /*
+ * A pruned result relation might not have been locked yet, so we
+ * must lock it now.
+ */
+ rel = table_open(rte->relid, rte->rellockmode);
+ }
One thing I'd like to avoid is taking any locks during ExecInitNode(),
including in functions like ExecGetRangeTableRelation(). Doing so
would require checking whether the CachedPlan is still valid and
handling invalidation if it isn't -- see commit 525392d5727f for
background.
Handling that correctly would mean aborting plan initialization,
cleaning up any partially initialized nodes, and returning early from
ExecInitNode(). To avoid that complexity, I've arranged for all
remaining necessary locks -- specifically on leaf partitions that
survive pruning -- to be taken in ExecDoInitialPruning(), which is
invoked by InitPlan() before any plan tree is initialized, and
checking CachedPlan validity once after returning from it:
/* ----------------------------------------------------------------
* InitPlan
*
* Initializes the query plan: open files, allocate storage
* and start up the rule manager
*
* If the plan originates from a CachedPlan (given in queryDesc->cplan),
* it can become invalid during runtime "initial" pruning when the
* remaining set of locks is taken. The function returns early in that
* case without initializing the plan, and the caller is expected to
* retry with a new valid plan.
* ----------------------------------------------------------------
*/
static void
InitPlan(QueryDesc *queryDesc, int eflags)
{
...
ExecDoInitialPruning(estate);
if (!ExecPlanStillValid(estate))
return;
So I'd prefer to keep all locking outside of ExecInitNode() for that reason.
One way to deal with multiple ModifyTable nodes is to have PlannedStmt
remember the "first" result relation of each, either as a List or a
Bitmapset. But I’m not sure Tom would be on board with that,
considering the work we did around 2018, e.g., commit 52ed730d511.
Also, adding a new field to PlannedStmt just to support a corner case
like this might be overkill -- though I’m fine with it if there’s no
other non-invasive way to address the issue. The alternative you
mentioned earlier -- modifying the code to use
ModifyTableState.rootResultRelInfo instead of resultRelInfo[0] --
seems at least somewhat invasive.
I’ve attached v3, which implements the above approach. It also
includes your changes to explain.c, the ExecGetRangeTableRelation()
interface adjustment, the Assert fixes in ExecInitModifyTable(), and
the test cases.
> I also adjusted explain.c slightly, to avoid the dummy pruned result
> relation that we might now keep from appearing in the EXPLAIN output
> as a target relation. Allowing it to be shown looked a bit odd,
> because it's not really the target relation in any real sense.
I agree about this part.
> I also noticed a few Asserts in ExecInitModifyTable() that didn't
> appear to be doing what they were originally intended to do.
Check too.
--
Thanks, Amit Langote
Attachments:
v3-0001-Ensure-first-result-relation-is-included-even-if-.patch (application/octet-stream, 19.6K) download
From dacc87da678bd159624be097cc00070e885fd4f3 Mon Sep 17 00:00:00 2001
From: Amit Langote <amitlan@postgresql.org>
Date: Thu, 13 Mar 2025 21:07:01 +0900
Subject: [PATCH v3] Ensure first result relation is included even if pruned
This started as an investigation into crashes and misbehavior in MERGE
statements when all target partitions were pruned. These issues became
possible after commit cbc127917e, which introduced tracking of
unpruned relids to avoid processing pruned relations, and changed
ExecInitModifyTable() to process only unpruned result relations.
Some executor code paths rely on ModifyTableState.resultRelInfo[0]
being present and initialized, even when no result relations remain
after pruning. For example, ExecMerge() and ExecMergeNotMatched() use
the first resultRelInfo to determine the appropriate action.
Similarly, ExecInitPartitionInfo() assumes that at least one result
relation exists.
To preserve these assumptions, ExecDoInitialPruning() now ensures that
the first result relation of each ModifyTable node in the plan is
locked, even if it was pruned. ExecInitModifyTable() then includes it
in the initialized result relation list. To support this, PlannedStmt
now includes a list of RT indexes identifying the first result
relation of each ModifyTable node in the plan.
This avoids crashes and undefined behavior while still allowing pruning
of other result relations to proceed as before.
Bug: #18830
Reported-by: Robins Tharakan <tharakan@gmail.com>
Diagnozed-by: Tender Wang <tndrwang@gmail.com>
Diagnozed-by: Dean Rasheed <dean.a.rasheed@gmail.com>
Suggested-by: Dean Rasheed <dean.a.rasheed@gmail.com>
Reviewed-by: Dean Rasheed <dean.a.rasheed@gmail.com>
Reviewed-by: Tender Wang <tndrwang@gmail.com>
Discussion: https://postgr.es/m/18830-1f31ea1dc930d444%40postgresql.org
---
src/backend/commands/explain.c | 11 ++-
src/backend/executor/execMain.c | 2 +-
src/backend/executor/execPartition.c | 25 +++++-
src/backend/executor/execUtils.c | 18 ++--
src/backend/executor/nodeModifyTable.c | 41 +++++++++-
src/backend/optimizer/plan/planner.c | 1 +
src/backend/optimizer/plan/setrefs.c | 3 +
src/include/executor/execPartition.h | 4 +-
src/include/executor/executor.h | 2 +-
src/include/nodes/pathnodes.h | 3 +
src/include/nodes/plannodes.h | 7 ++
src/test/regress/expected/partition_prune.out | 82 +++++++++++++++++++
src/test/regress/sql/partition_prune.sql | 32 ++++++++
13 files changed, 214 insertions(+), 17 deletions(-)
diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
index 19ffcc2cacb..f1da1325b2f 100644
--- a/src/backend/commands/explain.c
+++ b/src/backend/commands/explain.c
@@ -4695,10 +4695,17 @@ show_modifytable_info(ModifyTableState *mtstate, List *ancestors,
break;
}
- /* Should we explicitly label target relations? */
+ /*
+ * Should we explicitly label target relations?
+ *
+ * Do not show it in the nrels == 1 case if it is a pruned relation added
+ * by ExecInitModifyTable() in certain cases.
+ */
labeltargets = (mtstate->mt_nrels > 1 ||
(mtstate->mt_nrels == 1 &&
- mtstate->resultRelInfo[0].ri_RangeTableIndex != node->nominalRelation));
+ mtstate->resultRelInfo[0].ri_RangeTableIndex != node->nominalRelation &&
+ bms_is_member(mtstate->resultRelInfo[0].ri_RangeTableIndex,
+ mtstate->ps.state->es_unpruned_relids)));
if (labeltargets)
ExplainOpenGroup("Target Tables", "Target Tables", false, es);
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index 0493b7d5365..e9bd98c7738 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -1006,7 +1006,7 @@ InitPlan(QueryDesc *queryDesc, int eflags)
case ROW_MARK_SHARE:
case ROW_MARK_KEYSHARE:
case ROW_MARK_REFERENCE:
- relation = ExecGetRangeTableRelation(estate, rc->rti);
+ relation = ExecGetRangeTableRelation(estate, rc->rti, false);
break;
case ROW_MARK_COPY:
/* no physical table access is required */
diff --git a/src/backend/executor/execPartition.c b/src/backend/executor/execPartition.c
index 5cd5e2eeb80..d0557299032 100644
--- a/src/backend/executor/execPartition.c
+++ b/src/backend/executor/execPartition.c
@@ -1819,6 +1819,7 @@ adjust_partition_colnos_using_map(List *colnos, AttrMap *attrMap)
void
ExecDoInitialPruning(EState *estate)
{
+ PlannedStmt *stmt = estate->es_plannedstmt;
ListCell *lc;
List *locked_relids = NIL;
@@ -1867,6 +1868,28 @@ ExecDoInitialPruning(EState *estate)
validsubplans);
}
+ /*
+ * Lock the first result relation even if it was pruned. Some executor
+ * paths (e.g., in nodeModifyTable.c and execPartition.c) expect this
+ * relation to be locked regardless of pruning.
+ */
+ if (stmt->resultRelations && ExecShouldLockRelations(estate))
+ {
+ foreach(lc, stmt->firstResultRels)
+ {
+ Index firstResultRel = lfirst_int(lc);
+
+ if (!bms_is_member(firstResultRel, estate->es_unpruned_relids))
+ {
+ RangeTblEntry *rte = exec_rt_fetch(firstResultRel, estate);
+
+ Assert(rte->rtekind == RTE_RELATION && rte->rellockmode != NoLock);
+ LockRelationOid(rte->relid, rte->rellockmode);
+ locked_relids = lappend_int(locked_relids, firstResultRel);
+ }
+ }
+ }
+
/*
* Release the useless locks if the plan won't be executed. This is the
* same as what CheckCachedPlan() in plancache.c does.
@@ -2076,7 +2099,7 @@ CreatePartitionPruneState(EState *estate, PartitionPruneInfo *pruneinfo,
* because that entry will be held open and locked for the
* duration of this executor run.
*/
- partrel = ExecGetRangeTableRelation(estate, pinfo->rtindex);
+ partrel = ExecGetRangeTableRelation(estate, pinfo->rtindex, false);
/* Remember for InitExecPartitionPruneContext(). */
pprune->partrel = partrel;
diff --git a/src/backend/executor/execUtils.c b/src/backend/executor/execUtils.c
index 39d6f4d819e..55ab18fb826 100644
--- a/src/backend/executor/execUtils.c
+++ b/src/backend/executor/execUtils.c
@@ -746,7 +746,7 @@ ExecOpenScanRelation(EState *estate, Index scanrelid, int eflags)
Relation rel;
/* Open the relation. */
- rel = ExecGetRangeTableRelation(estate, scanrelid);
+ rel = ExecGetRangeTableRelation(estate, scanrelid, false);
/*
* Complain if we're attempting a scan of an unscannable relation, except
@@ -815,18 +815,22 @@ ExecInitRangeTable(EState *estate, List *rangeTable, List *permInfos,
*
* The Relations will be closed in ExecEndPlan().
*
- * Note: The caller must ensure that 'rti' refers to an unpruned relation
- * (i.e., it is a member of estate->es_unpruned_relids) before calling this
- * function. Attempting to open a pruned relation will result in an error.
+ * If isResultRel is true, the relation is being used as a result relation.
+ * Such a relation might have been pruned, which is OK for result relations,
+ * but not for scan relations; see the details in ExecInitModifyTable(). If
+ * isResultRel is false, the caller must ensure that 'rti' refers to an
+ * unpruned relation (i.e., it is a member of estate->es_unpruned_relids)
+ * before calling this function. Attempting to open a pruned relation for
+ * scanning will result in an error.
*/
Relation
-ExecGetRangeTableRelation(EState *estate, Index rti)
+ExecGetRangeTableRelation(EState *estate, Index rti, bool isResultRel)
{
Relation rel;
Assert(rti > 0 && rti <= estate->es_range_table_size);
- if (!bms_is_member(rti, estate->es_unpruned_relids))
+ if (!isResultRel && !bms_is_member(rti, estate->es_unpruned_relids))
elog(ERROR, "trying to open a pruned relation");
rel = estate->es_relations[rti - 1];
@@ -880,7 +884,7 @@ ExecInitResultRelation(EState *estate, ResultRelInfo *resultRelInfo,
{
Relation resultRelationDesc;
- resultRelationDesc = ExecGetRangeTableRelation(estate, rti);
+ resultRelationDesc = ExecGetRangeTableRelation(estate, rti, true);
InitResultRelInfo(resultRelInfo,
resultRelationDesc,
rti,
diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
index b0fe50075ad..97a1993f7c1 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -4471,6 +4471,7 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
ModifyTableState *mtstate;
Plan *subplan = outerPlan(node);
CmdType operation = node->operation;
+ int total_nrels = list_length(node->resultRelations);
int nrels;
List *resultRelations = NIL;
List *withCheckOptionLists = NIL;
@@ -4536,8 +4537,42 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
}
i++;
}
+
nrels = list_length(resultRelations);
+ /*
+ * We must avoid pruning every result relation. This is important for
+ * MERGE, since even if every result relation is pruned from the subplan,
+ * there might still be NOT MATCHED rows, for which there may be INSERT
+ * actions to perform. To allow these actions to be found, at least one
+ * result relation must be kept. In addition, when inserting into a
+ * partitioned table, ExecInitPartitionInfo() needs a ResultRelInfo struct
+ * as a reference for building the ResultRelInfo of the target partition.
+ * In either case, it doesn't matter which result relation is kept, so we
+ * just keep the first one, if all others have been pruned.
+ */
+ if (nrels == 0)
+ {
+ nrels = 1;
+ resultRelations = lappend_int(resultRelations,
+ linitial_int(node->resultRelations));
+ if (node->withCheckOptionLists)
+ withCheckOptionLists = lappend(withCheckOptionLists,
+ linitial(node->withCheckOptionLists));
+ if (node->returningLists)
+ returningLists = lappend(returningLists,
+ linitial(node->returningLists));
+ if (node->updateColnosLists)
+ updateColnosLists = lappend(updateColnosLists,
+ linitial(node->updateColnosLists));
+ if (node->mergeActionLists)
+ mergeActionLists = lappend(mergeActionLists,
+ linitial(node->mergeActionLists));
+ if (node->mergeJoinConditions)
+ mergeJoinConditions = lappend(mergeJoinConditions,
+ linitial(node->mergeJoinConditions));
+ }
+
/*
* create state structure
*/
@@ -4735,7 +4770,7 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
*/
mtstate->mt_resultOidAttno =
ExecFindJunkAttributeInTlist(subplan->targetlist, "tableoid");
- Assert(AttributeNumberIsValid(mtstate->mt_resultOidAttno) || nrels == 1);
+ Assert(AttributeNumberIsValid(mtstate->mt_resultOidAttno) || total_nrels == 1);
mtstate->mt_lastResultOid = InvalidOid; /* force lookup at first tuple */
mtstate->mt_lastResultIndex = 0; /* must be zero if no such attr */
@@ -4832,7 +4867,7 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
if (node->onConflictAction != ONCONFLICT_NONE)
{
/* insert may only have one relation, inheritance is not expanded */
- Assert(nrels == 1);
+ Assert(total_nrels == 1);
resultRelInfo->ri_onConflictArbiterIndexes = node->arbiterIndexes;
}
@@ -4979,7 +5014,7 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
if (operation == CMD_INSERT)
{
/* insert may only have one relation, inheritance is not expanded */
- Assert(nrels == 1);
+ Assert(total_nrels == 1);
resultRelInfo = mtstate->resultRelInfo;
if (!resultRelInfo->ri_usesFdwDirectModify &&
resultRelInfo->ri_FdwRoutine != NULL &&
diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index a4d523dcb0f..141177e7413 100644
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -562,6 +562,7 @@ standard_planner(Query *parse, const char *query_string, int cursorOptions,
glob->prunableRelids);
result->permInfos = glob->finalrteperminfos;
result->resultRelations = glob->resultRelations;
+ result->firstResultRels = glob->firstResultRels;
result->appendRelations = glob->appendRelations;
result->subplans = glob->subplans;
result->rewindPlanIDs = glob->rewindPlanIDs;
diff --git a/src/backend/optimizer/plan/setrefs.c b/src/backend/optimizer/plan/setrefs.c
index 999a5a8ab5a..150e9f060ee 100644
--- a/src/backend/optimizer/plan/setrefs.c
+++ b/src/backend/optimizer/plan/setrefs.c
@@ -1248,6 +1248,9 @@ set_plan_refs(PlannerInfo *root, Plan *plan, int rtoffset)
lappend_int(root->glob->resultRelations,
splan->rootRelation);
}
+ root->glob->firstResultRels =
+ lappend_int(root->glob->firstResultRels,
+ linitial_int(splan->resultRelations));
}
break;
case T_Append:
diff --git a/src/include/executor/execPartition.h b/src/include/executor/execPartition.h
index 626613012f9..3b3f46aced0 100644
--- a/src/include/executor/execPartition.h
+++ b/src/include/executor/execPartition.h
@@ -43,8 +43,8 @@ extern void ExecCleanupTupleRouting(ModifyTableState *mtstate,
* subpart_map contains indexes into PartitionPruningData.partrelprunedata[].
*
* partrel Partitioned table Relation; obtained by
- * ExecGetRangeTableRelation(estate, rti), where
- * rti is PartitionedRelPruneInfo.rtindex.
+ * ExecGetRangeTableRelation(estate, rti, false),
+ * where rti is PartitionedRelPruneInfo.rtindex.
* nparts Length of subplan_map[] and subpart_map[].
* subplan_map Subplan index by partition index, or -1.
* subpart_map Subpart index by partition index, or -1.
diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h
index 0d2ffabda68..3255bccb038 100644
--- a/src/include/executor/executor.h
+++ b/src/include/executor/executor.h
@@ -680,7 +680,7 @@ exec_rt_fetch(Index rti, EState *estate)
return (RangeTblEntry *) list_nth(estate->es_range_table, rti - 1);
}
-extern Relation ExecGetRangeTableRelation(EState *estate, Index rti);
+extern Relation ExecGetRangeTableRelation(EState *estate, Index rti, bool result_rel);
extern void ExecInitResultRelation(EState *estate, ResultRelInfo *resultRelInfo,
Index rti);
diff --git a/src/include/nodes/pathnodes.h b/src/include/nodes/pathnodes.h
index fbf05322c75..f8e5671726e 100644
--- a/src/include/nodes/pathnodes.h
+++ b/src/include/nodes/pathnodes.h
@@ -138,6 +138,9 @@ typedef struct PlannerGlobal
/* "flat" list of integer RT indexes */
List *resultRelations;
+ /* "flat" list of integer RT indexes */
+ List *firstResultRels;
+
/* "flat" list of AppendRelInfos */
List *appendRelations;
diff --git a/src/include/nodes/plannodes.h b/src/include/nodes/plannodes.h
index 22841211f48..f78bffd90cf 100644
--- a/src/include/nodes/plannodes.h
+++ b/src/include/nodes/plannodes.h
@@ -102,6 +102,13 @@ typedef struct PlannedStmt
/* integer list of RT indexes, or NIL */
List *resultRelations;
+ /*
+ * rtable indexes of first target relation in each ModifyTable node in the
+ * plan for INSERT/UPDATE/DELETE/MERGE
+ */
+ /* integer list of RT indexes, or NIL */
+ List *firstResultRels;
+
/* list of AppendRelInfo nodes */
List *appendRelations;
diff --git a/src/test/regress/expected/partition_prune.out b/src/test/regress/expected/partition_prune.out
index 8097f4e9282..0bf35260b46 100644
--- a/src/test/regress/expected/partition_prune.out
+++ b/src/test/regress/expected/partition_prune.out
@@ -4662,6 +4662,88 @@ table part_abc_view;
2 | c | t
(1 row)
+-- MERGE ... INSERT when all pruned from MERGE source.
+begin;
+explain (costs off)
+merge into part_abc_view pt
+using (select stable_one() + 1 as pid) as q join part_abc_2 pt2 on (q.pid = pt2.a) on pt.a = stable_one() + 2
+when not matched then insert values (1, 'd', false) returning pt.a;
+ QUERY PLAN
+------------------------------------------------
+ Merge on part_abc
+ -> Nested Loop Left Join
+ -> Seq Scan on part_abc_2 pt2
+ Filter: ((stable_one() + 1) = a)
+ -> Materialize
+ -> Append
+ Subplans Removed: 2
+(7 rows)
+
+merge into part_abc_view pt
+using (select stable_one() + 1 as pid) as q join part_abc_2 pt2 on (q.pid = pt2.a) on pt.a = stable_one() + 2
+when not matched then insert values (1, 'd', false) returning pt.a;
+ a
+---
+ 1
+(1 row)
+
+table part_abc_view;
+ a | b | c
+---+---+---
+ 1 | d | f
+ 2 | c | t
+(2 rows)
+
+rollback;
+-- A case with multiple ModifyTable nodes.
+begin;
+create table part_abc_log (action text, a int, b text, c bool);
+explain (costs off)
+with t as (
+ merge into part_abc_view pt
+ using (select stable_one() + 1 as pid) as q join part_abc_2 pt2 on (q.pid = pt2.a) on pt.a = stable_one() + 2
+ when not matched then insert values (1, 'd', false) returning merge_action(), pt.*
+)
+insert into part_abc_log select * from t returning *;
+ QUERY PLAN
+--------------------------------------------------------
+ Insert on part_abc_log
+ CTE t
+ -> Merge on part_abc
+ -> Nested Loop Left Join
+ -> Seq Scan on part_abc_2 pt2
+ Filter: ((stable_one() + 1) = a)
+ -> Materialize
+ -> Append
+ Subplans Removed: 2
+ -> CTE Scan on t
+(10 rows)
+
+with t as (
+ merge into part_abc_view pt
+ using (select stable_one() + 1 as pid) as q join part_abc_2 pt2 on (q.pid = pt2.a) on pt.a = stable_one() + 2
+ when not matched then insert values (1, 'd', false) returning merge_action(), pt.*
+)
+insert into part_abc_log select * from t returning *;
+ action | a | b | c
+--------+---+---+---
+ INSERT | 1 | d | f
+(1 row)
+
+table part_abc_view;
+ a | b | c
+---+---+---
+ 1 | d | f
+ 2 | c | t
+(2 rows)
+
+table part_abc_log;
+ action | a | b | c
+--------+---+---+---
+ INSERT | 1 | d | f
+(1 row)
+
+rollback;
-- A case with nested MergeAppend with its own PartitionPruneInfo.
create index on part_abc (a);
alter table part_abc add d int;
diff --git a/src/test/regress/sql/partition_prune.sql b/src/test/regress/sql/partition_prune.sql
index 4a2c74b0899..f6db9479f54 100644
--- a/src/test/regress/sql/partition_prune.sql
+++ b/src/test/regress/sql/partition_prune.sql
@@ -1401,6 +1401,38 @@ using (select stable_one() + 2 as pid) as q join part_abc_1 pt1 on (q.pid = pt1.
when matched then delete returning pt.a;
table part_abc_view;
+-- MERGE ... INSERT when all pruned from MERGE source.
+begin;
+explain (costs off)
+merge into part_abc_view pt
+using (select stable_one() + 1 as pid) as q join part_abc_2 pt2 on (q.pid = pt2.a) on pt.a = stable_one() + 2
+when not matched then insert values (1, 'd', false) returning pt.a;
+merge into part_abc_view pt
+using (select stable_one() + 1 as pid) as q join part_abc_2 pt2 on (q.pid = pt2.a) on pt.a = stable_one() + 2
+when not matched then insert values (1, 'd', false) returning pt.a;
+table part_abc_view;
+rollback;
+
+-- A case with multiple ModifyTable nodes.
+begin;
+create table part_abc_log (action text, a int, b text, c bool);
+explain (costs off)
+with t as (
+ merge into part_abc_view pt
+ using (select stable_one() + 1 as pid) as q join part_abc_2 pt2 on (q.pid = pt2.a) on pt.a = stable_one() + 2
+ when not matched then insert values (1, 'd', false) returning merge_action(), pt.*
+)
+insert into part_abc_log select * from t returning *;
+with t as (
+ merge into part_abc_view pt
+ using (select stable_one() + 1 as pid) as q join part_abc_2 pt2 on (q.pid = pt2.a) on pt.a = stable_one() + 2
+ when not matched then insert values (1, 'd', false) returning merge_action(), pt.*
+)
+insert into part_abc_log select * from t returning *;
+table part_abc_view;
+table part_abc_log;
+rollback;
+
-- A case with nested MergeAppend with its own PartitionPruneInfo.
create index on part_abc (a);
alter table part_abc add d int;
--
2.43.0
* Re: BUG #18830: ExecInitMerge Segfault on MERGE
2025-03-02 21:00 BUG #18830: ExecInitMerge Segfault on MERGE PG Bug reporting form <noreply@postgresql.org>
2025-03-03 15:46 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Tender Wang <tndrwang@gmail.com>
2025-03-04 06:58 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Tender Wang <tndrwang@gmail.com>
2025-03-04 09:29 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE David Rowley <dgrowleyml@gmail.com>
2025-03-04 18:56 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Dean Rasheed <dean.a.rasheed@gmail.com>
2025-03-12 12:24 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Amit Langote <amitlangote09@gmail.com>
2025-03-12 14:59 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Dean Rasheed <dean.a.rasheed@gmail.com>
2025-03-12 16:11 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Tender Wang <tndrwang@gmail.com>
2025-03-12 19:23 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Dean Rasheed <dean.a.rasheed@gmail.com>
2025-03-13 12:44 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Amit Langote <amitlangote09@gmail.com>
@ 2025-03-13 14:42 ` Dean Rasheed <dean.a.rasheed@gmail.com>
2025-03-14 02:48 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Amit Langote <amitlangote09@gmail.com>
0 siblings, 1 reply; 32+ messages in thread
From: Dean Rasheed @ 2025-03-13 14:42 UTC (permalink / raw)
To: Amit Langote <amitlangote09@gmail.com>; +Cc: Tender Wang <tndrwang@gmail.com>; David Rowley <dgrowleyml@gmail.com>; pgsql-bugs@lists.postgresql.org; tharakan@gmail.com
On Thu, 13 Mar 2025 at 12:44, Amit Langote <amitlangote09@gmail.com> wrote:
>
> One thing I'd like to avoid is taking any locks during ExecInitNode(),
> including in functions like ExecGetRangeTableRelation(). Doing so
> would require checking whether the CachedPlan is still valid and
> handling invalidation if it isn't -- see commit 525392d5727f for
> background.
>
Yes, I wondered about that. I saw that earlier commit, and the notes
in the executor README, but I still don't quite understand.
Specifically, how is locking this pruned leaf partition in
ExecInitModifyTable() different from locking any other pruned leaf
partition later in the INSERT path? (For example, as part of an UPDATE
that moves a tuple from an unpruned partition to a pruned one.)
Regards,
Dean
^ permalink raw reply [nested|flat] 32+ messages in thread
* Re: BUG #18830: ExecInitMerge Segfault on MERGE
2025-03-02 21:00 BUG #18830: ExecInitMerge Segfault on MERGE PG Bug reporting form <noreply@postgresql.org>
2025-03-03 15:46 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Tender Wang <tndrwang@gmail.com>
2025-03-04 06:58 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Tender Wang <tndrwang@gmail.com>
2025-03-04 09:29 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE David Rowley <dgrowleyml@gmail.com>
2025-03-04 18:56 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Dean Rasheed <dean.a.rasheed@gmail.com>
2025-03-12 12:24 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Amit Langote <amitlangote09@gmail.com>
2025-03-12 14:59 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Dean Rasheed <dean.a.rasheed@gmail.com>
2025-03-12 16:11 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Tender Wang <tndrwang@gmail.com>
2025-03-12 19:23 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Dean Rasheed <dean.a.rasheed@gmail.com>
2025-03-13 12:44 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Amit Langote <amitlangote09@gmail.com>
2025-03-13 14:42 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Dean Rasheed <dean.a.rasheed@gmail.com>
@ 2025-03-14 02:48 ` Amit Langote <amitlangote09@gmail.com>
2025-03-16 09:57 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Dean Rasheed <dean.a.rasheed@gmail.com>
0 siblings, 1 reply; 32+ messages in thread
From: Amit Langote @ 2025-03-14 02:48 UTC (permalink / raw)
To: Dean Rasheed <dean.a.rasheed@gmail.com>; +Cc: Tender Wang <tndrwang@gmail.com>; David Rowley <dgrowleyml@gmail.com>; pgsql-bugs@lists.postgresql.org; tharakan@gmail.com
On Thu, Mar 13, 2025 at 11:42 PM Dean Rasheed <dean.a.rasheed@gmail.com> wrote:
> On Thu, 13 Mar 2025 at 12:44, Amit Langote <amitlangote09@gmail.com> wrote:
> >
> > One thing I'd like to avoid is taking any locks during ExecInitNode(),
> > including in functions like ExecGetRangeTableRelation(). Doing so
> > would require checking whether the CachedPlan is still valid and
> > handling invalidation if it isn't -- see commit 525392d5727f for
> > background.
>
> Yes, I wondered about that. I saw that earlier commit, and the notes
> in the executor README, but I still don't quite understand.
> Specifically, how is locking this pruned leaf partition in
> ExecInitModifyTable() different from locking any other pruned leaf
> partition later in the INSERT path? (For example, as part of an UPDATE
> that moves a tuple from an unpruned partition to a pruned one.)
I think the key distinction is that the result relation we’re locking
was already present in the plan, and the executor code that uses
resultRelInfo[0] doesn’t rely on traversing the plan tree -- it
operates entirely on the state info built in ExecInitModifyTable(). So
in this specific case, the usual plan invalidation hazard may not
apply in the same way.
I understand the comparison to locking pruned partitions during UPDATE
execution, but I think that case is different: by then, the executor
is fully initialized, and while invalidations can still happen,
they’re assumed to be harmless for the remainder of execution. That’s
why the system doesn’t check for plan validity or attempt to restart
once execution has begun -- it’s expected to continue with the
existing plan, under the assumption that any changes would only affect
pruned partitions that are unreachable, especially via the scan path.
Since those plan nodes remain unreferenced during tuple routing which
only uses state initialized during execution, such changes don’t
impact correctness.
That said, my concern remains that taking any new locks during
ExecInitNode() risks triggering CachedPlan validation logic that we’ve
intentionally avoided dealing with, as per 525392d5727f. Even if this
particular usage might not trip over it, it still feels like we’re
stepping into a sensitive area. So I’d prefer to keep the locking in
ExecDoInitialPruning(), where interacting with plan invalidation is
expected and safe — and we don’t risk leaving the executor in a
partially initialized state.
--
Thanks, Amit Langote
^ permalink raw reply [nested|flat] 32+ messages in thread
* Re: BUG #18830: ExecInitMerge Segfault on MERGE
2025-03-02 21:00 BUG #18830: ExecInitMerge Segfault on MERGE PG Bug reporting form <noreply@postgresql.org>
2025-03-03 15:46 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Tender Wang <tndrwang@gmail.com>
2025-03-04 06:58 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Tender Wang <tndrwang@gmail.com>
2025-03-04 09:29 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE David Rowley <dgrowleyml@gmail.com>
2025-03-04 18:56 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Dean Rasheed <dean.a.rasheed@gmail.com>
2025-03-12 12:24 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Amit Langote <amitlangote09@gmail.com>
2025-03-12 14:59 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Dean Rasheed <dean.a.rasheed@gmail.com>
2025-03-12 16:11 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Tender Wang <tndrwang@gmail.com>
2025-03-12 19:23 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Dean Rasheed <dean.a.rasheed@gmail.com>
2025-03-13 12:44 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Amit Langote <amitlangote09@gmail.com>
2025-03-13 14:42 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Dean Rasheed <dean.a.rasheed@gmail.com>
2025-03-14 02:48 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Amit Langote <amitlangote09@gmail.com>
@ 2025-03-16 09:57 ` Dean Rasheed <dean.a.rasheed@gmail.com>
2025-03-17 12:21 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Amit Langote <amitlangote09@gmail.com>
0 siblings, 1 reply; 32+ messages in thread
From: Dean Rasheed @ 2025-03-16 09:57 UTC (permalink / raw)
To: Amit Langote <amitlangote09@gmail.com>; +Cc: Tender Wang <tndrwang@gmail.com>; David Rowley <dgrowleyml@gmail.com>; pgsql-bugs@lists.postgresql.org; tharakan@gmail.com
On Fri, 14 Mar 2025 at 02:48, Amit Langote <amitlangote09@gmail.com> wrote:
>
> That said, my concern remains that taking any new locks during
> ExecInitNode() risks triggering CachedPlan validation logic that we’ve
> intentionally avoided dealing with, as per 525392d5727f. Even if this
> particular usage might not trip over it, it still feels like we’re
> stepping into a sensitive area. So I’d prefer to keep the locking in
> ExecDoInitialPruning(), where interacting with plan invalidation is
> expected and safe — and we don’t risk leaving the executor in a
> partially initialized state.
>
OK, I think that makes sense. Trapping any plan invalidations early,
after ExecDoInitialPruning(), and before initialising the rest of the
executor state does seem preferable for that reason.
I have a couple of comments on the v3 patch:
* In ExecInitModifyTable(), it's possible to avoid code duplication. I
think that's worth it to make the code more maintainable if more
per-result relation logic is added in the future.
* In executor.h, the name of the new function argument doesn't match
the .c source file.
* In ExecDoInitialPruning(), there is room for improvement: we
actually only need to lock the first result relation if all other
result relations of the ModifyTable node have been pruned. As it
stands, there's no easy way to tell that, so I've just made a note of
it in a comment. I wondered about replacing the new field
"firstResultRels" with something like "mtResultRels", which would be a
list of lists, to allow that, but I didn't try it.
I'm attaching v4, which addresses those comments, and includes a few
other comment update suggestions.
Regards,
Dean
Attachments:
v4-Do-not-allow-all-ModifyTable-result-relations-to-be-pruned.patch (text/x-patch, 17.3K) download
diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
new file mode 100644
index 19ffcc2..3af6cf5
--- a/src/backend/commands/explain.c
+++ b/src/backend/commands/explain.c
@@ -4695,10 +4695,20 @@ show_modifytable_info(ModifyTableState *
break;
}
- /* Should we explicitly label target relations? */
+ /*
+ * Should we explicitly label target relations?
+ *
+ * If there's only one target relation, do not list it if it's the
+ * relation named in the query, or if it has been pruned. (Normally
+ * mtstate->resultRelInfo doesn't include pruned relations, but a single
+ * pruned target relation may be present, if all other target relations
+ * have been pruned. See ExecInitModifyTable().)
+ */
labeltargets = (mtstate->mt_nrels > 1 ||
(mtstate->mt_nrels == 1 &&
- mtstate->resultRelInfo[0].ri_RangeTableIndex != node->nominalRelation));
+ mtstate->resultRelInfo[0].ri_RangeTableIndex != node->nominalRelation &&
+ bms_is_member(mtstate->resultRelInfo[0].ri_RangeTableIndex,
+ mtstate->ps.state->es_unpruned_relids)));
if (labeltargets)
ExplainOpenGroup("Target Tables", "Target Tables", false, es);
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
new file mode 100644
index 0493b7d..e9bd98c
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -1006,7 +1006,7 @@ InitPlan(QueryDesc *queryDesc, int eflag
case ROW_MARK_SHARE:
case ROW_MARK_KEYSHARE:
case ROW_MARK_REFERENCE:
- relation = ExecGetRangeTableRelation(estate, rc->rti);
+ relation = ExecGetRangeTableRelation(estate, rc->rti, false);
break;
case ROW_MARK_COPY:
/* no physical table access is required */
diff --git a/src/backend/executor/execPartition.c b/src/backend/executor/execPartition.c
new file mode 100644
index 5cd5e2e..84ccd7d
--- a/src/backend/executor/execPartition.c
+++ b/src/backend/executor/execPartition.c
@@ -1819,6 +1819,7 @@ adjust_partition_colnos_using_map(List *
void
ExecDoInitialPruning(EState *estate)
{
+ PlannedStmt *stmt = estate->es_plannedstmt;
ListCell *lc;
List *locked_relids = NIL;
@@ -1868,6 +1869,34 @@ ExecDoInitialPruning(EState *estate)
}
/*
+ * Lock the first result relation of each ModifyTable node, even if it was
+ * pruned. This is required for ExecInitModifyTable(), which keeps its
+ * first result relation if all other result relations have been pruned,
+ * because some executor paths (e.g., in nodeModifyTable.c and
+ * execPartition.c) rely on there being at least one result relation.
+ *
+ * There's room for improvement here --- we actually only need to do this
+ * if all other result relations of the ModifyTable node were pruned, but
+ * we don't have an easy way to tell that here.
+ */
+ if (stmt->resultRelations && ExecShouldLockRelations(estate))
+ {
+ foreach(lc, stmt->firstResultRels)
+ {
+ Index firstResultRel = lfirst_int(lc);
+
+ if (!bms_is_member(firstResultRel, estate->es_unpruned_relids))
+ {
+ RangeTblEntry *rte = exec_rt_fetch(firstResultRel, estate);
+
+ Assert(rte->rtekind == RTE_RELATION && rte->rellockmode != NoLock);
+ LockRelationOid(rte->relid, rte->rellockmode);
+ locked_relids = lappend_int(locked_relids, firstResultRel);
+ }
+ }
+ }
+
+ /*
* Release the useless locks if the plan won't be executed. This is the
* same as what CheckCachedPlan() in plancache.c does.
*/
@@ -2076,7 +2105,7 @@ CreatePartitionPruneState(EState *estate
* because that entry will be held open and locked for the
* duration of this executor run.
*/
- partrel = ExecGetRangeTableRelation(estate, pinfo->rtindex);
+ partrel = ExecGetRangeTableRelation(estate, pinfo->rtindex, false);
/* Remember for InitExecPartitionPruneContext(). */
pprune->partrel = partrel;
diff --git a/src/backend/executor/execUtils.c b/src/backend/executor/execUtils.c
new file mode 100644
index 39d6f4d..55ab18f
--- a/src/backend/executor/execUtils.c
+++ b/src/backend/executor/execUtils.c
@@ -746,7 +746,7 @@ ExecOpenScanRelation(EState *estate, Ind
Relation rel;
/* Open the relation. */
- rel = ExecGetRangeTableRelation(estate, scanrelid);
+ rel = ExecGetRangeTableRelation(estate, scanrelid, false);
/*
* Complain if we're attempting a scan of an unscannable relation, except
@@ -815,18 +815,22 @@ ExecInitRangeTable(EState *estate, List
*
* The Relations will be closed in ExecEndPlan().
*
- * Note: The caller must ensure that 'rti' refers to an unpruned relation
- * (i.e., it is a member of estate->es_unpruned_relids) before calling this
- * function. Attempting to open a pruned relation will result in an error.
+ * If isResultRel is true, the relation is being used as a result relation.
+ * Such a relation might have been pruned, which is OK for result relations,
+ * but not for scan relations; see the details in ExecInitModifyTable(). If
+ * isResultRel is false, the caller must ensure that 'rti' refers to an
+ * unpruned relation (i.e., it is a member of estate->es_unpruned_relids)
+ * before calling this function. Attempting to open a pruned relation for
+ * scanning will result in an error.
*/
Relation
-ExecGetRangeTableRelation(EState *estate, Index rti)
+ExecGetRangeTableRelation(EState *estate, Index rti, bool isResultRel)
{
Relation rel;
Assert(rti > 0 && rti <= estate->es_range_table_size);
- if (!bms_is_member(rti, estate->es_unpruned_relids))
+ if (!isResultRel && !bms_is_member(rti, estate->es_unpruned_relids))
elog(ERROR, "trying to open a pruned relation");
rel = estate->es_relations[rti - 1];
@@ -880,7 +884,7 @@ ExecInitResultRelation(EState *estate, R
{
Relation resultRelationDesc;
- resultRelationDesc = ExecGetRangeTableRelation(estate, rti);
+ resultRelationDesc = ExecGetRangeTableRelation(estate, rti, true);
InitResultRelInfo(resultRelInfo,
resultRelationDesc,
rti,
diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
new file mode 100644
index b0fe500..87c8202
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -4471,6 +4471,7 @@ ExecInitModifyTable(ModifyTable *node, E
ModifyTableState *mtstate;
Plan *subplan = outerPlan(node);
CmdType operation = node->operation;
+ int total_nrels = list_length(node->resultRelations);
int nrels;
List *resultRelations = NIL;
List *withCheckOptionLists = NIL;
@@ -4490,13 +4491,35 @@ ExecInitModifyTable(ModifyTable *node, E
/*
* Only consider unpruned relations for initializing their ResultRelInfo
* struct and other fields such as withCheckOptions, etc.
+ *
+ * Note: We must avoid pruning every result relation. This is important
+ * for MERGE, since even if every result relation is pruned from the
+ * subplan, there might still be NOT MATCHED rows, for which there may be
+ * INSERT actions to perform. To allow these actions to be found, at
+ * least one result relation must be kept. Also, when inserting into a
+ * partitioned table, ExecInitPartitionInfo() needs a ResultRelInfo struct
+ * as a reference for building the ResultRelInfo of the target partition.
+ * In either case, it doesn't matter which result relation is kept, so we
+ * just keep the first one, if all others have been pruned. See also,
+ * ExecDoInitialPruning(), which ensures that this first result relation
+ * has been locked.
*/
i = 0;
foreach(l, node->resultRelations)
{
Index rti = lfirst_int(l);
+ bool keep_rel;
- if (bms_is_member(rti, estate->es_unpruned_relids))
+ keep_rel = bms_is_member(rti, estate->es_unpruned_relids);
+ if (!keep_rel && i == total_nrels - 1 && resultRelations == NIL)
+ {
+ /* all result relations pruned; keep the first one */
+ keep_rel = true;
+ rti = linitial_int(node->resultRelations);
+ i = 0;
+ }
+
+ if (keep_rel)
{
resultRelations = lappend_int(resultRelations, rti);
if (node->withCheckOptionLists)
@@ -4537,6 +4560,7 @@ ExecInitModifyTable(ModifyTable *node, E
i++;
}
nrels = list_length(resultRelations);
+ Assert(nrels > 0);
/*
* create state structure
@@ -4735,7 +4759,7 @@ ExecInitModifyTable(ModifyTable *node, E
*/
mtstate->mt_resultOidAttno =
ExecFindJunkAttributeInTlist(subplan->targetlist, "tableoid");
- Assert(AttributeNumberIsValid(mtstate->mt_resultOidAttno) || nrels == 1);
+ Assert(AttributeNumberIsValid(mtstate->mt_resultOidAttno) || total_nrels == 1);
mtstate->mt_lastResultOid = InvalidOid; /* force lookup at first tuple */
mtstate->mt_lastResultIndex = 0; /* must be zero if no such attr */
@@ -4832,7 +4856,7 @@ ExecInitModifyTable(ModifyTable *node, E
if (node->onConflictAction != ONCONFLICT_NONE)
{
/* insert may only have one relation, inheritance is not expanded */
- Assert(nrels == 1);
+ Assert(total_nrels == 1);
resultRelInfo->ri_onConflictArbiterIndexes = node->arbiterIndexes;
}
@@ -4979,7 +5003,7 @@ ExecInitModifyTable(ModifyTable *node, E
if (operation == CMD_INSERT)
{
/* insert may only have one relation, inheritance is not expanded */
- Assert(nrels == 1);
+ Assert(total_nrels == 1);
resultRelInfo = mtstate->resultRelInfo;
if (!resultRelInfo->ri_usesFdwDirectModify &&
resultRelInfo->ri_FdwRoutine != NULL &&
diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
new file mode 100644
index a4d523d..141177e
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -562,6 +562,7 @@ standard_planner(Query *parse, const cha
glob->prunableRelids);
result->permInfos = glob->finalrteperminfos;
result->resultRelations = glob->resultRelations;
+ result->firstResultRels = glob->firstResultRels;
result->appendRelations = glob->appendRelations;
result->subplans = glob->subplans;
result->rewindPlanIDs = glob->rewindPlanIDs;
diff --git a/src/backend/optimizer/plan/setrefs.c b/src/backend/optimizer/plan/setrefs.c
new file mode 100644
index 999a5a8..150e9f0
--- a/src/backend/optimizer/plan/setrefs.c
+++ b/src/backend/optimizer/plan/setrefs.c
@@ -1248,6 +1248,9 @@ set_plan_refs(PlannerInfo *root, Plan *p
lappend_int(root->glob->resultRelations,
splan->rootRelation);
}
+ root->glob->firstResultRels =
+ lappend_int(root->glob->firstResultRels,
+ linitial_int(splan->resultRelations));
}
break;
case T_Append:
diff --git a/src/include/executor/execPartition.h b/src/include/executor/execPartition.h
new file mode 100644
index 6266130..3b3f46a
--- a/src/include/executor/execPartition.h
+++ b/src/include/executor/execPartition.h
@@ -43,8 +43,8 @@ extern void ExecCleanupTupleRouting(Modi
* subpart_map contains indexes into PartitionPruningData.partrelprunedata[].
*
* partrel Partitioned table Relation; obtained by
- * ExecGetRangeTableRelation(estate, rti), where
- * rti is PartitionedRelPruneInfo.rtindex.
+ * ExecGetRangeTableRelation(estate, rti, false),
+ * where rti is PartitionedRelPruneInfo.rtindex.
* nparts Length of subplan_map[] and subpart_map[].
* subplan_map Subplan index by partition index, or -1.
* subpart_map Subpart index by partition index, or -1.
diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h
new file mode 100644
index 0d2ffab..0db5d18
--- a/src/include/executor/executor.h
+++ b/src/include/executor/executor.h
@@ -680,7 +680,8 @@ exec_rt_fetch(Index rti, EState *estate)
return (RangeTblEntry *) list_nth(estate->es_range_table, rti - 1);
}
-extern Relation ExecGetRangeTableRelation(EState *estate, Index rti);
+extern Relation ExecGetRangeTableRelation(EState *estate, Index rti,
+ bool isResultRel);
extern void ExecInitResultRelation(EState *estate, ResultRelInfo *resultRelInfo,
Index rti);
diff --git a/src/include/nodes/pathnodes.h b/src/include/nodes/pathnodes.h
new file mode 100644
index fbf0532..c24a1fc
--- a/src/include/nodes/pathnodes.h
+++ b/src/include/nodes/pathnodes.h
@@ -138,6 +138,9 @@ typedef struct PlannerGlobal
/* "flat" list of integer RT indexes */
List *resultRelations;
+ /* "flat" list of integer RT indexes (one per ModifyTable node) */
+ List *firstResultRels;
+
/* "flat" list of AppendRelInfos */
List *appendRelations;
diff --git a/src/include/nodes/plannodes.h b/src/include/nodes/plannodes.h
new file mode 100644
index 2284121..f78bffd
--- a/src/include/nodes/plannodes.h
+++ b/src/include/nodes/plannodes.h
@@ -102,6 +102,13 @@ typedef struct PlannedStmt
/* integer list of RT indexes, or NIL */
List *resultRelations;
+ /*
+ * rtable indexes of first target relation in each ModifyTable node in the
+ * plan for INSERT/UPDATE/DELETE/MERGE
+ */
+ /* integer list of RT indexes, or NIL */
+ List *firstResultRels;
+
/* list of AppendRelInfo nodes */
List *appendRelations;
diff --git a/src/test/regress/expected/partition_prune.out b/src/test/regress/expected/partition_prune.out
new file mode 100644
index 8097f4e..0bf3526
--- a/src/test/regress/expected/partition_prune.out
+++ b/src/test/regress/expected/partition_prune.out
@@ -4662,6 +4662,88 @@ table part_abc_view;
2 | c | t
(1 row)
+-- MERGE ... INSERT when all pruned from MERGE source.
+begin;
+explain (costs off)
+merge into part_abc_view pt
+using (select stable_one() + 1 as pid) as q join part_abc_2 pt2 on (q.pid = pt2.a) on pt.a = stable_one() + 2
+when not matched then insert values (1, 'd', false) returning pt.a;
+ QUERY PLAN
+------------------------------------------------
+ Merge on part_abc
+ -> Nested Loop Left Join
+ -> Seq Scan on part_abc_2 pt2
+ Filter: ((stable_one() + 1) = a)
+ -> Materialize
+ -> Append
+ Subplans Removed: 2
+(7 rows)
+
+merge into part_abc_view pt
+using (select stable_one() + 1 as pid) as q join part_abc_2 pt2 on (q.pid = pt2.a) on pt.a = stable_one() + 2
+when not matched then insert values (1, 'd', false) returning pt.a;
+ a
+---
+ 1
+(1 row)
+
+table part_abc_view;
+ a | b | c
+---+---+---
+ 1 | d | f
+ 2 | c | t
+(2 rows)
+
+rollback;
+-- A case with multiple ModifyTable nodes.
+begin;
+create table part_abc_log (action text, a int, b text, c bool);
+explain (costs off)
+with t as (
+ merge into part_abc_view pt
+ using (select stable_one() + 1 as pid) as q join part_abc_2 pt2 on (q.pid = pt2.a) on pt.a = stable_one() + 2
+ when not matched then insert values (1, 'd', false) returning merge_action(), pt.*
+)
+insert into part_abc_log select * from t returning *;
+ QUERY PLAN
+--------------------------------------------------------
+ Insert on part_abc_log
+ CTE t
+ -> Merge on part_abc
+ -> Nested Loop Left Join
+ -> Seq Scan on part_abc_2 pt2
+ Filter: ((stable_one() + 1) = a)
+ -> Materialize
+ -> Append
+ Subplans Removed: 2
+ -> CTE Scan on t
+(10 rows)
+
+with t as (
+ merge into part_abc_view pt
+ using (select stable_one() + 1 as pid) as q join part_abc_2 pt2 on (q.pid = pt2.a) on pt.a = stable_one() + 2
+ when not matched then insert values (1, 'd', false) returning merge_action(), pt.*
+)
+insert into part_abc_log select * from t returning *;
+ action | a | b | c
+--------+---+---+---
+ INSERT | 1 | d | f
+(1 row)
+
+table part_abc_view;
+ a | b | c
+---+---+---
+ 1 | d | f
+ 2 | c | t
+(2 rows)
+
+table part_abc_log;
+ action | a | b | c
+--------+---+---+---
+ INSERT | 1 | d | f
+(1 row)
+
+rollback;
-- A case with nested MergeAppend with its own PartitionPruneInfo.
create index on part_abc (a);
alter table part_abc add d int;
diff --git a/src/test/regress/sql/partition_prune.sql b/src/test/regress/sql/partition_prune.sql
new file mode 100644
index 4a2c74b..f6db947
--- a/src/test/regress/sql/partition_prune.sql
+++ b/src/test/regress/sql/partition_prune.sql
@@ -1401,6 +1401,38 @@ using (select stable_one() + 2 as pid) a
when matched then delete returning pt.a;
table part_abc_view;
+-- MERGE ... INSERT when all pruned from MERGE source.
+begin;
+explain (costs off)
+merge into part_abc_view pt
+using (select stable_one() + 1 as pid) as q join part_abc_2 pt2 on (q.pid = pt2.a) on pt.a = stable_one() + 2
+when not matched then insert values (1, 'd', false) returning pt.a;
+merge into part_abc_view pt
+using (select stable_one() + 1 as pid) as q join part_abc_2 pt2 on (q.pid = pt2.a) on pt.a = stable_one() + 2
+when not matched then insert values (1, 'd', false) returning pt.a;
+table part_abc_view;
+rollback;
+
+-- A case with multiple ModifyTable nodes.
+begin;
+create table part_abc_log (action text, a int, b text, c bool);
+explain (costs off)
+with t as (
+ merge into part_abc_view pt
+ using (select stable_one() + 1 as pid) as q join part_abc_2 pt2 on (q.pid = pt2.a) on pt.a = stable_one() + 2
+ when not matched then insert values (1, 'd', false) returning merge_action(), pt.*
+)
+insert into part_abc_log select * from t returning *;
+with t as (
+ merge into part_abc_view pt
+ using (select stable_one() + 1 as pid) as q join part_abc_2 pt2 on (q.pid = pt2.a) on pt.a = stable_one() + 2
+ when not matched then insert values (1, 'd', false) returning merge_action(), pt.*
+)
+insert into part_abc_log select * from t returning *;
+table part_abc_view;
+table part_abc_log;
+rollback;
+
-- A case with nested MergeAppend with its own PartitionPruneInfo.
create index on part_abc (a);
alter table part_abc add d int;
* Re: BUG #18830: ExecInitMerge Segfault on MERGE
2025-03-02 21:00 BUG #18830: ExecInitMerge Segfault on MERGE PG Bug reporting form <noreply@postgresql.org>
2025-03-03 15:46 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Tender Wang <tndrwang@gmail.com>
2025-03-04 06:58 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Tender Wang <tndrwang@gmail.com>
2025-03-04 09:29 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE David Rowley <dgrowleyml@gmail.com>
2025-03-04 18:56 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Dean Rasheed <dean.a.rasheed@gmail.com>
2025-03-12 12:24 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Amit Langote <amitlangote09@gmail.com>
2025-03-12 14:59 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Dean Rasheed <dean.a.rasheed@gmail.com>
2025-03-12 16:11 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Tender Wang <tndrwang@gmail.com>
2025-03-12 19:23 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Dean Rasheed <dean.a.rasheed@gmail.com>
2025-03-13 12:44 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Amit Langote <amitlangote09@gmail.com>
2025-03-13 14:42 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Dean Rasheed <dean.a.rasheed@gmail.com>
2025-03-14 02:48 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Amit Langote <amitlangote09@gmail.com>
2025-03-16 09:57 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Dean Rasheed <dean.a.rasheed@gmail.com>
@ 2025-03-17 12:21 ` Amit Langote <amitlangote09@gmail.com>
2025-03-17 23:35 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Dean Rasheed <dean.a.rasheed@gmail.com>
0 siblings, 1 reply; 32+ messages in thread
From: Amit Langote @ 2025-03-17 12:21 UTC (permalink / raw)
To: Dean Rasheed <dean.a.rasheed@gmail.com>; +Cc: Tender Wang <tndrwang@gmail.com>; David Rowley <dgrowleyml@gmail.com>; pgsql-bugs@lists.postgresql.org; tharakan@gmail.com
On Sun, Mar 16, 2025 at 6:57 PM Dean Rasheed <dean.a.rasheed@gmail.com> wrote:
> On Fri, 14 Mar 2025 at 02:48, Amit Langote <amitlangote09@gmail.com> wrote:>
> > That said, my concern remains that taking any new locks during
> > ExecInitNode() risks triggering CachedPlan validation logic that we’ve
> > intentionally avoided dealing with, as per 525392d5727f. Even if this
> > particular usage might not trip over it, it still feels like we’re
> > stepping into a sensitive area. So I’d prefer to keep the locking in
> > ExecDoInitialPruning(), where interacting with plan invalidation is
> > expected and safe — and we don’t risk leaving the executor in a
> > partially initialized state.
> >
>
> OK, I think that makes sense. Trapping any plan invalidations early,
> after ExecDoInitialPruning(), and before initialising the rest of the
> executor state does seem preferable for that reason.
>
> I have a couple of comments on the v3 patch:
>
> * In ExecInitModifyTable(), it's possible to avoid code duplication. I
> think that's worth it to make the code more maintainable if more
> per-result relation logic is added in the future.
>
> * In executor.h, the name of the new function argument doesn't match
> the .c source file.
Thanks for making those changes.
> * In ExecDoInitialPruning(), there is room for improvement: we
> actually only need to lock the first result relation if all other
> result relations of the ModifyTable node have been pruned. As it
> stands, there's no easy way to tell that, so I've just made a note of
> it in a comment. I wondered about replacing the new field
> "firstResultRels" with something like "mtResultRels", which would be a
> list of lists, to allow that, but I didn't try it.
Or a List of Bitmapset, so that bms_intersect() can be used to check
whether all result relations of a given ModifyTable have been pruned.
But whether it’s that or a List of Lists, both would add a potentially
large amount of memory to PlannedStmt, to avoid taking an extra lock.
Probably not a great tradeoff, but we can revisit if it becomes
worthwhile?
> I'm attaching v4, which addresses those comments, and includes a few
> other comment update suggestions.
I've attached v5 with my commit message but were you planning to
commit it yourself? If not, does the commit message look good to you?
--
Thanks, Amit Langote
Attachments:
v5-0001-Ensure-first-result-relation-is-included-even-if-.patch (application/octet-stream, 20.2K) download
From 78030c25436be6b55381a4e80d5c2b60a166300a Mon Sep 17 00:00:00 2001
From: Amit Langote <amitlan@postgresql.org>
Date: Mon, 17 Mar 2025 21:13:54 +0900
Subject: [PATCH v5] Ensure first result relation is included even if pruned
Commit cbc127917e introduced tracking of unpruned relids to avoid
processing pruned relations, and changed ExecInitModifyTable() to
initialize only unpruned result relations. As a result, MERGE
statements that prune all target partitions can now hit crashes or
misbehavior during execution.
Some executor code paths rely on ModifyTableState.resultRelInfo[0]
being present and initialized, even when no result relations remain
after pruning. For example, ExecMerge() and ExecMergeNotMatched() use
the first resultRelInfo to determine the appropriate action.
Similarly, ExecInitPartitionInfo() assumes that at least one result
relation exists.
To preserve these assumptions, ExecDoInitialPruning() now ensures that
the first result relation of each ModifyTable node in the plan is
locked, even if it was pruned. ExecInitModifyTable() then includes it
in the initialized result relation list. To support this, PlannedStmt
now includes a list of RT indexes identifying the first result relation
of each ModifyTable node in the plan.
This avoids crashes and undefined behavior while still allowing pruning
of other result relations to proceed as before.
Bug: #18830
Reported-by: Robins Tharakan <tharakan@gmail.com>
Diagnozed-by: Tender Wang <tndrwang@gmail.com>
Diagnozed-by: Dean Rasheed <dean.a.rasheed@gmail.com>
Co-authored-by: Dean Rasheed <dean.a.rasheed@gmail.com>
Reviewed-by: Tender Wang <tndrwang@gmail.com>
Reviewed-by: Dean Rasheed <dean.a.rasheed@gmail.com>
Discussion: https://postgr.es/m/18830-1f31ea1dc930d444%40postgresql.org
---
src/backend/commands/explain.c | 14 +++-
src/backend/executor/execMain.c | 2 +-
src/backend/executor/execPartition.c | 31 ++++++-
src/backend/executor/execUtils.c | 18 ++--
src/backend/executor/nodeModifyTable.c | 32 +++++++-
src/backend/optimizer/plan/planner.c | 1 +
src/backend/optimizer/plan/setrefs.c | 3 +
src/include/executor/execPartition.h | 4 +-
src/include/executor/executor.h | 3 +-
src/include/nodes/pathnodes.h | 3 +
src/include/nodes/plannodes.h | 7 ++
src/test/regress/expected/partition_prune.out | 82 +++++++++++++++++++
src/test/regress/sql/partition_prune.sql | 32 ++++++++
13 files changed, 214 insertions(+), 18 deletions(-)
diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
index 19ffcc2cacb..3af6cf538e4 100644
--- a/src/backend/commands/explain.c
+++ b/src/backend/commands/explain.c
@@ -4695,10 +4695,20 @@ show_modifytable_info(ModifyTableState *mtstate, List *ancestors,
break;
}
- /* Should we explicitly label target relations? */
+ /*
+ * Should we explicitly label target relations?
+ *
+ * If there's only one target relation, do not list it if it's the
+ * relation named in the query, or if it has been pruned. (Normally
+ * mtstate->resultRelInfo doesn't include pruned relations, but a single
+ * pruned target relation may be present, if all other target relations
+ * have been pruned. See ExecInitModifyTable().)
+ */
labeltargets = (mtstate->mt_nrels > 1 ||
(mtstate->mt_nrels == 1 &&
- mtstate->resultRelInfo[0].ri_RangeTableIndex != node->nominalRelation));
+ mtstate->resultRelInfo[0].ri_RangeTableIndex != node->nominalRelation &&
+ bms_is_member(mtstate->resultRelInfo[0].ri_RangeTableIndex,
+ mtstate->ps.state->es_unpruned_relids)));
if (labeltargets)
ExplainOpenGroup("Target Tables", "Target Tables", false, es);
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index 0493b7d5365..e9bd98c7738 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -1006,7 +1006,7 @@ InitPlan(QueryDesc *queryDesc, int eflags)
case ROW_MARK_SHARE:
case ROW_MARK_KEYSHARE:
case ROW_MARK_REFERENCE:
- relation = ExecGetRangeTableRelation(estate, rc->rti);
+ relation = ExecGetRangeTableRelation(estate, rc->rti, false);
break;
case ROW_MARK_COPY:
/* no physical table access is required */
diff --git a/src/backend/executor/execPartition.c b/src/backend/executor/execPartition.c
index 5cd5e2eeb80..84ccd7d457d 100644
--- a/src/backend/executor/execPartition.c
+++ b/src/backend/executor/execPartition.c
@@ -1819,6 +1819,7 @@ adjust_partition_colnos_using_map(List *colnos, AttrMap *attrMap)
void
ExecDoInitialPruning(EState *estate)
{
+ PlannedStmt *stmt = estate->es_plannedstmt;
ListCell *lc;
List *locked_relids = NIL;
@@ -1867,6 +1868,34 @@ ExecDoInitialPruning(EState *estate)
validsubplans);
}
+ /*
+ * Lock the first result relation of each ModifyTable node, even if it was
+ * pruned. This is required for ExecInitModifyTable(), which keeps its
+ * first result relation if all other result relations have been pruned,
+ * because some executor paths (e.g., in nodeModifyTable.c and
+ * execPartition.c) rely on there being at least one result relation.
+ *
+ * There's room for improvement here --- we actually only need to do this
+ * if all other result relations of the ModifyTable node were pruned, but
+ * we don't have an easy way to tell that here.
+ */
+ if (stmt->resultRelations && ExecShouldLockRelations(estate))
+ {
+ foreach(lc, stmt->firstResultRels)
+ {
+ Index firstResultRel = lfirst_int(lc);
+
+ if (!bms_is_member(firstResultRel, estate->es_unpruned_relids))
+ {
+ RangeTblEntry *rte = exec_rt_fetch(firstResultRel, estate);
+
+ Assert(rte->rtekind == RTE_RELATION && rte->rellockmode != NoLock);
+ LockRelationOid(rte->relid, rte->rellockmode);
+ locked_relids = lappend_int(locked_relids, firstResultRel);
+ }
+ }
+ }
+
/*
* Release the useless locks if the plan won't be executed. This is the
* same as what CheckCachedPlan() in plancache.c does.
@@ -2076,7 +2105,7 @@ CreatePartitionPruneState(EState *estate, PartitionPruneInfo *pruneinfo,
* because that entry will be held open and locked for the
* duration of this executor run.
*/
- partrel = ExecGetRangeTableRelation(estate, pinfo->rtindex);
+ partrel = ExecGetRangeTableRelation(estate, pinfo->rtindex, false);
/* Remember for InitExecPartitionPruneContext(). */
pprune->partrel = partrel;
diff --git a/src/backend/executor/execUtils.c b/src/backend/executor/execUtils.c
index 39d6f4d819e..55ab18fb826 100644
--- a/src/backend/executor/execUtils.c
+++ b/src/backend/executor/execUtils.c
@@ -746,7 +746,7 @@ ExecOpenScanRelation(EState *estate, Index scanrelid, int eflags)
Relation rel;
/* Open the relation. */
- rel = ExecGetRangeTableRelation(estate, scanrelid);
+ rel = ExecGetRangeTableRelation(estate, scanrelid, false);
/*
* Complain if we're attempting a scan of an unscannable relation, except
@@ -815,18 +815,22 @@ ExecInitRangeTable(EState *estate, List *rangeTable, List *permInfos,
*
* The Relations will be closed in ExecEndPlan().
*
- * Note: The caller must ensure that 'rti' refers to an unpruned relation
- * (i.e., it is a member of estate->es_unpruned_relids) before calling this
- * function. Attempting to open a pruned relation will result in an error.
+ * If isResultRel is true, the relation is being used as a result relation.
+ * Such a relation might have been pruned, which is OK for result relations,
+ * but not for scan relations; see the details in ExecInitModifyTable(). If
+ * isResultRel is false, the caller must ensure that 'rti' refers to an
+ * unpruned relation (i.e., it is a member of estate->es_unpruned_relids)
+ * before calling this function. Attempting to open a pruned relation for
+ * scanning will result in an error.
*/
Relation
-ExecGetRangeTableRelation(EState *estate, Index rti)
+ExecGetRangeTableRelation(EState *estate, Index rti, bool isResultRel)
{
Relation rel;
Assert(rti > 0 && rti <= estate->es_range_table_size);
- if (!bms_is_member(rti, estate->es_unpruned_relids))
+ if (!isResultRel && !bms_is_member(rti, estate->es_unpruned_relids))
elog(ERROR, "trying to open a pruned relation");
rel = estate->es_relations[rti - 1];
@@ -880,7 +884,7 @@ ExecInitResultRelation(EState *estate, ResultRelInfo *resultRelInfo,
{
Relation resultRelationDesc;
- resultRelationDesc = ExecGetRangeTableRelation(estate, rti);
+ resultRelationDesc = ExecGetRangeTableRelation(estate, rti, true);
InitResultRelInfo(resultRelInfo,
resultRelationDesc,
rti,
diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
index b0fe50075ad..87c820276a8 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -4471,6 +4471,7 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
ModifyTableState *mtstate;
Plan *subplan = outerPlan(node);
CmdType operation = node->operation;
+ int total_nrels = list_length(node->resultRelations);
int nrels;
List *resultRelations = NIL;
List *withCheckOptionLists = NIL;
@@ -4490,13 +4491,35 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
/*
* Only consider unpruned relations for initializing their ResultRelInfo
* struct and other fields such as withCheckOptions, etc.
+ *
+ * Note: We must avoid pruning every result relation. This is important
+ * for MERGE, since even if every result relation is pruned from the
+ * subplan, there might still be NOT MATCHED rows, for which there may be
+ * INSERT actions to perform. To allow these actions to be found, at
+ * least one result relation must be kept. Also, when inserting into a
+ * partitioned table, ExecInitPartitionInfo() needs a ResultRelInfo struct
+ * as a reference for building the ResultRelInfo of the target partition.
+ * In either case, it doesn't matter which result relation is kept, so we
+ * just keep the first one, if all others have been pruned. See also,
+ * ExecDoInitialPruning(), which ensures that this first result relation
+ * has been locked.
*/
i = 0;
foreach(l, node->resultRelations)
{
Index rti = lfirst_int(l);
+ bool keep_rel;
+
+ keep_rel = bms_is_member(rti, estate->es_unpruned_relids);
+ if (!keep_rel && i == total_nrels - 1 && resultRelations == NIL)
+ {
+ /* all result relations pruned; keep the first one */
+ keep_rel = true;
+ rti = linitial_int(node->resultRelations);
+ i = 0;
+ }
- if (bms_is_member(rti, estate->es_unpruned_relids))
+ if (keep_rel)
{
resultRelations = lappend_int(resultRelations, rti);
if (node->withCheckOptionLists)
@@ -4537,6 +4560,7 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
i++;
}
nrels = list_length(resultRelations);
+ Assert(nrels > 0);
/*
* create state structure
@@ -4735,7 +4759,7 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
*/
mtstate->mt_resultOidAttno =
ExecFindJunkAttributeInTlist(subplan->targetlist, "tableoid");
- Assert(AttributeNumberIsValid(mtstate->mt_resultOidAttno) || nrels == 1);
+ Assert(AttributeNumberIsValid(mtstate->mt_resultOidAttno) || total_nrels == 1);
mtstate->mt_lastResultOid = InvalidOid; /* force lookup at first tuple */
mtstate->mt_lastResultIndex = 0; /* must be zero if no such attr */
@@ -4832,7 +4856,7 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
if (node->onConflictAction != ONCONFLICT_NONE)
{
/* insert may only have one relation, inheritance is not expanded */
- Assert(nrels == 1);
+ Assert(total_nrels == 1);
resultRelInfo->ri_onConflictArbiterIndexes = node->arbiterIndexes;
}
@@ -4979,7 +5003,7 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
if (operation == CMD_INSERT)
{
/* insert may only have one relation, inheritance is not expanded */
- Assert(nrels == 1);
+ Assert(total_nrels == 1);
resultRelInfo = mtstate->resultRelInfo;
if (!resultRelInfo->ri_usesFdwDirectModify &&
resultRelInfo->ri_FdwRoutine != NULL &&
diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index a4d523dcb0f..141177e7413 100644
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -562,6 +562,7 @@ standard_planner(Query *parse, const char *query_string, int cursorOptions,
glob->prunableRelids);
result->permInfos = glob->finalrteperminfos;
result->resultRelations = glob->resultRelations;
+ result->firstResultRels = glob->firstResultRels;
result->appendRelations = glob->appendRelations;
result->subplans = glob->subplans;
result->rewindPlanIDs = glob->rewindPlanIDs;
diff --git a/src/backend/optimizer/plan/setrefs.c b/src/backend/optimizer/plan/setrefs.c
index 999a5a8ab5a..150e9f060ee 100644
--- a/src/backend/optimizer/plan/setrefs.c
+++ b/src/backend/optimizer/plan/setrefs.c
@@ -1248,6 +1248,9 @@ set_plan_refs(PlannerInfo *root, Plan *plan, int rtoffset)
lappend_int(root->glob->resultRelations,
splan->rootRelation);
}
+ root->glob->firstResultRels =
+ lappend_int(root->glob->firstResultRels,
+ linitial_int(splan->resultRelations));
}
break;
case T_Append:
diff --git a/src/include/executor/execPartition.h b/src/include/executor/execPartition.h
index 626613012f9..3b3f46aced0 100644
--- a/src/include/executor/execPartition.h
+++ b/src/include/executor/execPartition.h
@@ -43,8 +43,8 @@ extern void ExecCleanupTupleRouting(ModifyTableState *mtstate,
* subpart_map contains indexes into PartitionPruningData.partrelprunedata[].
*
* partrel Partitioned table Relation; obtained by
- * ExecGetRangeTableRelation(estate, rti), where
- * rti is PartitionedRelPruneInfo.rtindex.
+ * ExecGetRangeTableRelation(estate, rti, false),
+ * where rti is PartitionedRelPruneInfo.rtindex.
* nparts Length of subplan_map[] and subpart_map[].
* subplan_map Subplan index by partition index, or -1.
* subpart_map Subpart index by partition index, or -1.
diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h
index 0d2ffabda68..0db5d18ba22 100644
--- a/src/include/executor/executor.h
+++ b/src/include/executor/executor.h
@@ -680,7 +680,8 @@ exec_rt_fetch(Index rti, EState *estate)
return (RangeTblEntry *) list_nth(estate->es_range_table, rti - 1);
}
-extern Relation ExecGetRangeTableRelation(EState *estate, Index rti);
+extern Relation ExecGetRangeTableRelation(EState *estate, Index rti,
+ bool isResultRel);
extern void ExecInitResultRelation(EState *estate, ResultRelInfo *resultRelInfo,
Index rti);
diff --git a/src/include/nodes/pathnodes.h b/src/include/nodes/pathnodes.h
index fbf05322c75..c24a1fc8514 100644
--- a/src/include/nodes/pathnodes.h
+++ b/src/include/nodes/pathnodes.h
@@ -138,6 +138,9 @@ typedef struct PlannerGlobal
/* "flat" list of integer RT indexes */
List *resultRelations;
+ /* "flat" list of integer RT indexes (one per ModifyTable node) */
+ List *firstResultRels;
+
/* "flat" list of AppendRelInfos */
List *appendRelations;
diff --git a/src/include/nodes/plannodes.h b/src/include/nodes/plannodes.h
index 22841211f48..f78bffd90cf 100644
--- a/src/include/nodes/plannodes.h
+++ b/src/include/nodes/plannodes.h
@@ -102,6 +102,13 @@ typedef struct PlannedStmt
/* integer list of RT indexes, or NIL */
List *resultRelations;
+ /*
+ * rtable indexes of first target relation in each ModifyTable node in the
+ * plan for INSERT/UPDATE/DELETE/MERGE
+ */
+ /* integer list of RT indexes, or NIL */
+ List *firstResultRels;
+
/* list of AppendRelInfo nodes */
List *appendRelations;
diff --git a/src/test/regress/expected/partition_prune.out b/src/test/regress/expected/partition_prune.out
index 8097f4e9282..0bf35260b46 100644
--- a/src/test/regress/expected/partition_prune.out
+++ b/src/test/regress/expected/partition_prune.out
@@ -4662,6 +4662,88 @@ table part_abc_view;
2 | c | t
(1 row)
+-- MERGE ... INSERT when all pruned from MERGE source.
+begin;
+explain (costs off)
+merge into part_abc_view pt
+using (select stable_one() + 1 as pid) as q join part_abc_2 pt2 on (q.pid = pt2.a) on pt.a = stable_one() + 2
+when not matched then insert values (1, 'd', false) returning pt.a;
+ QUERY PLAN
+------------------------------------------------
+ Merge on part_abc
+ -> Nested Loop Left Join
+ -> Seq Scan on part_abc_2 pt2
+ Filter: ((stable_one() + 1) = a)
+ -> Materialize
+ -> Append
+ Subplans Removed: 2
+(7 rows)
+
+merge into part_abc_view pt
+using (select stable_one() + 1 as pid) as q join part_abc_2 pt2 on (q.pid = pt2.a) on pt.a = stable_one() + 2
+when not matched then insert values (1, 'd', false) returning pt.a;
+ a
+---
+ 1
+(1 row)
+
+table part_abc_view;
+ a | b | c
+---+---+---
+ 1 | d | f
+ 2 | c | t
+(2 rows)
+
+rollback;
+-- A case with multiple ModifyTable nodes.
+begin;
+create table part_abc_log (action text, a int, b text, c bool);
+explain (costs off)
+with t as (
+ merge into part_abc_view pt
+ using (select stable_one() + 1 as pid) as q join part_abc_2 pt2 on (q.pid = pt2.a) on pt.a = stable_one() + 2
+ when not matched then insert values (1, 'd', false) returning merge_action(), pt.*
+)
+insert into part_abc_log select * from t returning *;
+ QUERY PLAN
+--------------------------------------------------------
+ Insert on part_abc_log
+ CTE t
+ -> Merge on part_abc
+ -> Nested Loop Left Join
+ -> Seq Scan on part_abc_2 pt2
+ Filter: ((stable_one() + 1) = a)
+ -> Materialize
+ -> Append
+ Subplans Removed: 2
+ -> CTE Scan on t
+(10 rows)
+
+with t as (
+ merge into part_abc_view pt
+ using (select stable_one() + 1 as pid) as q join part_abc_2 pt2 on (q.pid = pt2.a) on pt.a = stable_one() + 2
+ when not matched then insert values (1, 'd', false) returning merge_action(), pt.*
+)
+insert into part_abc_log select * from t returning *;
+ action | a | b | c
+--------+---+---+---
+ INSERT | 1 | d | f
+(1 row)
+
+table part_abc_view;
+ a | b | c
+---+---+---
+ 1 | d | f
+ 2 | c | t
+(2 rows)
+
+table part_abc_log;
+ action | a | b | c
+--------+---+---+---
+ INSERT | 1 | d | f
+(1 row)
+
+rollback;
-- A case with nested MergeAppend with its own PartitionPruneInfo.
create index on part_abc (a);
alter table part_abc add d int;
diff --git a/src/test/regress/sql/partition_prune.sql b/src/test/regress/sql/partition_prune.sql
index 4a2c74b0899..f6db9479f54 100644
--- a/src/test/regress/sql/partition_prune.sql
+++ b/src/test/regress/sql/partition_prune.sql
@@ -1401,6 +1401,38 @@ using (select stable_one() + 2 as pid) as q join part_abc_1 pt1 on (q.pid = pt1.
when matched then delete returning pt.a;
table part_abc_view;
+-- MERGE ... INSERT when all pruned from MERGE source.
+begin;
+explain (costs off)
+merge into part_abc_view pt
+using (select stable_one() + 1 as pid) as q join part_abc_2 pt2 on (q.pid = pt2.a) on pt.a = stable_one() + 2
+when not matched then insert values (1, 'd', false) returning pt.a;
+merge into part_abc_view pt
+using (select stable_one() + 1 as pid) as q join part_abc_2 pt2 on (q.pid = pt2.a) on pt.a = stable_one() + 2
+when not matched then insert values (1, 'd', false) returning pt.a;
+table part_abc_view;
+rollback;
+
+-- A case with multiple ModifyTable nodes.
+begin;
+create table part_abc_log (action text, a int, b text, c bool);
+explain (costs off)
+with t as (
+ merge into part_abc_view pt
+ using (select stable_one() + 1 as pid) as q join part_abc_2 pt2 on (q.pid = pt2.a) on pt.a = stable_one() + 2
+ when not matched then insert values (1, 'd', false) returning merge_action(), pt.*
+)
+insert into part_abc_log select * from t returning *;
+with t as (
+ merge into part_abc_view pt
+ using (select stable_one() + 1 as pid) as q join part_abc_2 pt2 on (q.pid = pt2.a) on pt.a = stable_one() + 2
+ when not matched then insert values (1, 'd', false) returning merge_action(), pt.*
+)
+insert into part_abc_log select * from t returning *;
+table part_abc_view;
+table part_abc_log;
+rollback;
+
-- A case with nested MergeAppend with its own PartitionPruneInfo.
create index on part_abc (a);
alter table part_abc add d int;
--
2.43.0
* Re: BUG #18830: ExecInitMerge Segfault on MERGE
2025-03-02 21:00 BUG #18830: ExecInitMerge Segfault on MERGE PG Bug reporting form <noreply@postgresql.org>
2025-03-03 15:46 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Tender Wang <tndrwang@gmail.com>
2025-03-04 06:58 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Tender Wang <tndrwang@gmail.com>
2025-03-04 09:29 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE David Rowley <dgrowleyml@gmail.com>
2025-03-04 18:56 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Dean Rasheed <dean.a.rasheed@gmail.com>
2025-03-12 12:24 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Amit Langote <amitlangote09@gmail.com>
2025-03-12 14:59 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Dean Rasheed <dean.a.rasheed@gmail.com>
2025-03-12 16:11 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Tender Wang <tndrwang@gmail.com>
2025-03-12 19:23 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Dean Rasheed <dean.a.rasheed@gmail.com>
2025-03-13 12:44 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Amit Langote <amitlangote09@gmail.com>
2025-03-13 14:42 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Dean Rasheed <dean.a.rasheed@gmail.com>
2025-03-14 02:48 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Amit Langote <amitlangote09@gmail.com>
2025-03-16 09:57 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Dean Rasheed <dean.a.rasheed@gmail.com>
2025-03-17 12:21 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Amit Langote <amitlangote09@gmail.com>
@ 2025-03-17 23:35 ` Dean Rasheed <dean.a.rasheed@gmail.com>
2025-03-18 07:52 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Amit Langote <amitlangote09@gmail.com>
0 siblings, 1 reply; 32+ messages in thread
From: Dean Rasheed @ 2025-03-17 23:35 UTC (permalink / raw)
To: Amit Langote <amitlangote09@gmail.com>; +Cc: Tender Wang <tndrwang@gmail.com>; David Rowley <dgrowleyml@gmail.com>; pgsql-bugs@lists.postgresql.org; tharakan@gmail.com
On Mon, 17 Mar 2025 at 12:21, Amit Langote <amitlangote09@gmail.com> wrote:
>
> > I wondered about replacing the new field
> > "firstResultRels" with something like "mtResultRels", which would be a
> > list of lists, to allow that, but I didn't try it.
>
> Or a List of Bitmapset, so that bms_intersect() can be used to check
> whether all result relations of a given ModifyTable have been pruned.
> But whether it’s that or a List of Lists, both would add a potentially
> large amount of memory to PlannedStmt, to avoid taking an extra lock.
> Probably not a great tradeoff, but we can revisit if it becomes
> worthwhile?
Yeah. It would have been nice to fix this without adding a new field
to PlannedStmt at all, but I couldn't see an easy way to do that.
Actually I don't think a list of lists or Btimapsets would be that
large. There are unlikely to be more than one or two ModifyTable nodes
in most queries, and the number of bottom-level elements would be less
than in the flat resultRelations list. But still, it doesn't really
seem worth the effort.
> I've attached v5 with my commit message but were you planning to
> commit it yourself?
I'll leave it to you, since it was your commit originally :-)
> If not, does the commit message look good to you?
It could perhaps be more specific about saying that
ExecInitModifyTable() includes the first result relation if all others
have been pruned, so it isn't always included.
Regards,
Dean
^ permalink raw reply [nested|flat] 32+ messages in thread
* Re: BUG #18830: ExecInitMerge Segfault on MERGE
2025-03-02 21:00 BUG #18830: ExecInitMerge Segfault on MERGE PG Bug reporting form <noreply@postgresql.org>
2025-03-03 15:46 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Tender Wang <tndrwang@gmail.com>
2025-03-04 06:58 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Tender Wang <tndrwang@gmail.com>
2025-03-04 09:29 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE David Rowley <dgrowleyml@gmail.com>
2025-03-04 18:56 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Dean Rasheed <dean.a.rasheed@gmail.com>
2025-03-12 12:24 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Amit Langote <amitlangote09@gmail.com>
2025-03-12 14:59 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Dean Rasheed <dean.a.rasheed@gmail.com>
2025-03-12 16:11 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Tender Wang <tndrwang@gmail.com>
2025-03-12 19:23 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Dean Rasheed <dean.a.rasheed@gmail.com>
2025-03-13 12:44 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Amit Langote <amitlangote09@gmail.com>
2025-03-13 14:42 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Dean Rasheed <dean.a.rasheed@gmail.com>
2025-03-14 02:48 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Amit Langote <amitlangote09@gmail.com>
2025-03-16 09:57 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Dean Rasheed <dean.a.rasheed@gmail.com>
2025-03-17 12:21 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Amit Langote <amitlangote09@gmail.com>
2025-03-17 23:35 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Dean Rasheed <dean.a.rasheed@gmail.com>
@ 2025-03-18 07:52 ` Amit Langote <amitlangote09@gmail.com>
2025-03-19 03:16 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Amit Langote <amitlangote09@gmail.com>
0 siblings, 1 reply; 32+ messages in thread
From: Amit Langote @ 2025-03-18 07:52 UTC (permalink / raw)
To: Dean Rasheed <dean.a.rasheed@gmail.com>; +Cc: Tender Wang <tndrwang@gmail.com>; David Rowley <dgrowleyml@gmail.com>; pgsql-bugs@lists.postgresql.org; tharakan@gmail.com
On Tue, Mar 18, 2025 at 8:36 AM Dean Rasheed <dean.a.rasheed@gmail.com> wrote:
> On Mon, 17 Mar 2025 at 12:21, Amit Langote <amitlangote09@gmail.com> wrote:
> >
> > > I wondered about replacing the new field
> > > "firstResultRels" with something like "mtResultRels", which would be a
> > > list of lists, to allow that, but I didn't try it.
> >
> > Or a List of Bitmapset, so that bms_intersect() can be used to check
> > whether all result relations of a given ModifyTable have been pruned.
> > But whether it’s that or a List of Lists, both would add a potentially
> > large amount of memory to PlannedStmt, to avoid taking an extra lock.
> > Probably not a great tradeoff, but we can revisit if it becomes
> > worthwhile?
>
> Yeah. It would have been nice to fix this without adding a new field
> to PlannedStmt at all, but I couldn't see an easy way to do that.
> Actually I don't think a list of lists or Btimapsets would be that
> large. There are unlikely to be more than one or two ModifyTable nodes
> in most queries, and the number of bottom-level elements would be less
> than in the flat resultRelations list. But still, it doesn't really
> seem worth the effort.
Ok, thanks -- appreciate your thoughts. I agree it would’ve been nice
to avoid adding a new field to PlannedStmt, but I couldn’t see a
simple way around it either, given the lock timing restrictions.
> > I've attached v5 with my commit message but were you planning to
> > commit it yourself?
>
> I'll leave it to you, since it was your commit originally :-)
Yes, of course. :-)
> > If not, does the commit message look good to you?
>
> It could perhaps be more specific about saying that
> ExecInitModifyTable() includes the first result relation if all others
> have been pruned, so it isn't always included.
Ok, done in the attached.
I'll push this tomorrow.
--
Thanks, Amit Langote
Attachments:
v6-0001-Ensure-first-ModifyTable-rel-initialized-if-all-a.patch (application/octet-stream, 20.5K) download
From 03cf6cf4a6ad1e69e80222ab2def0c347c3b27f2 Mon Sep 17 00:00:00 2001
From: Amit Langote <amitlan@postgresql.org>
Date: Tue, 18 Mar 2025 16:43:36 +0900
Subject: [PATCH v6] Ensure first ModifyTable rel initialized if all are pruned
Commit cbc127917e introduced tracking of unpruned relids to avoid
processing pruned relations, and changed ExecInitModifyTable() to
initialize only unpruned result relations. As a result, MERGE
statements that prune all target partitions can now lead to crashes
or incorrect behavior during execution.
Some executor code paths rely on ModifyTableState.resultRelInfo[0]
being present and initialized, even when no result relations remain
after pruning. For example, ExecMerge() and ExecMergeNotMatched() use
the first resultRelInfo to determine the appropriate action.
Similarly, ExecInitPartitionInfo() assumes that at least one result
relation exists.
To preserve these assumptions, ExecDoInitialPruning() now ensures
that the first result relation of each ModifyTable node in the plan
is locked if it was pruned and locking is necessary.
ExecInitModifyTable() then includes it in the initialized result
relation list only if all result relations for that ModifyTable were
pruned.
To support this exception to the pruning logic, PlannedStmt now
includes a list of RT indexes identifying the first result relation
of each ModifyTable node in the plan. This allows
ExecDoInitialPruning() to check whether each such relation was
pruned and, if so, lock it if necessary.
This avoids crashes and incorrect behavior while still allowing
pruning of other result relations to proceed as before.
Bug: #18830
Reported-by: Robins Tharakan <tharakan@gmail.com>
Diagnozed-by: Tender Wang <tndrwang@gmail.com>
Diagnozed-by: Dean Rasheed <dean.a.rasheed@gmail.com>
Co-authored-by: Dean Rasheed <dean.a.rasheed@gmail.com>
Reviewed-by: Tender Wang <tndrwang@gmail.com>
Reviewed-by: Dean Rasheed <dean.a.rasheed@gmail.com>
Discussion: https://postgr.es/m/18830-1f31ea1dc930d444%40postgresql.org
---
src/backend/commands/explain.c | 14 +++-
src/backend/executor/execMain.c | 2 +-
src/backend/executor/execPartition.c | 31 ++++++-
src/backend/executor/execUtils.c | 18 ++--
src/backend/executor/nodeModifyTable.c | 32 +++++++-
src/backend/optimizer/plan/planner.c | 1 +
src/backend/optimizer/plan/setrefs.c | 3 +
src/include/executor/execPartition.h | 4 +-
src/include/executor/executor.h | 3 +-
src/include/nodes/pathnodes.h | 3 +
src/include/nodes/plannodes.h | 7 ++
src/test/regress/expected/partition_prune.out | 82 +++++++++++++++++++
src/test/regress/sql/partition_prune.sql | 32 ++++++++
13 files changed, 214 insertions(+), 18 deletions(-)
diff --git a/src/backend/commands/explain.c b/src/backend/commands/explain.c
index 19ffcc2cacb..3af6cf538e4 100644
--- a/src/backend/commands/explain.c
+++ b/src/backend/commands/explain.c
@@ -4695,10 +4695,20 @@ show_modifytable_info(ModifyTableState *mtstate, List *ancestors,
break;
}
- /* Should we explicitly label target relations? */
+ /*
+ * Should we explicitly label target relations?
+ *
+ * If there's only one target relation, do not list it if it's the
+ * relation named in the query, or if it has been pruned. (Normally
+ * mtstate->resultRelInfo doesn't include pruned relations, but a single
+ * pruned target relation may be present, if all other target relations
+ * have been pruned. See ExecInitModifyTable().)
+ */
labeltargets = (mtstate->mt_nrels > 1 ||
(mtstate->mt_nrels == 1 &&
- mtstate->resultRelInfo[0].ri_RangeTableIndex != node->nominalRelation));
+ mtstate->resultRelInfo[0].ri_RangeTableIndex != node->nominalRelation &&
+ bms_is_member(mtstate->resultRelInfo[0].ri_RangeTableIndex,
+ mtstate->ps.state->es_unpruned_relids)));
if (labeltargets)
ExplainOpenGroup("Target Tables", "Target Tables", false, es);
diff --git a/src/backend/executor/execMain.c b/src/backend/executor/execMain.c
index 0493b7d5365..e9bd98c7738 100644
--- a/src/backend/executor/execMain.c
+++ b/src/backend/executor/execMain.c
@@ -1006,7 +1006,7 @@ InitPlan(QueryDesc *queryDesc, int eflags)
case ROW_MARK_SHARE:
case ROW_MARK_KEYSHARE:
case ROW_MARK_REFERENCE:
- relation = ExecGetRangeTableRelation(estate, rc->rti);
+ relation = ExecGetRangeTableRelation(estate, rc->rti, false);
break;
case ROW_MARK_COPY:
/* no physical table access is required */
diff --git a/src/backend/executor/execPartition.c b/src/backend/executor/execPartition.c
index 5cd5e2eeb80..84ccd7d457d 100644
--- a/src/backend/executor/execPartition.c
+++ b/src/backend/executor/execPartition.c
@@ -1819,6 +1819,7 @@ adjust_partition_colnos_using_map(List *colnos, AttrMap *attrMap)
void
ExecDoInitialPruning(EState *estate)
{
+ PlannedStmt *stmt = estate->es_plannedstmt;
ListCell *lc;
List *locked_relids = NIL;
@@ -1867,6 +1868,34 @@ ExecDoInitialPruning(EState *estate)
validsubplans);
}
+ /*
+ * Lock the first result relation of each ModifyTable node, even if it was
+ * pruned. This is required for ExecInitModifyTable(), which keeps its
+ * first result relation if all other result relations have been pruned,
+ * because some executor paths (e.g., in nodeModifyTable.c and
+ * execPartition.c) rely on there being at least one result relation.
+ *
+ * There's room for improvement here --- we actually only need to do this
+ * if all other result relations of the ModifyTable node were pruned, but
+ * we don't have an easy way to tell that here.
+ */
+ if (stmt->resultRelations && ExecShouldLockRelations(estate))
+ {
+ foreach(lc, stmt->firstResultRels)
+ {
+ Index firstResultRel = lfirst_int(lc);
+
+ if (!bms_is_member(firstResultRel, estate->es_unpruned_relids))
+ {
+ RangeTblEntry *rte = exec_rt_fetch(firstResultRel, estate);
+
+ Assert(rte->rtekind == RTE_RELATION && rte->rellockmode != NoLock);
+ LockRelationOid(rte->relid, rte->rellockmode);
+ locked_relids = lappend_int(locked_relids, firstResultRel);
+ }
+ }
+ }
+
/*
* Release the useless locks if the plan won't be executed. This is the
* same as what CheckCachedPlan() in plancache.c does.
@@ -2076,7 +2105,7 @@ CreatePartitionPruneState(EState *estate, PartitionPruneInfo *pruneinfo,
* because that entry will be held open and locked for the
* duration of this executor run.
*/
- partrel = ExecGetRangeTableRelation(estate, pinfo->rtindex);
+ partrel = ExecGetRangeTableRelation(estate, pinfo->rtindex, false);
/* Remember for InitExecPartitionPruneContext(). */
pprune->partrel = partrel;
diff --git a/src/backend/executor/execUtils.c b/src/backend/executor/execUtils.c
index 39d6f4d819e..55ab18fb826 100644
--- a/src/backend/executor/execUtils.c
+++ b/src/backend/executor/execUtils.c
@@ -746,7 +746,7 @@ ExecOpenScanRelation(EState *estate, Index scanrelid, int eflags)
Relation rel;
/* Open the relation. */
- rel = ExecGetRangeTableRelation(estate, scanrelid);
+ rel = ExecGetRangeTableRelation(estate, scanrelid, false);
/*
* Complain if we're attempting a scan of an unscannable relation, except
@@ -815,18 +815,22 @@ ExecInitRangeTable(EState *estate, List *rangeTable, List *permInfos,
*
* The Relations will be closed in ExecEndPlan().
*
- * Note: The caller must ensure that 'rti' refers to an unpruned relation
- * (i.e., it is a member of estate->es_unpruned_relids) before calling this
- * function. Attempting to open a pruned relation will result in an error.
+ * If isResultRel is true, the relation is being used as a result relation.
+ * Such a relation might have been pruned, which is OK for result relations,
+ * but not for scan relations; see the details in ExecInitModifyTable(). If
+ * isResultRel is false, the caller must ensure that 'rti' refers to an
+ * unpruned relation (i.e., it is a member of estate->es_unpruned_relids)
+ * before calling this function. Attempting to open a pruned relation for
+ * scanning will result in an error.
*/
Relation
-ExecGetRangeTableRelation(EState *estate, Index rti)
+ExecGetRangeTableRelation(EState *estate, Index rti, bool isResultRel)
{
Relation rel;
Assert(rti > 0 && rti <= estate->es_range_table_size);
- if (!bms_is_member(rti, estate->es_unpruned_relids))
+ if (!isResultRel && !bms_is_member(rti, estate->es_unpruned_relids))
elog(ERROR, "trying to open a pruned relation");
rel = estate->es_relations[rti - 1];
@@ -880,7 +884,7 @@ ExecInitResultRelation(EState *estate, ResultRelInfo *resultRelInfo,
{
Relation resultRelationDesc;
- resultRelationDesc = ExecGetRangeTableRelation(estate, rti);
+ resultRelationDesc = ExecGetRangeTableRelation(estate, rti, true);
InitResultRelInfo(resultRelInfo,
resultRelationDesc,
rti,
diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
index b0fe50075ad..87c820276a8 100644
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -4471,6 +4471,7 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
ModifyTableState *mtstate;
Plan *subplan = outerPlan(node);
CmdType operation = node->operation;
+ int total_nrels = list_length(node->resultRelations);
int nrels;
List *resultRelations = NIL;
List *withCheckOptionLists = NIL;
@@ -4490,13 +4491,35 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
/*
* Only consider unpruned relations for initializing their ResultRelInfo
* struct and other fields such as withCheckOptions, etc.
+ *
+ * Note: We must avoid pruning every result relation. This is important
+ * for MERGE, since even if every result relation is pruned from the
+ * subplan, there might still be NOT MATCHED rows, for which there may be
+ * INSERT actions to perform. To allow these actions to be found, at
+ * least one result relation must be kept. Also, when inserting into a
+ * partitioned table, ExecInitPartitionInfo() needs a ResultRelInfo struct
+ * as a reference for building the ResultRelInfo of the target partition.
+ * In either case, it doesn't matter which result relation is kept, so we
+ * just keep the first one, if all others have been pruned. See also,
+ * ExecDoInitialPruning(), which ensures that this first result relation
+ * has been locked.
*/
i = 0;
foreach(l, node->resultRelations)
{
Index rti = lfirst_int(l);
+ bool keep_rel;
+
+ keep_rel = bms_is_member(rti, estate->es_unpruned_relids);
+ if (!keep_rel && i == total_nrels - 1 && resultRelations == NIL)
+ {
+ /* all result relations pruned; keep the first one */
+ keep_rel = true;
+ rti = linitial_int(node->resultRelations);
+ i = 0;
+ }
- if (bms_is_member(rti, estate->es_unpruned_relids))
+ if (keep_rel)
{
resultRelations = lappend_int(resultRelations, rti);
if (node->withCheckOptionLists)
@@ -4537,6 +4560,7 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
i++;
}
nrels = list_length(resultRelations);
+ Assert(nrels > 0);
/*
* create state structure
@@ -4735,7 +4759,7 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
*/
mtstate->mt_resultOidAttno =
ExecFindJunkAttributeInTlist(subplan->targetlist, "tableoid");
- Assert(AttributeNumberIsValid(mtstate->mt_resultOidAttno) || nrels == 1);
+ Assert(AttributeNumberIsValid(mtstate->mt_resultOidAttno) || total_nrels == 1);
mtstate->mt_lastResultOid = InvalidOid; /* force lookup at first tuple */
mtstate->mt_lastResultIndex = 0; /* must be zero if no such attr */
@@ -4832,7 +4856,7 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
if (node->onConflictAction != ONCONFLICT_NONE)
{
/* insert may only have one relation, inheritance is not expanded */
- Assert(nrels == 1);
+ Assert(total_nrels == 1);
resultRelInfo->ri_onConflictArbiterIndexes = node->arbiterIndexes;
}
@@ -4979,7 +5003,7 @@ ExecInitModifyTable(ModifyTable *node, EState *estate, int eflags)
if (operation == CMD_INSERT)
{
/* insert may only have one relation, inheritance is not expanded */
- Assert(nrels == 1);
+ Assert(total_nrels == 1);
resultRelInfo = mtstate->resultRelInfo;
if (!resultRelInfo->ri_usesFdwDirectModify &&
resultRelInfo->ri_FdwRoutine != NULL &&
diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c
index a4d523dcb0f..141177e7413 100644
--- a/src/backend/optimizer/plan/planner.c
+++ b/src/backend/optimizer/plan/planner.c
@@ -562,6 +562,7 @@ standard_planner(Query *parse, const char *query_string, int cursorOptions,
glob->prunableRelids);
result->permInfos = glob->finalrteperminfos;
result->resultRelations = glob->resultRelations;
+ result->firstResultRels = glob->firstResultRels;
result->appendRelations = glob->appendRelations;
result->subplans = glob->subplans;
result->rewindPlanIDs = glob->rewindPlanIDs;
diff --git a/src/backend/optimizer/plan/setrefs.c b/src/backend/optimizer/plan/setrefs.c
index 999a5a8ab5a..150e9f060ee 100644
--- a/src/backend/optimizer/plan/setrefs.c
+++ b/src/backend/optimizer/plan/setrefs.c
@@ -1248,6 +1248,9 @@ set_plan_refs(PlannerInfo *root, Plan *plan, int rtoffset)
lappend_int(root->glob->resultRelations,
splan->rootRelation);
}
+ root->glob->firstResultRels =
+ lappend_int(root->glob->firstResultRels,
+ linitial_int(splan->resultRelations));
}
break;
case T_Append:
diff --git a/src/include/executor/execPartition.h b/src/include/executor/execPartition.h
index 626613012f9..3b3f46aced0 100644
--- a/src/include/executor/execPartition.h
+++ b/src/include/executor/execPartition.h
@@ -43,8 +43,8 @@ extern void ExecCleanupTupleRouting(ModifyTableState *mtstate,
* subpart_map contains indexes into PartitionPruningData.partrelprunedata[].
*
* partrel Partitioned table Relation; obtained by
- * ExecGetRangeTableRelation(estate, rti), where
- * rti is PartitionedRelPruneInfo.rtindex.
+ * ExecGetRangeTableRelation(estate, rti, false),
+ * where rti is PartitionedRelPruneInfo.rtindex.
* nparts Length of subplan_map[] and subpart_map[].
* subplan_map Subplan index by partition index, or -1.
* subpart_map Subpart index by partition index, or -1.
diff --git a/src/include/executor/executor.h b/src/include/executor/executor.h
index 0d2ffabda68..0db5d18ba22 100644
--- a/src/include/executor/executor.h
+++ b/src/include/executor/executor.h
@@ -680,7 +680,8 @@ exec_rt_fetch(Index rti, EState *estate)
return (RangeTblEntry *) list_nth(estate->es_range_table, rti - 1);
}
-extern Relation ExecGetRangeTableRelation(EState *estate, Index rti);
+extern Relation ExecGetRangeTableRelation(EState *estate, Index rti,
+ bool isResultRel);
extern void ExecInitResultRelation(EState *estate, ResultRelInfo *resultRelInfo,
Index rti);
diff --git a/src/include/nodes/pathnodes.h b/src/include/nodes/pathnodes.h
index fbf05322c75..c24a1fc8514 100644
--- a/src/include/nodes/pathnodes.h
+++ b/src/include/nodes/pathnodes.h
@@ -138,6 +138,9 @@ typedef struct PlannerGlobal
/* "flat" list of integer RT indexes */
List *resultRelations;
+ /* "flat" list of integer RT indexes (one per ModifyTable node) */
+ List *firstResultRels;
+
/* "flat" list of AppendRelInfos */
List *appendRelations;
diff --git a/src/include/nodes/plannodes.h b/src/include/nodes/plannodes.h
index 22841211f48..f78bffd90cf 100644
--- a/src/include/nodes/plannodes.h
+++ b/src/include/nodes/plannodes.h
@@ -102,6 +102,13 @@ typedef struct PlannedStmt
/* integer list of RT indexes, or NIL */
List *resultRelations;
+ /*
+ * rtable indexes of first target relation in each ModifyTable node in the
+ * plan for INSERT/UPDATE/DELETE/MERGE
+ */
+ /* integer list of RT indexes, or NIL */
+ List *firstResultRels;
+
/* list of AppendRelInfo nodes */
List *appendRelations;
diff --git a/src/test/regress/expected/partition_prune.out b/src/test/regress/expected/partition_prune.out
index 8097f4e9282..0bf35260b46 100644
--- a/src/test/regress/expected/partition_prune.out
+++ b/src/test/regress/expected/partition_prune.out
@@ -4662,6 +4662,88 @@ table part_abc_view;
2 | c | t
(1 row)
+-- MERGE ... INSERT when all pruned from MERGE source.
+begin;
+explain (costs off)
+merge into part_abc_view pt
+using (select stable_one() + 1 as pid) as q join part_abc_2 pt2 on (q.pid = pt2.a) on pt.a = stable_one() + 2
+when not matched then insert values (1, 'd', false) returning pt.a;
+ QUERY PLAN
+------------------------------------------------
+ Merge on part_abc
+ -> Nested Loop Left Join
+ -> Seq Scan on part_abc_2 pt2
+ Filter: ((stable_one() + 1) = a)
+ -> Materialize
+ -> Append
+ Subplans Removed: 2
+(7 rows)
+
+merge into part_abc_view pt
+using (select stable_one() + 1 as pid) as q join part_abc_2 pt2 on (q.pid = pt2.a) on pt.a = stable_one() + 2
+when not matched then insert values (1, 'd', false) returning pt.a;
+ a
+---
+ 1
+(1 row)
+
+table part_abc_view;
+ a | b | c
+---+---+---
+ 1 | d | f
+ 2 | c | t
+(2 rows)
+
+rollback;
+-- A case with multiple ModifyTable nodes.
+begin;
+create table part_abc_log (action text, a int, b text, c bool);
+explain (costs off)
+with t as (
+ merge into part_abc_view pt
+ using (select stable_one() + 1 as pid) as q join part_abc_2 pt2 on (q.pid = pt2.a) on pt.a = stable_one() + 2
+ when not matched then insert values (1, 'd', false) returning merge_action(), pt.*
+)
+insert into part_abc_log select * from t returning *;
+ QUERY PLAN
+--------------------------------------------------------
+ Insert on part_abc_log
+ CTE t
+ -> Merge on part_abc
+ -> Nested Loop Left Join
+ -> Seq Scan on part_abc_2 pt2
+ Filter: ((stable_one() + 1) = a)
+ -> Materialize
+ -> Append
+ Subplans Removed: 2
+ -> CTE Scan on t
+(10 rows)
+
+with t as (
+ merge into part_abc_view pt
+ using (select stable_one() + 1 as pid) as q join part_abc_2 pt2 on (q.pid = pt2.a) on pt.a = stable_one() + 2
+ when not matched then insert values (1, 'd', false) returning merge_action(), pt.*
+)
+insert into part_abc_log select * from t returning *;
+ action | a | b | c
+--------+---+---+---
+ INSERT | 1 | d | f
+(1 row)
+
+table part_abc_view;
+ a | b | c
+---+---+---
+ 1 | d | f
+ 2 | c | t
+(2 rows)
+
+table part_abc_log;
+ action | a | b | c
+--------+---+---+---
+ INSERT | 1 | d | f
+(1 row)
+
+rollback;
-- A case with nested MergeAppend with its own PartitionPruneInfo.
create index on part_abc (a);
alter table part_abc add d int;
diff --git a/src/test/regress/sql/partition_prune.sql b/src/test/regress/sql/partition_prune.sql
index 4a2c74b0899..f6db9479f54 100644
--- a/src/test/regress/sql/partition_prune.sql
+++ b/src/test/regress/sql/partition_prune.sql
@@ -1401,6 +1401,38 @@ using (select stable_one() + 2 as pid) as q join part_abc_1 pt1 on (q.pid = pt1.
when matched then delete returning pt.a;
table part_abc_view;
+-- MERGE ... INSERT when all pruned from MERGE source.
+begin;
+explain (costs off)
+merge into part_abc_view pt
+using (select stable_one() + 1 as pid) as q join part_abc_2 pt2 on (q.pid = pt2.a) on pt.a = stable_one() + 2
+when not matched then insert values (1, 'd', false) returning pt.a;
+merge into part_abc_view pt
+using (select stable_one() + 1 as pid) as q join part_abc_2 pt2 on (q.pid = pt2.a) on pt.a = stable_one() + 2
+when not matched then insert values (1, 'd', false) returning pt.a;
+table part_abc_view;
+rollback;
+
+-- A case with multiple ModifyTable nodes.
+begin;
+create table part_abc_log (action text, a int, b text, c bool);
+explain (costs off)
+with t as (
+ merge into part_abc_view pt
+ using (select stable_one() + 1 as pid) as q join part_abc_2 pt2 on (q.pid = pt2.a) on pt.a = stable_one() + 2
+ when not matched then insert values (1, 'd', false) returning merge_action(), pt.*
+)
+insert into part_abc_log select * from t returning *;
+with t as (
+ merge into part_abc_view pt
+ using (select stable_one() + 1 as pid) as q join part_abc_2 pt2 on (q.pid = pt2.a) on pt.a = stable_one() + 2
+ when not matched then insert values (1, 'd', false) returning merge_action(), pt.*
+)
+insert into part_abc_log select * from t returning *;
+table part_abc_view;
+table part_abc_log;
+rollback;
+
-- A case with nested MergeAppend with its own PartitionPruneInfo.
create index on part_abc (a);
alter table part_abc add d int;
--
2.43.0
* Re: BUG #18830: ExecInitMerge Segfault on MERGE
2025-03-02 21:00 BUG #18830: ExecInitMerge Segfault on MERGE PG Bug reporting form <noreply@postgresql.org>
2025-03-03 15:46 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Tender Wang <tndrwang@gmail.com>
2025-03-04 06:58 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Tender Wang <tndrwang@gmail.com>
2025-03-04 09:29 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE David Rowley <dgrowleyml@gmail.com>
2025-03-04 18:56 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Dean Rasheed <dean.a.rasheed@gmail.com>
2025-03-12 12:24 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Amit Langote <amitlangote09@gmail.com>
2025-03-12 14:59 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Dean Rasheed <dean.a.rasheed@gmail.com>
2025-03-12 16:11 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Tender Wang <tndrwang@gmail.com>
2025-03-12 19:23 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Dean Rasheed <dean.a.rasheed@gmail.com>
2025-03-13 12:44 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Amit Langote <amitlangote09@gmail.com>
2025-03-13 14:42 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Dean Rasheed <dean.a.rasheed@gmail.com>
2025-03-14 02:48 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Amit Langote <amitlangote09@gmail.com>
2025-03-16 09:57 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Dean Rasheed <dean.a.rasheed@gmail.com>
2025-03-17 12:21 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Amit Langote <amitlangote09@gmail.com>
2025-03-17 23:35 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Dean Rasheed <dean.a.rasheed@gmail.com>
2025-03-18 07:52 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Amit Langote <amitlangote09@gmail.com>
@ 2025-03-19 03:16 ` Amit Langote <amitlangote09@gmail.com>
0 siblings, 0 replies; 32+ messages in thread
From: Amit Langote @ 2025-03-19 03:16 UTC (permalink / raw)
To: Dean Rasheed <dean.a.rasheed@gmail.com>; +Cc: Tender Wang <tndrwang@gmail.com>; David Rowley <dgrowleyml@gmail.com>; pgsql-bugs@lists.postgresql.org; tharakan@gmail.com
On Tue, Mar 18, 2025 at 4:52 PM Amit Langote <amitlangote09@gmail.com> wrote:
> On Tue, Mar 18, 2025 at 8:36 AM Dean Rasheed <dean.a.rasheed@gmail.com> wrote:
> > On Mon, 17 Mar 2025 at 12:21, Amit Langote <amitlangote09@gmail.com> wrote:
> > > If not, does the commit message look good to you?
> >
> > It could perhaps be more specific about saying that
> > ExecInitModifyTable() includes the first result relation if all others
> > have been pruned, so it isn't always included.
>
> Ok, done in the attached.
>
> I'll push this tomorrow.
Pushed after a few more tweaks to the commit message for better
logical flow, remove a redundant sentence.
Thanks a lot all for your help on this.
--
Thanks, Amit Langote
^ permalink raw reply [nested|flat] 32+ messages in thread
* Re: BUG #18830: ExecInitMerge Segfault on MERGE
2025-03-02 21:00 BUG #18830: ExecInitMerge Segfault on MERGE PG Bug reporting form <noreply@postgresql.org>
2025-03-03 15:46 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Tender Wang <tndrwang@gmail.com>
@ 2025-03-04 11:51 ` Amit Langote <amitlangote09@gmail.com>
2025-03-04 12:06 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Tender Wang <tndrwang@gmail.com>
1 sibling, 1 reply; 32+ messages in thread
From: Amit Langote @ 2025-03-04 11:51 UTC (permalink / raw)
To: Tender Wang <tndrwang@gmail.com>; +Cc: tharakan@gmail.com; pgsql-bugs@lists.postgresql.org
On Mon, Mar 3, 2025 at 9:16 PM Tender Wang <tndrwang@gmail.com> wrote:
> nrels = list_length(resultRelations);
> ...
> mtstate->resultRelInfo = (ResultRelInfo *)
> palloc(nrels * sizeof(ResultRelInfo));
>
> The memory of mtstate->resultRelInfo point to is undefined. When we access its memory in ExecInitMerge(),
This needs to be fixed saparately.
> relationDesc = RelationGetDescr(resultRelInfo->ri_RelationDesc);
>
> crash happened.
Do you have a case where this access to undefined
ModifyTableState.resultRelInfo occurs? I would have thought that it
should not happen.
--
Thanks, Amit Langote
^ permalink raw reply [nested|flat] 32+ messages in thread
* Re: BUG #18830: ExecInitMerge Segfault on MERGE
2025-03-02 21:00 BUG #18830: ExecInitMerge Segfault on MERGE PG Bug reporting form <noreply@postgresql.org>
2025-03-03 15:46 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Tender Wang <tndrwang@gmail.com>
2025-03-04 11:51 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Amit Langote <amitlangote09@gmail.com>
@ 2025-03-04 12:06 ` Tender Wang <tndrwang@gmail.com>
2025-03-04 12:30 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Amit Langote <amitlangote09@gmail.com>
0 siblings, 1 reply; 32+ messages in thread
From: Tender Wang @ 2025-03-04 12:06 UTC (permalink / raw)
To: Amit Langote <amitlangote09@gmail.com>; +Cc: tharakan@gmail.com; pgsql-bugs@lists.postgresql.org
Amit Langote <amitlangote09@gmail.com> 于2025年3月4日周二 19:51写道：
> On Mon, Mar 3, 2025 at 9:16 PM Tender Wang <tndrwang@gmail.com> wrote:
> > nrels = list_length(resultRelations);
> > ...
> > mtstate->resultRelInfo = (ResultRelInfo *)
> > palloc(nrels * sizeof(ResultRelInfo));
> >
> > The memory of mtstate->resultRelInfo point to is undefined. When we
> access its memory in ExecInitMerge(),
>
> This needs to be fixed saparately.
>
> > relationDesc = RelationGetDescr(resultRelInfo->ri_RelationDesc);
> >
> > crash happened.
>
> Do you have a case where this access to undefined
> ModifyTableState.resultRelInfo occurs? I would have thought that it
> should not happen.
>
"undefined" may not be accurate, "invalid" seems more correct.
I still use the case:
merge into part_abc_view pt
using (select stable_one() + 2 as pid) as q join part_abc_1 pt1 on (true)
on pt.a = stable_one() +2
when not matched then insert values(1, 'd', false);
(gdb) p *resultRelInfo
$5 = {type = 2139062142, ri_RangeTableIndex = 2139062143, ri_RelationDesc =
0x180, ri_NumIndices = 195, ri_IndexRelationDescs = 0x500000182,
ri_IndexRelationInfo = 0x7f8773b2aa38, ri_RowIdAttNo = 0,
ri_extraUpdatedCols = 0x0, ri_projectNew = 0x0,
ri_newTupleSlot = 0x0, ri_oldTupleSlot = 0x0, ri_projectNewInfoValid =
false, ri_needLockTagTuple = false, ri_TrigDesc = 0x0, ri_TrigFunctions =
0x0, ri_TrigWhenExprs = 0x0, ri_TrigInstrument = 0x0, ri_ReturningSlot =
0x0, ri_TrigOldSlot = 0x0,
ri_TrigNewSlot = 0x0, ri_AllNullSlot = 0x0, ri_FdwRoutine = 0x0,
ri_FdwState = 0x0, ri_usesFdwDirectModify = false, ri_NumSlots = 0,
ri_NumSlotsInitialized = 0, ri_BatchSize = 0, ri_Slots = 0x0, ri_PlanSlots
= 0x0, ri_WithCheckOptions = 0x0,
ri_WithCheckOptionExprs = 0x0, ri_ConstraintExprs = 0x0,
ri_GeneratedExprsI = 0x0, ri_GeneratedExprsU = 0x0, ri_NumGeneratedNeededI
= 0, ri_NumGeneratedNeededU = 0, ri_returningList = 0x0,
ri_projectReturning = 0x0,
ri_onConflictArbiterIndexes = 0x0, ri_onConflict = 0x0, ri_MergeActions =
{0x0, 0x0, 0x0}, ri_MergeJoinCondition = 0x0, ri_PartitionCheckExpr = 0x0,
ri_ChildToRootMap = 0x0, ri_ChildToRootMapValid = false, ri_RootToChildMap
= 0x0,
ri_RootToChildMapValid = false, ri_RootResultRelInfo = 0x0,
ri_PartitionTupleSlot = 0x0, ri_CopyMultiInsertBuffer = 0x0,
ri_ancestorResultRels = 0x0}
ri_RelationDesc = 0x180, this address is not invalid.
--
Thanks,
Tender Wang
^ permalink raw reply [nested|flat] 32+ messages in thread
* Re: BUG #18830: ExecInitMerge Segfault on MERGE
2025-03-02 21:00 BUG #18830: ExecInitMerge Segfault on MERGE PG Bug reporting form <noreply@postgresql.org>
2025-03-03 15:46 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Tender Wang <tndrwang@gmail.com>
2025-03-04 11:51 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Amit Langote <amitlangote09@gmail.com>
2025-03-04 12:06 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Tender Wang <tndrwang@gmail.com>
@ 2025-03-04 12:30 ` Amit Langote <amitlangote09@gmail.com>
2025-03-04 12:46 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Tender Wang <tndrwang@gmail.com>
0 siblings, 1 reply; 32+ messages in thread
From: Amit Langote @ 2025-03-04 12:30 UTC (permalink / raw)
To: Tender Wang <tndrwang@gmail.com>; +Cc: tharakan@gmail.com; pgsql-bugs@lists.postgresql.org
On Tue, Mar 4, 2025 at 5:36 PM Tender Wang <tndrwang@gmail.com> wrote:
> Amit Langote <amitlangote09@gmail.com> 于2025年3月4日周二 19:51写道：
>> On Mon, Mar 3, 2025 at 9:16 PM Tender Wang <tndrwang@gmail.com> wrote:
>> > nrels = list_length(resultRelations);
>> > ...
>> > mtstate->resultRelInfo = (ResultRelInfo *)
>> > palloc(nrels * sizeof(ResultRelInfo));
>> >
>> > The memory of mtstate->resultRelInfo point to is undefined. When we access its memory in ExecInitMerge(),
>>
>> This needs to be fixed saparately.
>>
>> > relationDesc = RelationGetDescr(resultRelInfo->ri_RelationDesc);
>> >
>> > crash happened.
>>
>> Do you have a case where this access to undefined
>> ModifyTableState.resultRelInfo occurs? I would have thought that it
>> should not happen.
>
>
> "undefined" may not be accurate, "invalid" seems more correct.
> I still use the case:
> merge into part_abc_view pt
> using (select stable_one() + 2 as pid) as q join part_abc_1 pt1 on (true)
> on pt.a = stable_one() +2
> when not matched then insert values(1, 'd', false);
>
> (gdb) p *resultRelInfo
> $5 = {type = 2139062142, ri_RangeTableIndex = 2139062143, ri_RelationDesc = 0x180, ri_NumIndices = 195, ri_IndexRelationDescs = 0x500000182, ri_IndexRelationInfo = 0x7f8773b2aa38, ri_RowIdAttNo = 0, ri_extraUpdatedCols = 0x0, ri_projectNew = 0x0,
> ri_newTupleSlot = 0x0, ri_oldTupleSlot = 0x0, ri_projectNewInfoValid = false, ri_needLockTagTuple = false, ri_TrigDesc = 0x0, ri_TrigFunctions = 0x0, ri_TrigWhenExprs = 0x0, ri_TrigInstrument = 0x0, ri_ReturningSlot = 0x0, ri_TrigOldSlot = 0x0,
> ri_TrigNewSlot = 0x0, ri_AllNullSlot = 0x0, ri_FdwRoutine = 0x0, ri_FdwState = 0x0, ri_usesFdwDirectModify = false, ri_NumSlots = 0, ri_NumSlotsInitialized = 0, ri_BatchSize = 0, ri_Slots = 0x0, ri_PlanSlots = 0x0, ri_WithCheckOptions = 0x0,
> ri_WithCheckOptionExprs = 0x0, ri_ConstraintExprs = 0x0, ri_GeneratedExprsI = 0x0, ri_GeneratedExprsU = 0x0, ri_NumGeneratedNeededI = 0, ri_NumGeneratedNeededU = 0, ri_returningList = 0x0, ri_projectReturning = 0x0,
> ri_onConflictArbiterIndexes = 0x0, ri_onConflict = 0x0, ri_MergeActions = {0x0, 0x0, 0x0}, ri_MergeJoinCondition = 0x0, ri_PartitionCheckExpr = 0x0, ri_ChildToRootMap = 0x0, ri_ChildToRootMapValid = false, ri_RootToChildMap = 0x0,
> ri_RootToChildMapValid = false, ri_RootResultRelInfo = 0x0, ri_PartitionTupleSlot = 0x0, ri_CopyMultiInsertBuffer = 0x0, ri_ancestorResultRels = 0x0}
>
> ri_RelationDesc = 0x180, this address is not invalid.
Does the query crash or are you just able to see invalid memory
address using gdb?
--
Thanks, Amit Langote
^ permalink raw reply [nested|flat] 32+ messages in thread
* Re: BUG #18830: ExecInitMerge Segfault on MERGE
2025-03-02 21:00 BUG #18830: ExecInitMerge Segfault on MERGE PG Bug reporting form <noreply@postgresql.org>
2025-03-03 15:46 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Tender Wang <tndrwang@gmail.com>
2025-03-04 11:51 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Amit Langote <amitlangote09@gmail.com>
2025-03-04 12:06 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Tender Wang <tndrwang@gmail.com>
2025-03-04 12:30 ` Re: BUG #18830: ExecInitMerge Segfault on MERGE Amit Langote <amitlangote09@gmail.com>
@ 2025-03-04 12:46 ` Tender Wang <tndrwang@gmail.com>
0 siblings, 0 replies; 32+ messages in thread
From: Tender Wang @ 2025-03-04 12:46 UTC (permalink / raw)
To: Amit Langote <amitlangote09@gmail.com>; +Cc: tharakan@gmail.com; pgsql-bugs@lists.postgresql.org
Amit Langote <amitlangote09@gmail.com> 于2025年3月4日周二 20:30写道：
> On Tue, Mar 4, 2025 at 5:36 PM Tender Wang <tndrwang@gmail.com> wrote:
> > Amit Langote <amitlangote09@gmail.com> 于2025年3月4日周二 19:51写道：
> >> On Mon, Mar 3, 2025 at 9:16 PM Tender Wang <tndrwang@gmail.com> wrote:
> >> > nrels = list_length(resultRelations);
> >> > ...
> >> > mtstate->resultRelInfo = (ResultRelInfo *)
> >> > palloc(nrels * sizeof(ResultRelInfo));
> >> >
> >> > The memory of mtstate->resultRelInfo point to is undefined. When we
> access its memory in ExecInitMerge(),
> >>
> >> This needs to be fixed saparately.
> >>
> >> > relationDesc = RelationGetDescr(resultRelInfo->ri_RelationDesc);
> >> >
> >> > crash happened.
> >>
> >> Do you have a case where this access to undefined
> >> ModifyTableState.resultRelInfo occurs? I would have thought that it
> >> should not happen.
> >
> >
> > "undefined" may not be accurate, "invalid" seems more correct.
> > I still use the case:
> > merge into part_abc_view pt
> > using (select stable_one() + 2 as pid) as q join part_abc_1 pt1 on (true)
> > on pt.a = stable_one() +2
> > when not matched then insert values(1, 'd', false);
> >
> > (gdb) p *resultRelInfo
> > $5 = {type = 2139062142, ri_RangeTableIndex = 2139062143,
> ri_RelationDesc = 0x180, ri_NumIndices = 195, ri_IndexRelationDescs =
> 0x500000182, ri_IndexRelationInfo = 0x7f8773b2aa38, ri_RowIdAttNo = 0,
> ri_extraUpdatedCols = 0x0, ri_projectNew = 0x0,
> > ri_newTupleSlot = 0x0, ri_oldTupleSlot = 0x0, ri_projectNewInfoValid =
> false, ri_needLockTagTuple = false, ri_TrigDesc = 0x0, ri_TrigFunctions =
> 0x0, ri_TrigWhenExprs = 0x0, ri_TrigInstrument = 0x0, ri_ReturningSlot =
> 0x0, ri_TrigOldSlot = 0x0,
> > ri_TrigNewSlot = 0x0, ri_AllNullSlot = 0x0, ri_FdwRoutine = 0x0,
> ri_FdwState = 0x0, ri_usesFdwDirectModify = false, ri_NumSlots = 0,
> ri_NumSlotsInitialized = 0, ri_BatchSize = 0, ri_Slots = 0x0, ri_PlanSlots
> = 0x0, ri_WithCheckOptions = 0x0,
> > ri_WithCheckOptionExprs = 0x0, ri_ConstraintExprs = 0x0,
> ri_GeneratedExprsI = 0x0, ri_GeneratedExprsU = 0x0, ri_NumGeneratedNeededI
> = 0, ri_NumGeneratedNeededU = 0, ri_returningList = 0x0,
> ri_projectReturning = 0x0,
> > ri_onConflictArbiterIndexes = 0x0, ri_onConflict = 0x0,
> ri_MergeActions = {0x0, 0x0, 0x0}, ri_MergeJoinCondition = 0x0,
> ri_PartitionCheckExpr = 0x0, ri_ChildToRootMap = 0x0,
> ri_ChildToRootMapValid = false, ri_RootToChildMap = 0x0,
> > ri_RootToChildMapValid = false, ri_RootResultRelInfo = 0x0,
> ri_PartitionTupleSlot = 0x0, ri_CopyMultiInsertBuffer = 0x0,
> ri_ancestorResultRels = 0x0}
> >
> > ri_RelationDesc = 0x180, this address is not invalid.
>
> Does the query crash or are you just able to see invalid memory
> address using gdb?
>
>
git reset --hard cbc127917e
The query will crash on relationDesc =
RelationGetDescr(resultRelInfo->ri_RelationDesc);
#0 ExecInitMerge (mtstate=0x55b08fc19a00, estate=0x55b08fc18c20) at
nodeModifyTable.c:3663
3663 relationDesc =
RelationGetDescr(resultRelInfo->ri_RelationDesc);
(gdb) bt
#0 ExecInitMerge (mtstate=0x55b08fc19a00, estate=0x55b08fc18c20) at
nodeModifyTable.c:3663
#1 0x000055b07f1046af in ExecInitModifyTable (node=0x55b08fc3d338,
estate=0x55b08fc18c20, eflags=0) at nodeModifyTable.c:4889
#2 0x000055b07f0bdd77 in ExecInitNode (node=0x55b08fc3d338,
estate=0x55b08fc18c20, eflags=0) at execProcnode.c:177
#3 0x000055b07f0b26a9 in InitPlan (queryDesc=0x55b08fbe4990, eflags=0) at
execMain.c:985
#4 0x000055b07f0b1435 in standard_ExecutorStart (queryDesc=0x55b08fbe4990,
eflags=0) at execMain.c:259
#5 0x000055b07f0b1143 in ExecutorStart (queryDesc=0x55b08fbe4990,
eflags=0) at execMain.c:135
#6 0x000055b07f395fab in ProcessQuery (plan=0x55b08fc3f0f8,
sourceText=0x55b08fb19420 "merge into part_abc_view pt\nusing (select
stable_one() + 2 as pid) as q join part_abc_1 pt1 on (true)\non pt.a =
stable_one() +2\nwhen not matched then insert values(1, 'd', false);",
params=0x0, queryEnv=0x0,
dest=0x55b08fc3f278, qc=0x7ffc72bad230) at pquery.c:155
#7 0x000055b07f397af1 in PortalRunMulti (portal=0x55b08fb99c40,
isTopLevel=true, setHoldSnapshot=false, dest=0x55b08fc3f278,
altdest=0x55b08fc3f278, qc=0x7ffc72bad230) at pquery.c:1271
#8 0x000055b07f396ffd in PortalRun (portal=0x55b08fb99c40,
count=9223372036854775807, isTopLevel=true, dest=0x55b08fc3f278,
altdest=0x55b08fc3f278, qc=0x7ffc72bad230) at pquery.c:787
#9 0x000055b07f38fafd in exec_simple_query (
query_string=0x55b08fb19420 "merge into part_abc_view pt\nusing (select
stable_one() + 2 as pid) as q join part_abc_1 pt1 on (true)\non pt.a =
stable_one() +2\nwhen not matched then insert values(1, 'd', false);") at
postgres.c:1271
#10 0x000055b07f394e65 in PostgresMain (dbname=0x55b08fb533e8 "postgres",
username=0x55b08fb533c8 "ecs-user") at postgres.c:4691
#11 0x000055b07f38b71a in BackendMain (startup_data=0x7ffc72bad4b0 "",
startup_data_len=4) at backend_startup.c:107
#12 0x000055b07f298873 in postmaster_child_launch (child_type=B_BACKEND,
child_slot=1, startup_data=0x7ffc72bad4b0 "", startup_data_len=4,
client_sock=0x7ffc72bad510) at launch_backend.c:274
#13 0x000055b07f29f1ce in BackendStartup (client_sock=0x7ffc72bad510) at
postmaster.c:3519
#14 0x000055b07f29c7ec in ServerLoop () at postmaster.c:1688
#15 0x000055b07f29c0e4 in PostmasterMain (argc=3, argv=0x55b08fb13a30) at
postmaster.c:1386
#16 0x000055b07f13fbd1 in main (argc=3, argv=0x55b08fb13a30) at main.c:230
But after you commit the 75dfde1, crash happened on ExecMergeNotMatched(),
econtext->ecxt_scantuple = NULL;
because econtext is null.
After 75dfde1, mergeActionLists is NIL, it directly returns in if
(mergeActionLists == NIL).
I think the crash and the inconsistent result are all caused by cbc127917e.
--
Thanks,
Tender Wang
^ permalink raw reply [nested|flat] 32+ messages in thread
end of thread, other threads:[~2025-03-19 03:16 UTC | newest]
Thread overview: 32+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2025-03-02 21:00 BUG #18830: ExecInitMerge Segfault on MERGE PG Bug reporting form <noreply@postgresql.org>
2025-03-03 15:46 ` Tender Wang <tndrwang@gmail.com>
2025-03-04 06:58 ` Tender Wang <tndrwang@gmail.com>
2025-03-04 09:29 ` David Rowley <dgrowleyml@gmail.com>
2025-03-04 09:41 ` Amit Langote <amitlangote09@gmail.com>
2025-03-04 09:45 ` Tender Wang <tndrwang@gmail.com>
2025-03-04 10:09 ` Amit Langote <amitlangote09@gmail.com>
2025-03-04 10:19 ` Tender Wang <tndrwang@gmail.com>
2025-03-04 10:27 ` Tender Wang <tndrwang@gmail.com>
2025-03-04 11:06 ` Tender Wang <tndrwang@gmail.com>
2025-03-04 11:32 ` Amit Langote <amitlangote09@gmail.com>
2025-03-04 18:56 ` Dean Rasheed <dean.a.rasheed@gmail.com>
2025-03-05 01:26 ` Tender Wang <tndrwang@gmail.com>
2025-03-12 12:24 ` Amit Langote <amitlangote09@gmail.com>
2025-03-12 13:24 ` Tender Wang <tndrwang@gmail.com>
2025-03-12 14:59 ` Dean Rasheed <dean.a.rasheed@gmail.com>
2025-03-12 16:11 ` Tender Wang <tndrwang@gmail.com>
2025-03-12 19:23 ` Dean Rasheed <dean.a.rasheed@gmail.com>
2025-03-13 07:29 ` Tender Wang <tndrwang@gmail.com>
2025-03-13 12:44 ` Amit Langote <amitlangote09@gmail.com>
2025-03-13 12:44 ` Amit Langote <amitlangote09@gmail.com>
2025-03-13 14:42 ` Dean Rasheed <dean.a.rasheed@gmail.com>
2025-03-14 02:48 ` Amit Langote <amitlangote09@gmail.com>
2025-03-16 09:57 ` Dean Rasheed <dean.a.rasheed@gmail.com>
2025-03-17 12:21 ` Amit Langote <amitlangote09@gmail.com>
2025-03-17 23:35 ` Dean Rasheed <dean.a.rasheed@gmail.com>
2025-03-18 07:52 ` Amit Langote <amitlangote09@gmail.com>
2025-03-19 03:16 ` Amit Langote <amitlangote09@gmail.com>
2025-03-04 11:51 ` Amit Langote <amitlangote09@gmail.com>
2025-03-04 12:06 ` Tender Wang <tndrwang@gmail.com>
2025-03-04 12:30 ` Amit Langote <amitlangote09@gmail.com>
2025-03-04 12:46 ` Tender Wang <tndrwang@gmail.com>
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox