Re: BUG #17540: Prepared statement: PG switches to a generic query plan which is consistently much slower - Mailing list pgsql-bugs

From David Rowley
Subject Re: BUG #17540: Prepared statement: PG switches to a generic query plan which is consistently much slower
Date
Msg-id CAApHDvqRR81Dvur_zcrY+RsKxv_e8uDpJ2xxAPMmnfgDY7VNew@mail.gmail.com
Whole thread Raw
In response to BUG #17540: Prepared statement: PG switches to a generic query plan which is consistently much slower  (PG Bug reporting form <noreply@postgresql.org>)
Responses Re: BUG #17540: Prepared statement: PG switches to a generic query plan which is consistently much slower  (David Rowley <dgrowleyml@gmail.com>)
Re: BUG #17540: Prepared statement: PG switches to a generic query plan which is consistently much slower  (Tom Lane <tgl@sss.pgh.pa.us>)
List pgsql-bugs
(On Thu, 7 Jul 2022 at 09:41, PG Bug reporting form
<noreply@postgresql.org> wrote:
> Scenario:
> - I create a fairly simple table (id + timestamp). Timestamp is indexed.
> - I create a simple-ish prepared statement for `SELECT MIN(id), MAX(id) from
> relation_tuple_transaction WHERE timestamp >= $1;`
> - I execute the prepared statement multiple times (> 5 times)
>
> From the 6th time onwards, the query plan used by Postgres changes, which
> isn't fully unexpected as the documentation linked above does make it clear
> that Postgres might decide to change the query plan for a generic query plan
> after the 5th execution. And indeed, the estimated "cost" of the generic
> plan is lower than the custom plan's: therefore the query planner behaves
> correctly according to the documentation.

It's a pretty narrow fix for a fairly generic problem, but I think the
planner wouldn't have picked the pk_rttx index if build_minmax_path()
hadn't added the "id IS NOT NULL" qual.

I know that Andy Fan has been proposing a patch to add a Bitmapset
field to RelOptInfo to record the non-NULLable columns. That's a
fairly lightweight patch, so it might be worth adding that just so
build_minmax_path() can skip adding the NULL test if the column is a
NOT NULL column.

I see that preprocess_minmax_aggregates() won't touch anything that's
not a query to a single relation, so the Var can't be NULLable from
being on the outside of an outer join. So it looks like to plumb in
Andy's patch, build_minmax_path() would need to be modified to check
if mminfo->target is a plain Var and then test if that Var is NOT
NULLable then skip adding the NullTest.

All seems fairly trivial. It's just a fairly narrow fix to side-step a
more generic costing problem we have for Params.  I just don't have
any bright ideas on how to fix the more generic problem right now.

I've been looking for a good excuse to commit Andy's NOT NULL patch so
that he has some more foundations for the other work he's doing.  This
might be that excuse.

Does anyone think differently?

David

[1] https://www.postgresql.org/message-id/CAKU4AWoZrFaWAkTn9tE2_dd4RYnUiQUiX8xc=ryUywhBWQv89w@mail.gmail.com



pgsql-bugs by date:

Previous
From: "David G. Johnston"
Date:
Subject: Re: BUG #17540: Prepared statement: PG switches to a generic query plan which is consistently much slower
Next
From: David Rowley
Date:
Subject: Re: BUG #17540: Prepared statement: PG switches to a generic query plan which is consistently much slower