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: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
Cc: 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 03:31:30
Message-ID: CAApHDvqhr1eU9v42w5gt_gL=G15pB9RBpAH_Jmg7eDU6iZ_2zg@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-bugs pgsql-hackers

On Thu, 7 Jul 2022 at 15:07, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> While I don't have any problem with tracking column NOT NULL flags
> in RelOptInfo once the planner has a use for that info, I'm not sure
> that we have a solid use-case for it quite yet. In particular, the
> fact that the table column is marked NOT NULL doesn't mean that any
> particular occurrence of that column's Var can be freely assumed to be
> non-null. The patch I'm working on to label Vars that have possibly
> been nulled by outer joins [1] seems like essential infrastructure for
> doing anything very useful with the info.

I was aware that you'd done that work. I'm interested in it, but just
not found the time to look yet.

> Maybe that objection doesn't apply to build_minmax_path's usage in
> particular, but that's an awfully narrow use-case.

I thought I'd quickly put the idea together and fairly quickly noticed
that we do preprocess_minmax_aggregates() in grouping_planner(), which
is long before we load the RelOptInfo data in
add_base_rels_to_query(), which is called in query_planner(). I
considered if we could move the preprocess_minmax_aggregates(), but
that does not seem right, although, surprisingly, no tests seem to
fail from doing so. I'd have expected at least some EXPLAIN outputs to
have changed from the no-longer-present IS NOT NULL quals.

I imagine a much less narrow case would be to check for redundant
RestrictInfos in distribute_restrictinfo_to_rels(). That would also
catch cases such as WHERE non_nullable_col IS NULL, provided that qual
made it down to baserestrictinfo. When I realised that, I thought I
might be starting to overlap with your work in the link below.

> [1] https://www.postgresql.org/message-id/flat/830269(dot)1656693747(at)sss(dot)pgh(dot)pa(dot)us

The 2 attached patches do fix the bad reported plan, it's just that
it's a very roundabout way of fixing it

Anyway, I've no current plans to take the attached any further. I
think it'll be better to pursue your NULLable-Var stuff and see if we
can do something more generic like remove provably redundant NullTests
from baserestrictinfo.

David

Attachment Content-Type Size
v1-0001-Track-NOT-NULL-columns-in-RelOptInfo-for-base-rel.patch text/plain 2.5 KB
v1-0002-Don-t-add-is-NOT-NULL-clause-to-MinMaxAggInfo-tra.patch text/plain 4.0 KB

In response to

Responses

Browse pgsql-bugs by date

  From Date Subject
Next Message Tom Lane 2022-07-07 03:50:48 Re: BUG #17540: Prepared statement: PG switches to a generic query plan which is consistently much slower
Previous Message Tom Lane 2022-07-07 03:13:18 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 shiy.fnst@fujitsu.com 2022-07-07 03:31:57 RE: Perform streaming logical transactions by background workers and parallel apply
Previous Message Tom Lane 2022-07-07 03:15:30 Re: tuplesort Generation memory contexts don't play nicely with index builds