On Mon, Apr 6, 2026 at 4:39 PM Tomas Vondra <tomas@vondra.me> wrote:
>
> On 4/6/26 18:50, Melanie Plageman wrote:
>
> > I cleaned up the first patch in the set that refactors index-only and
> > index scan to use this pattern and realized that I wasn't sure what to
> > do about the duplication between index-only and index scans for these
> > functions.
>
<--snip-->
>
> I think this is ready to go, with a tiny amount of polishing.
>
> 1) "amount of DSM" sounds a bit strange to me. The wording "amount of
> space in ..." from the other nodes seems better to me. Or maybe I'm just
> used to it, not sure.
Fixed that before pushing.
> 2) I wonder if maybe PARALLEL_KEY_SCAN_INSTRUMENT_OFFSET should be
> placed in plannodes.h, because that's where Plan->plan_node_id is
> defined. instrument_node.h works too, but the places accessing the
> plan_node_id already have to include plannodes.h.
I don't think it really belongs there. It is very specific to
execution. We use the plan node id as the key for convenience, but it
isn't the purpose of plan_node_id.
PARALLEL_KEY_SCAN_INSTRUMENT_OFFSET's only purpose is to be used
during execution for instrumentation, so it feels like it belongs in
node_instrument.h. And we don't need a separate include for it either.
It goes with the other stuff being defined in instrument_node.h (like
SharedIndexScanInstrumentation) and being used by those callers. I
admit the comment above it is a bit odd, but I think it is ultimately
okay.
> We need to decide whether to push this into PG19. This was primarily
> motivated by the index prefetching work, but we now know that won't
> happen in PG19 :-( But the instrumentation is useful even for the three
> scans using read streams, so I think it's a meaningful improvement.
I think it is a meaningful improvement too. I think we should do it.
> If you think you can get this pushed, I'll do my best to finalize the
> instrumentation and SeqScan/TidRangeScan parts.
I've pushed the first patch for index/index-only scans. Attached is
the BHS fix that uses the new pattern. It needs at least one review
before pushing. While I was polishing it, I realized I neglected to
use add_size()/mul_size() in the index-only/index scan patches. So,
0002 is just a fix commit to do that. Feel free to push these if you
think they're ready. Otherwise, I'll do so pending your review in my
morning.
- Melanie
