Re: EXPLAIN: showing ReadStream / prefetch stats

On 3/31/26 20:12, Andres Freund wrote:
> Hi,
>
> On 2026-03-31 20:03:02 +0200, Tomas Vondra wrote:
>> On 3/31/26 19:41, Andres Freund wrote:
>>> Hi,
>>>
>>> On 2026-03-30 20:21:29 +0200, Tomas Vondra wrote:
>>>>>> From 410eaaebe7b814ac9f44c080e153f4ec1d6d6b86 Mon Sep 17 00:00:00 2001
>>>>>> From: Tomas Vondra <tomas(at)vondra(dot)me>
>>>>>> Date: Thu, 19 Mar 2026 22:25:09 +0100
>>>>>> Subject: [PATCH v5 3/6] explain: show prefetch stats in EXPLAIN (ANALYZE)
>>>>>>
>>>>>> This adds details about AIO / prefetch for executor nodes using a
>>>>>> ReadStream. Right now this applies only to BitmapHeapScan, because
>>>>>> that's the only scan node using a ReadStream and collecting
>>>>>> instrumentation from workers.
>>>>>
>>>>> I don't understand why that means it should be done as part of this commit,
>>>>> whereas seqscans shouldn't?
>>>>>
>>>>
>>>> Are you suggesting the commit adds support for all those scans (BHS,
>>>> SeqScan and TidRangeScan) or none of them?
>>>
>>> I guess I mostly just didn't quite understand what differentiates bitmap scans
>>> from the other scans, based on this explanation.
>>>
>>>
>>>> To me it seems better to have at least some scan because of testing. But
>>>> SeqScan/TidRangeScan don't have the instrumentation infrastructure for
>>>> parallel queries, and I don't want to do that in the main patch - it seems
>>>> rather unrelated. And I also don't want to add it before the main patch.
>>>
>>> I'd probably do the latter, i.e. add it before the main patch. Or at least
>>> separately from the change to show read stream instrumentation.
>>>
>>
>> Sorry, I'm confused. Which "latter" option you mean?
>
> Adding the instrumentation infrastructure for seqscan, tidscan as a separate
> patch before the main commit.
>

I don't like that, because then we're adding instrumentation that
doesn't actually contain anything. How would you even know it does the
right thing? I think it's better to add that after the main commit.

regards

--
Tomas Vondra