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

From: Matthias van de Meent <boekewurm+postgres(at)gmail(dot)com>
To: Peter Geoghegan <pg(at)bowt(dot)ie>
Cc: PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Showing primitive index scan count in EXPLAIN ANALYZE (for skip scan and SAOP scans)
Date: 2024-08-15 21:46:51
Message-ID: CAEze2WgnAA9xH7D8SfSO=pNhofTe-DkJw_EycyR3Fgf4gwOihQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Thu, 15 Aug 2024 at 23:10, Peter Geoghegan <pg(at)bowt(dot)ie> wrote:
>
> On Thu, Aug 15, 2024 at 4:34 PM Matthias van de Meent
> <boekewurm+postgres(at)gmail(dot)com> 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.

Yes.

> 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?

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

> 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?

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.

> 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?

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.

> Personally I think that the consistency argument works in favor of
> 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)

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Heikki Linnakangas 2024-08-15 22:03:57 Re: Add new protocol message to change GUCs for usage with future protocol-only GUCs
Previous Message Peter Geoghegan 2024-08-15 21:45:02 Re: Showing primitive index scan count in EXPLAIN ANALYZE (for skip scan and SAOP scans)