pg.ddx.io pgsql-bugs@postgresql.org mailing list archive
help / color / mirror / Atom feedBUG #19111: Using EXPLAIN ANALYZE with MERGE causes failed assert
4+ messages / 3 participants
[nested] [flat]
* BUG #19111: Using EXPLAIN ANALYZE with MERGE causes failed assert
@ 2025-11-13 10:04 PG Bug reporting form <noreply@postgresql.org>
0 siblings, 1 reply; 4+ messages in thread
From: PG Bug reporting form @ 2025-11-13 10:04 UTC (permalink / raw)
To: pgsql-bugs@lists.postgresql.org; +Cc: o.tselebrovskiy@postgrespro.ru
The following bug has been logged on the website:
Bug reference: 19111
Logged by: Oleg Tselebrovskiy
Email address: o.tselebrovskiy@postgrespro.ru
PostgreSQL version: 17.6
Operating system: Ubuntu 22.04
Description:
On PostgreSQL 17+ if you do the following:
-- first psql
CREATE TABLE target (key int primary key, val text);
INSERT INTO target VALUES (1, 'setup1');
CREATE TABLE pa_target (key integer, val text)
PARTITION BY LIST (key);
CREATE TABLE part1 (key integer, val text);
CREATE TABLE part2 (val text, key integer);
CREATE TABLE part3 (key integer, val text);
ALTER TABLE pa_target ATTACH PARTITION part1 FOR VALUES IN (1,4);
ALTER TABLE pa_target ATTACH PARTITION part2 FOR VALUES IN (2,5,6);
ALTER TABLE pa_target ATTACH PARTITION part3 DEFAULT;
INSERT INTO pa_target VALUES (1, 'initial');
INSERT INTO pa_target VALUES (2, 'initial');
BEGIN ISOLATION LEVEL READ COMMITTED;
MERGE INTO target t
USING (SELECT 1 as key, 'merge1' as val) s
ON s.key = t.key
WHEN NOT MATCHED THEN
INSERT VALUES (s.key, s.val)
WHEN MATCHED THEN
UPDATE set key = t.key + 1, val = t.val || ' updated by ' || s.val;
-- second psql
BEGIN ISOLATION LEVEL READ COMMITTED;
EXPLAIN ANALYZE MERGE INTO target t
USING (SELECT 1 as key, 'merge2a' as val) s
ON s.key = t.key
WHEN NOT MATCHED THEN
INSERT VALUES (s.key, s.val)
WHEN MATCHED THEN
UPDATE set key = t.key + 1, val = t.val || ' updated by ' || s.val
WHEN NOT MATCHED BY SOURCE THEN
UPDATE set key = t.key + 1, val = t.val || ' source not matched by
merge2a'
RETURNING merge_action(), t.*;
-- first psql again
COMMIT;
Then the backend for the second psql crashes. With asserts turned on:
TRAP: failed Assert("skipped_path >= 0"), File: "explain.c", Line: 4732,
PID: 66557
postgres: postgres postgres [local]
EXPLAIN(ExceptionalCondition+0xbb)[0x5cd80c4ea887]
postgres: postgres postgres [local] EXPLAIN(+0x34feb6)[0x5cd80bf13eb6]
postgres: postgres postgres [local] EXPLAIN(+0x349ceb)[0x5cd80bf0dceb]
postgres: postgres postgres [local]
EXPLAIN(ExplainPrintPlan+0x1f5)[0x5cd80bf09804]
postgres: postgres postgres [local]
EXPLAIN(ExplainOnePlan+0x43f)[0x5cd80bf0907d]
postgres: postgres postgres [local]
EXPLAIN(standard_ExplainOneQuery+0x30f)[0x5cd80bf08845]
postgres: postgres postgres [local] EXPLAIN(+0x344530)[0x5cd80bf08530]
postgres: postgres postgres [local]
EXPLAIN(ExplainQuery+0x15e)[0x5cd80bf08198]
postgres: postgres postgres [local]
EXPLAIN(standard_ProcessUtility+0xa8a)[0x5cd80c2ec462]
postgres: postgres postgres [local]
EXPLAIN(ProcessUtility+0x13a)[0x5cd80c2eb9d1]
postgres: postgres postgres [local] EXPLAIN(+0x72628a)[0x5cd80c2ea28a]
postgres: postgres postgres [local] EXPLAIN(+0x725fc7)[0x5cd80c2e9fc7]
postgres: postgres postgres [local] EXPLAIN(PortalRun+0x249)[0x5cd80c2e9890]
postgres: postgres postgres [local] EXPLAIN(+0x71e04c)[0x5cd80c2e204c]
postgres: postgres postgres [local]
EXPLAIN(PostgresMain+0xb43)[0x5cd80c2e7843]
postgres: postgres postgres [local] EXPLAIN(+0x719525)[0x5cd80c2dd525]
postgres: postgres postgres [local]
EXPLAIN(postmaster_child_launch+0x174)[0x5cd80c1d8676]
postgres: postgres postgres [local] EXPLAIN(+0x61b1d7)[0x5cd80c1df1d7]
postgres: postgres postgres [local] EXPLAIN(+0x6186cd)[0x5cd80c1dc6cd]
postgres: postgres postgres [local]
EXPLAIN(PostmasterMain+0x159e)[0x5cd80c1dbf97]
postgres: postgres postgres [local] EXPLAIN(main+0x38c)[0x5cd80c072cca]
/lib/x86_64-linux-gnu/libc.so.6(+0x29d90)[0x77c287829d90]
/lib/x86_64-linux-gnu/libc.so.6(__libc_start_main+0x80)[0x77c287829e40]
postgres: postgres postgres [local] EXPLAIN(_start+0x25)[0x5cd80bcaa2c5]
^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: BUG #19111: Using EXPLAIN ANALYZE with MERGE causes failed assert
@ 2025-11-13 15:32 Dean Rasheed <dean.a.rasheed@gmail.com>
parent: PG Bug reporting form <noreply@postgresql.org>
0 siblings, 1 reply; 4+ messages in thread
From: Dean Rasheed @ 2025-11-13 15:32 UTC (permalink / raw)
To: o.tselebrovskiy@postgrespro.ru; pgsql-bugs@lists.postgresql.org
On Thu, 13 Nov 2025 at 12:18, PG Bug reporting form
<noreply@postgresql.org> wrote:
>
> On PostgreSQL 17+ if you do the following:
> ...
> Then the backend for the second psql crashes. With asserts turned on.
Thanks for the report.
What's happening here is that the MERGE in the second query has both
NOT MATCHED BY SOURCE and NOT MATCHED BY TARGET actions, so it does a
full join between the two tables. Initially there is a single matched
row, but the concurrent update turns that into a not matched pair of
rows and both actions are executed. So the ModifyTable node processes
it as 2 rows, whereas its parent node only outputs 1 row, which is
something the explain code doesn't like (because it computes the
difference, interpreting that as the number of rows skipped).
A possible solution would be something like the attached. It feels a
little ugly, but I don't see any other easy fix.
It's only a rough patch (it should have an isolation test case), but
it fixes the problem by causing the parent (full join) node to report
that it returned 2 rows, which it didn't really, but it would have
done, if the other update had happened before the MERGE, rather than
concurrently.
(Of course we could just drop that Assert, and it wouldn't cause any
harm, but it seems preferable to try to get the row counts right.)
Regards,
Dean
Attachments:
[text/x-patch] merge-explain-analyze-fix.patch (1.2K, ../../CAEZATCWqE70qVHv6fbdEXVrD7=hCQ_quKRKnirmukMqaKO5T=Q@mail.gmail.com/2-merge-explain-analyze-fix.patch)
download | inline diff:
diff --git a/src/backend/executor/nodeModifyTable.c b/src/backend/executor/nodeModifyTable.c
new file mode 100644
index 4c5647a..28ace59
--- a/src/backend/executor/nodeModifyTable.c
+++ b/src/backend/executor/nodeModifyTable.c
@@ -3467,7 +3467,27 @@ lmerge_matched:
/* Switch lists, if necessary */
if (!*matched)
+ {
actionStates = mergeActions[MERGE_WHEN_NOT_MATCHED_BY_SOURCE];
+
+ /*
+ * If we have both NOT MATCHED BY SOURCE
+ * and NOT MATCHED BY TARGET actions (a
+ * full join between the source and target
+ * relations), the single previously
+ * matched tuple from the outer plan node
+ * is treated as two not matched tuples,
+ * in the same way as if they had not
+ * matched to start with. Therefore, we
+ * must adjust the outer plan node's tuple
+ * count, if we're instrumenting the
+ * query.
+ */
+ if (outerPlanState(mtstate)->instrument &&
+ mergeActions[MERGE_WHEN_NOT_MATCHED_BY_SOURCE] &&
+ mergeActions[MERGE_WHEN_NOT_MATCHED_BY_TARGET])
+ InstrUpdateTupleCount(outerPlanState(mtstate)->instrument, 1.0);
+ }
}
/*
^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: BUG #19111: Using EXPLAIN ANALYZE with MERGE causes failed assert
@ 2025-11-16 13:20 Dilip Kumar <dilipbalaut@gmail.com>
parent: Dean Rasheed <dean.a.rasheed@gmail.com>
0 siblings, 1 reply; 4+ messages in thread
From: Dilip Kumar @ 2025-11-16 13:20 UTC (permalink / raw)
To: Dean Rasheed <dean.a.rasheed@gmail.com>; +Cc: o.tselebrovskiy@postgrespro.ru; pgsql-bugs@lists.postgresql.org
On Thu, Nov 13, 2025 at 9:02 PM Dean Rasheed <dean.a.rasheed@gmail.com> wrote:
>
> On Thu, 13 Nov 2025 at 12:18, PG Bug reporting form
> <noreply@postgresql.org> wrote:
> >
> > On PostgreSQL 17+ if you do the following:
> > ...
> > Then the backend for the second psql crashes. With asserts turned on.
>
> Thanks for the report.
>
> What's happening here is that the MERGE in the second query has both
> NOT MATCHED BY SOURCE and NOT MATCHED BY TARGET actions, so it does a
> full join between the two tables. Initially there is a single matched
> row, but the concurrent update turns that into a not matched pair of
> rows and both actions are executed. So the ModifyTable node processes
> it as 2 rows, whereas its parent node only outputs 1 row, which is
> something the explain code doesn't like (because it computes the
> difference, interpreting that as the number of rows skipped).
>
> A possible solution would be something like the attached. It feels a
> little ugly, but I don't see any other easy fix.
>
> It's only a rough patch (it should have an isolation test case), but
> it fixes the problem by causing the parent (full join) node to report
> that it returned 2 rows, which it didn't really, but it would have
> done, if the other update had happened before the MERGE, rather than
> concurrently.
IMHO it makes sense to make a full join node to report 2 rows because
if you see internally merge is making the behavior as if it would have
returned 2 rows by parent [1], so I think it's right to fix the
instrument to report that, otherwise the plan might look confusing.
OTOH, someone might argue that we should just show in instrument what
really happened that the parent returned just 1 row and then it got
converted to 2 actions, and for doing that we may just remove the
assert.
I have attached an isolation test for the same.
[1]
if (!matched)
{
/*
* If a concurrent update turned a MATCHED case into a NOT MATCHED
* case, and we have both WHEN NOT MATCHED BY SOURCE and WHEN NOT
* MATCHED [BY TARGET] actions, and there is a RETURNING clause,
* ExecMergeMatched() may have already executed a WHEN NOT MATCHED BY
* SOURCE action, and computed the row to return. If so, we cannot
* execute a WHEN NOT MATCHED [BY TARGET] action now, so mark it as
* pending (to be processed on the next call to ExecModifyTable()).
* Otherwise, just process the action now.
*/
if (rslot == NULL)
rslot = ExecMergeNotMatched(context, resultRelInfo, canSetTag);
else
context->mtstate->mt_merge_pending_not_matched = context->planSlot;
}
--
Regards,
Dilip Kumar
Google
Attachments:
[application/x-patch] 0001-isolation-test.patch (4.6K, ../../CAFiTN-taSkL=4o3=mgPJZb=aSpCNo9giHej+Kyj_biPNrG+oTQ@mail.gmail.com/2-0001-isolation-test.patch)
download | inline diff:
From aafb53357d612e9d25443891fb5dc2c815bc7ad2 Mon Sep 17 00:00:00 2001
From: Dilip Kumar <dilipkumarb@google.com>
Date: Sun, 16 Nov 2025 18:38:38 +0530
Subject: [PATCH] isolation test
---
.../expected/merge-match-to-not-matched.out | 40 +++++++++++++
src/test/isolation/isolation_schedule | 1 +
.../specs/merge-match-to-not-matched.spec | 57 +++++++++++++++++++
3 files changed, 98 insertions(+)
create mode 100644 src/test/isolation/expected/merge-match-to-not-matched.out
create mode 100644 src/test/isolation/specs/merge-match-to-not-matched.spec
diff --git a/src/test/isolation/expected/merge-match-to-not-matched.out b/src/test/isolation/expected/merge-match-to-not-matched.out
new file mode 100644
index 00000000000..11d50210132
--- /dev/null
+++ b/src/test/isolation/expected/merge-match-to-not-matched.out
@@ -0,0 +1,40 @@
+Parsed test spec with 2 sessions
+
+starting permutation: merge_update explain_analyze_merge c1 c2
+step merge_update:
+ MERGE INTO target t
+ USING (SELECT 1 as key, 'merge1' as val) s
+ ON s.key = t.key
+ WHEN NOT MATCHED THEN
+ INSERT VALUES (s.key, s.val)
+ WHEN MATCHED THEN
+ UPDATE set key = t.key + 1, val = t.val || ' updated by ' || s.val;
+
+step explain_analyze_merge:
+ EXPLAIN (ANALYZE, COSTS OFF, TIMING OFF, SUMMARY OFF, BUFFERS OFF) MERGE INTO target t
+ USING (SELECT 1 as key, 'merge2a' as val) s
+ ON s.key = t.key
+ WHEN NOT MATCHED THEN
+ INSERT VALUES (s.key, s.val)
+ WHEN MATCHED THEN
+ UPDATE set key = t.key + 1, val = t.val || ' updated by ' || s.val
+ WHEN NOT MATCHED BY SOURCE THEN
+ UPDATE set key = t.key + 1, val = t.val || ' source not matched by
+ merge2a'
+ RETURNING merge_action(), t.*;
+ <waiting ...>
+step c1: COMMIT;
+step explain_analyze_merge: <... completed>
+QUERY PLAN
+-----------------------------------------------------------
+Merge on target t (actual rows=2.00 loops=1)
+ Tuples: inserted=1 updated=1
+ -> Hash Full Join (actual rows=2.00 loops=1)
+ Hash Cond: (t.key = (1))
+ -> Seq Scan on target t (actual rows=1.00 loops=1)
+ -> Hash (actual rows=1.00 loops=1)
+ Buckets: 1024 Batches: 1 Memory Usage: 9kB
+ -> Result (actual rows=1.00 loops=1)
+(8 rows)
+
+step c2: COMMIT;
diff --git a/src/test/isolation/isolation_schedule b/src/test/isolation/isolation_schedule
index 5afae33d370..a0cf34489c3 100644
--- a/src/test/isolation/isolation_schedule
+++ b/src/test/isolation/isolation_schedule
@@ -59,6 +59,7 @@ test: merge-delete
test: merge-update
test: merge-match-recheck
test: merge-join
+test: merge-match-to-not-matched
test: delete-abort-savept
test: delete-abort-savept-2
test: aborted-keyrevoke
diff --git a/src/test/isolation/specs/merge-match-to-not-matched.spec b/src/test/isolation/specs/merge-match-to-not-matched.spec
new file mode 100644
index 00000000000..97738985687
--- /dev/null
+++ b/src/test/isolation/specs/merge-match-to-not-matched.spec
@@ -0,0 +1,57 @@
+# MERGE MATCHED TO NOT MATCHED
+#
+# This test looks at what happens when we have WHEN MATCHED
+# WHEN NOT MATCHED BY SOURCE and NOT MATCHED BY TARGET and a concurrent UPDATE
+# causes a MATCHED to MATCHED BY SOURCE and NOT MATCHED BY TARGET
+
+setup
+{
+ CREATE TABLE target (key int primary key, val text);
+ INSERT INTO target VALUES (1, 'setup');
+}
+
+teardown
+{
+ DROP TABLE target;
+}
+
+session "s1"
+setup
+{
+ BEGIN ISOLATION LEVEL READ COMMITTED;
+}
+step "merge_update"
+{
+ MERGE INTO target t
+ USING (SELECT 1 as key, 'merge1' as val) s
+ ON s.key = t.key
+ WHEN NOT MATCHED THEN
+ INSERT VALUES (s.key, s.val)
+ WHEN MATCHED THEN
+ UPDATE set key = t.key + 1, val = t.val || ' updated by ' || s.val;
+}
+step "c1" { COMMIT; }
+
+session "s2"
+setup
+{
+ BEGIN ISOLATION LEVEL READ COMMITTED;
+}
+step "explain_analyze_merge"
+{
+ EXPLAIN (ANALYZE, COSTS OFF, TIMING OFF, SUMMARY OFF, BUFFERS OFF) MERGE INTO target t
+ USING (SELECT 1 as key, 'merge2a' as val) s
+ ON s.key = t.key
+ WHEN NOT MATCHED THEN
+ INSERT VALUES (s.key, s.val)
+ WHEN MATCHED THEN
+ UPDATE set key = t.key + 1, val = t.val || ' updated by ' || s.val
+ WHEN NOT MATCHED BY SOURCE THEN
+ UPDATE set key = t.key + 1, val = t.val || ' source not matched by
+ merge2a'
+ RETURNING merge_action(), t.*;
+}
+step "c2" { COMMIT; }
+
+# merge_status sees concurrently updated row and changed matched to not matched by source and not matched by target
+permutation "merge_update" "explain_analyze_merge" "c1" "c2"
--
2.49.0
^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: BUG #19111: Using EXPLAIN ANALYZE with MERGE causes failed assert
@ 2025-11-16 22:41 Dean Rasheed <dean.a.rasheed@gmail.com>
parent: Dilip Kumar <dilipbalaut@gmail.com>
0 siblings, 0 replies; 4+ messages in thread
From: Dean Rasheed @ 2025-11-16 22:41 UTC (permalink / raw)
To: Dilip Kumar <dilipbalaut@gmail.com>; +Cc: o.tselebrovskiy@postgrespro.ru; pgsql-bugs@lists.postgresql.org
On Sun, 16 Nov 2025 at 13:21, Dilip Kumar <dilipbalaut@gmail.com> wrote:
>
> On Thu, Nov 13, 2025 at 9:02 PM Dean Rasheed <dean.a.rasheed@gmail.com> wrote:
> >
> > A possible solution would be something like the attached. It feels a
> > little ugly, but I don't see any other easy fix.
> >
> > It's only a rough patch (it should have an isolation test case), but
> > it fixes the problem by causing the parent (full join) node to report
> > that it returned 2 rows, which it didn't really, but it would have
> > done, if the other update had happened before the MERGE, rather than
> > concurrently.
>
> IMHO it makes sense to make a full join node to report 2 rows because
> if you see internally merge is making the behavior as if it would have
> returned 2 rows by parent [1], so I think it's right to fix the
> instrument to report that, otherwise the plan might look confusing.
> OTOH, someone might argue that we should just show in instrument what
> really happened that the parent returned just 1 row and then it got
> converted to 2 actions, and for doing that we may just remove the
> assert.
After thinking about this some more, I decided that it's preferable to
report 2 rows rather than just removing the Assert because, in a more
complex MERGE with conditional or DO NOTHING actions, there might be
rows that are skipped, and doing it this way ensures that EXPLAIN
ANALYZE correctly reports the number of rows skipped.
> I have attached an isolation test for the same.
I decided to add isolation tests to mege-update.spec, rather than
adding a new spec file, because it already had the necessary setup, so
the new tests just EXPLAIN ANALYZE existing test cases for which the
expected results are already known.
In addition, I added a function to filter the memory usage from the
EXPLAIN output, because that could vary by platform (see similar code
in other regression tests).
Pushed, and back-patched to v17.
Regards,
Dean
^ permalink raw reply [nested|flat] 4+ messages in thread
end of thread, other threads:[~2025-11-16 22:41 UTC | newest]
Thread overview: 4+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2025-11-13 10:04 BUG #19111: Using EXPLAIN ANALYZE with MERGE causes failed assert PG Bug reporting form <noreply@postgresql.org>
2025-11-13 15:32 ` Dean Rasheed <dean.a.rasheed@gmail.com>
2025-11-16 13:20 ` Dilip Kumar <dilipbalaut@gmail.com>
2025-11-16 22:41 ` Dean Rasheed <dean.a.rasheed@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