Re: Showing primitive index scan count in EXPLAIN ANALYZE (for skip scan and SAOP scans)

On Thu, Mar 6, 2025 at 2:12 PM Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> Well, I think this calls the basic design into question. We discussed
> putting this into IndexScanDescData as a convenient way of piping it
> through to EXPLAIN, but what I think we have now discovered is that
> there isn't actually convenient at all, because every process has its
> own IndexScanDescData and the leader sees only its own.\

I agree that it isn't convenient. But there's an inescapable need to
pass *something* down to amgettuple. Everything that we currently pass
to amgettuple goes through the IndexScanDesc arg (its only other arg
is ScanDirection), which isn't a bad reason to put this here too.

So I still think that we need to either store something like nsearches
in IndexScanDescData, or store a pointer to some other struct that
contains the nsearches field (and possibly other fields, in the
future). The only alternative is to change the amtuple signature
(e.g., pass down planstate), which doesn't seem like an improvement.

> It seems like
> what you need is to have each process accumulate stats locally, and
> then at the end total them up. Maybe show_sort_info() has some useful
> precedent, since that's also a bit of node-specific instrumentation,
> and it seems to know what to do about workers.

That seems similar to the hash join case I looked at.

I think that the main problem with the reverted patch isn't that it
uses IndexScanDescData -- that detail is almost inevitable. The main
problem is that it failed to teach
nodeIndexscan.c/nodeIndexonlyscan.c/nodeBitmapIndexscan.c to place the
IndexScanDescData.nsearches counter somewhere where explain.c could
later get to reliably. That'd probably be easier if
IndexScanDescData.nsearches was a pointer instead of a raw integer.

Thanks
--
Peter Geoghegan