| From: | Etsuro Fujita <etsuro(dot)fujita(at)gmail(dot)com> |
|---|---|
| To: | Alexander Pyhalov <a(dot)pyhalov(at)postgrespro(dot)ru> |
| Cc: | Alexander Korotkov <aekorotkov(at)gmail(dot)com>, Matheus Alcantara <matheusssilv97(at)gmail(dot)com>, Pgsql Hackers <pgsql-hackers(at)postgresql(dot)org> |
| Subject: | Re: Asynchronous MergeAppend |
| Date: | 2026-11-03 02:20:01 |
| Message-ID: | CAPmGK14YATtpoUk6X54u1sW97mj-tccf4oc0hMX=mzG5mz02VQ@mail.gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi,
On Mon, Nov 2, 2026 at 3:58 PM Alexander Pyhalov
<a(dot)pyhalov(at)postgrespro(dot)ru> wrote:
> Etsuro Fujita писал(а) 2026-11-01 20:44:
> > I also tweaked the comments a bit further.
> + * requestor for a preceding subplan is currently
> in-progress. Skip
> + * the configuration even in the MergeAppend case; in
> that case just
> + * difer the configuration until the caller processes
> the request(s)
> + * for the preceding subplan(s).
> */
>
> s/difer/defer/
Sorry, Fixed.
> Firstly I've confused these tests with another one:
>
> +-- Test ExecMergeAppendAsyncReset code path that asserts async requests
> +-- made for subplans aren't pending anymore at ReScan
> +-- SubPlan case
> +EXPLAIN (VERBOSE, COSTS OFF)
> +SELECT
> + ARRAY(SELECT f.i FROM (SELECT b + g.i FROM async_pt WHERE a >
> g.i ORDER BY b) f(i) ORDER BY f.i LIMIT 10)
> +FROM generate_series(1, 3) g(i);
>
> The reason is obvious - comment is misleading.
> In the previous version of the patch it was
> -- Test async Merge Append rescan
> The original intention was not this, it was to show that valid async
> subplans are recalculated after rescan (and identification of new valid
> asyncplans).
> Likely, the comment has to be fixed.
You put the test case after this one:
-- Test ExecAppendAsyncReset code path that drains outstanding async requests
-- (case where subplans are re-scanned with parameter changes)
EXPLAIN (VERBOSE, COSTS OFF)
SELECT o.x FROM (VALUES (2505), (3505)) o(x), LATERAL (SELECT a FROM
async_pt WHERE a = o.x OR a = 1505 LIMIT 1) s ORDER BY o.x;
and the original comment doesn't really say anything, so I thought the
purpose of the test case would be similar to this one. Sorry.
I understand your intention, but the test case seems misplaced to me,
so I moved it to a more appropriate place and re-added a comment. (I
also added one more test case there.) Attached is a new version of
the patch. What do you think about it?
> And yes, as now we wait for all async requests to deliver results, it
> seems we can't have "old" results from previous runs while looking at
> node->ms_slots, corresponding
> to async plans. So, I suppose that tests that you removed, can be safely
> removed.
OK
Thanks again for reviewing!
Best regards,
Etsuro Fujita
| Attachment | Content-Type | Size |
|---|---|---|
| v25-MergeAppend-should-support-Async-Foreign-Scan-efujita.patch | application/octet-stream | 74.5 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Fujii Masao | 2026-11-03 02:24:10 | Re: [PATCH v1] Free oversized decoded WAL records in XLogReaderFree() |
| Previous Message | Fujii Masao | 2026-11-03 01:57:14 | Re: Wrong LSN in WAL decoding error messages |