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

From: Peter Geoghegan <[email-masked]>
Date: Thu, 15 Aug 2024 17:10:01 -0400

On Thu, Aug 15, 2024 at 4:34 PM Matthias van de Meent
<[email-masked]> wrote:
> > Attached patch has EXPLAIN ANALYZE display the total number of
> > primitive index scans for all 3 kinds of index scan node. This is
> > useful for index scans that happen to use SAOP arrays. It also seems
> > almost essential to offer this kind of instrumentation for the skip
> > scan patch [1]. Skip scan works by reusing all of the Postgres 17 work
> > (see commit 5bf748b8) to skip over irrelevant sections of a composite
> > index with a low cardinality leading column, so it has all the same
> > issues.
>
> Did you notice the patch over at [0], where additional diagnostic
> EXPLAIN output for btrees is being discussed, too?

To be clear, for those that haven't been paying attention to that
other thread: that other EXPLAIN patch (the one authored by Masahiro
Ikeda) surfaces information about a distinction that the skip scan
patch renders obsolete. That is, the skip scan patch makes all "Non
Key Filter" quals into quals that can relocate the scan to a later
leaf page by starting a new primitive index scan. Technically, skip
scan removes the concept that that patch calls "Non Key Filter"
altogether.

Note that this isn't the same thing as making that other patch
obsolete. Skip scan renders the whole concept of "Non Key Filter"
obsolete *in name only*. You might prefer to think of it as making
that whole concept squishy. Just because we can theoretically use the
leading column to skip doesn't mean we actually will. It isn't an
either/or thing. We might skip during some parts of a scan, but not
during other parts.

It's just not clear how to handle those sorts of fuzzy distinctions
right now. It does seem worth pursuing, but I see no conflict.

> I'm asking, because
> I'm not very convinced that 'primitive scans' are a useful metric
> across all (or even: most) index AMs (e.g. BRIN probably never will
> have a 'primitive scans' metric that differs from the loop count), so
> maybe this would better be implemented in that framework?

What do you mean by "within that framework"? They seem orthogonal?

It's true that BRIN index scans will probably never show more than a
single primitive index scan. I don't think that the same is true of
any other index AM, though. Don't they all support SAOPs, albeit
non-natively?

The important question is: what do you want to do about cases like the
BRIN case? Our choices are all fairly obvious choices. We can be
selective, and *not* show this information when a set of heuristics
indicate that it's not relevant. This is fairly straightforward to
implement. Which do you prefer: overall consistency, or less
verbosity?

Personally I think that the consistency argument works in favor of
displaying this information for every kind of index scan. That's a
hopelessly subjective position, though.

-- 
Peter Geoghegan

---

From: Matthias van de Meent <[email-masked]>
Date: Thu, 15 Aug 2024 23:46:51 +0200

> either/or thing. We might skip during some parts of a scan, but not

Yes.

> What do you mean by "within that framework"? They seem orthogonal?

What I meant was putting this 'primitive scans' info into the
AM-specific explain callback as seen in the latest patch version.

> any other index AM, though. Don't they all support SAOPs, albeit

Not always. For Bitmap Index Scan the node's functions can allow
non-native SAOP support (it ORs the bitmaps), but normal indexes
without SAOP support won't get SAOP-functionality from the IS/IOS
node's infrastructure, it'll need to be added as Filter.

> implement. Which do you prefer: overall consistency, or less

Consistency, I suppose. But adding explain attributes left and right
in Index Scan's explain output when and where every index type needs
them doesn't scale, so I'd put index-specific output into it's own
system (see the linked thread for more rationale). And, in this case,
the use case seems quite index-specific, at least for IS/IOS nodes.

> displaying this information for every kind of index scan.

Agreed, assuming "this information" is indeed shared (and useful)
across all AMs.

This made me notice that you add a new metric that should generally be
exactly the same as pg_stat_all_indexes.idx_scan (you mention the
same). Can't you pull that data, instead of inventing a new place
every AMs needs to touch for it's metrics?

Kind regards,

Matthias van de Meent
Neon (https://neon.tech)