Re: Postgres: Queries are too slow after upgrading to PG17 from PG15

Peter Geoghegan <pg@bowt.ie> writes:
> On Mon, Aug 4, 2025 at 6:26 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> We have our hands on enough
>> info about the index that we could skip trying to do the sort if
>> it's not a btree, and I think we should do so, at least for today.

> I think that this boils down to the following (correct me if I'm wrong):

> There is a need to remember that we've presorted, rather than making
> any kind of hardcoded assumption that that has happened. In practice
> we'll always presort with a Const-arg'd ScalarArrayOp nbtree index
> qual, but ExecIndexBuildScanKeys has no business knowing about any of
> that. This can be achieved by limiting application of the optimization
> to nbtree from within createplan.c -- probably by testing
> "IndexOptInfo.relam == BTREE_AM_OID" (much like
> match_rowcompare_to_indexcol does it already).

I agree that ExecIndexBuildScanKeys should not be hard-wiring such
an assumption: it's too far away from both the producer and the
consumer of the knowledge.  Having said that, if we're to produce
a back-patchable fix, I'm not seeing a way to have an end-to-end
confirmation that the planner did the sort.  That would require some
sort of change to the plan tree.  I'd be for doing that in master
only, but I think it's unworkable given ABI restrictions in v17.

I think the most expedient answer for v17 is to have a private
agreement between createplan.c and access/nbtree that if the
index type is btree then createplan.c will presort Const array
arguments for ScalarArrayOp indexquals.  The nbtree code would
have to check for this by looking to see if the indexqual node
is a Const.

I do agree with the idea that a scankey flag could be a better
answer in HEAD.  But it has to be fed forward from some planner
output, or else it's just a kluge.  Not that what I said in the
previous para isn't a kluge ... but my kluge touches fewer bits
of code than your v1, so I think it's better.  Ugliness is in
the eye of the beholder of course, so maybe you'll disagree.

Anyway, my points are (1) it's definitely not safe to call the
BTORDER_PROC unless the index is a btree, and (2) we should not
expect that what we do today to put into the back branches will
be a good long-term answer.

> And if it does, do you have any
> thoughts on the best place for the new "presorted" struct field?
> Should it go in the ScalarArrayOpExpr struct itself?

Doubt it.  I suppose if we had an immediate application for
presorting ScalarArrayOpExpr arrays in other contexts, that
could start to look attractive.  But for now I'd think more
about adding additional info to IndexScan plan nodes.  Maybe
it'd be interesting to store an array of scankey flag bitmasks,
with an eye to precomputing some of those flags at plan time?

            regards, tom lane