Re: BUG #17540: Prepared statement: PG switches to a generic query plan which is consistently much slower

From: David Rowley <dgrowleyml(at)gmail(dot)com>
To: william(dot)duclot(at)gmail(dot)com, PostgreSQL mailing lists <pgsql-bugs(at)lists(dot)postgresql(dot)org>
Subject: Re: BUG #17540: Prepared statement: PG switches to a generic query plan which is consistently much slower
Date: 2022-07-07 00:23:01
Message-ID: CAApHDvqRR81Dvur_zcrY+RsKxv_e8uDpJ2xxAPMmnfgDY7VNew@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-bugs pgsql-hackers

(On Thu, 7 Jul 2022 at 09:41, PG Bug reporting form
<noreply(at)postgresql(dot)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

In response to

Responses

Browse pgsql-bugs by date

  From Date Subject
Next Message David Rowley 2022-07-07 00:46:17 Re: BUG #17540: Prepared statement: PG switches to a generic query plan which is consistently much slower
Previous Message David G. Johnston 2022-07-06 22:07:46 Re: BUG #17540: Prepared statement: PG switches to a generic query plan which is consistently much slower

Browse pgsql-hackers by date

  From Date Subject
Next Message David Rowley 2022-07-07 00:46:17 Re: BUG #17540: Prepared statement: PG switches to a generic query plan which is consistently much slower
Previous Message Michael Paquier 2022-07-07 00:11:57 Re: Bump MIN_WINNT to 0x0600 (Vista) as minimal runtime in 16~