| From: | Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)com> |
|---|---|
| To: | Dilip Kumar <dilipbalaut(at)gmail(dot)com> |
| Cc: | o(dot)tselebrovskiy(at)postgrespro(dot)ru, pgsql-bugs(at)lists(dot)postgresql(dot)org |
| Subject: | Re: BUG #19111: Using EXPLAIN ANALYZE with MERGE causes failed assert |
| Date: | 2025-11-16 22:41:52 |
| Message-ID: | CAEZATCXe80QbinX2xE90K6tX0pW2FCMRnXa+r+LwKhUnLW-efA@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-bugs |
On Sun, 16 Nov 2025 at 13:21, Dilip Kumar <dilipbalaut(at)gmail(dot)com> wrote:
>
> On Thu, Nov 13, 2025 at 9:02 PM Dean Rasheed <dean(dot)a(dot)rasheed(at)gmail(dot)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
| From | Date | Subject | |
|---|---|---|---|
| Next Message | ocean_li_996 | 2025-11-17 01:34:03 | Re: BUG #19109: catalog lookup with the wrong snapshot during logical decoding causes coredump |
| Previous Message | Dilip Kumar | 2025-11-16 13:20:51 | Re: BUG #19111: Using EXPLAIN ANALYZE with MERGE causes failed assert |