pg.ddx.io  pgsql-committers@postgresql.org mailing list archive  
help / color / mirror / Atom feed
From: Tom Lane <tgl@sss.pgh.pa.us>
To: pgsql-committers@lists.postgresql.org
Subject: pgsql: Avoid inserting PlaceHolderVars in cases where pre-v16 PG did no
Date: Fri, 30 Aug 2024 16:42:23 +0000
Message-ID: <E1sk4hb-0029Es-6A@gemulon.postgresql.org> (raw)

Avoid inserting PlaceHolderVars in cases where pre-v16 PG did not.

Commit 2489d76c4 removed some logic from pullup_replace_vars()
that avoided wrapping a PlaceHolderVar around a pulled-up
subquery output expression if the expression could be proven
to go to NULL anyway (because it contained Vars or PHVs of the
pulled-up relation and did not contain non-strict constructs).
But removing that logic turns out to cause performance regressions
in some cases, because the extra PHV blocks subexpression folding,
and will do so even if outer-join reduction later turns it into a
no-op with no phnullingrels bits.  This can for example prevent
an expression from being matched to an index.

The reason for always adding a PHV was to ensure we had someplace
to put the varnullingrels marker bits of the Var being replaced.
However, it turns out we can optimize in exactly the same cases that
the previous code did, because we can instead attach the needed
varnullingrels bits to the contained Var(s)/PHV(s).

This is not a complete solution --- it would be even better if we
could remove PHVs after reducing them to no-ops.  It doesn't look
practical to back-patch such an improvement, but this change seems
safe and at least gets rid of the performance-regression cases.

Per complaint from Nikhil Raj.  Back-patch to v16 where the
problem appeared.

Discussion: https://postgr.es/m/CAG1ps1xvnTZceKK24OUfMKLPvDP2vjT-d+F2AOCWbw_v3KeEgg@mail.gmail.com

Branch
------
REL_17_STABLE

Details
-------
https://git.postgresql.org/pg/commitdiff/b43110869fd82b549df5c6ab7ade476d6e414f37

Modified Files
--------------
src/backend/optimizer/prep/prepjointree.c | 66 +++++++++++++++++++++++++------
src/backend/rewrite/rewriteManip.c        |  9 +++--
src/test/regress/expected/subselect.out   | 29 ++++++++++++++
src/test/regress/sql/subselect.sql        | 18 +++++++++
4 files changed, 107 insertions(+), 15 deletions(-)



view thread (3+ messages)  latest in thread

Message-ID: <E1sk4hb-0029Es-6A@gemulon.postgresql.org>
Permalink:  ../E1sk4hb-0029Es-6A@gemulon.postgresql.org/
Also on:    postgresql.org/message-id/E1sk4hb-0029Es-6A@gemulon.postgresql.org

 · 

reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-committers@postgresql.org
  Cc: tgl@sss.pgh.pa.us, pgsql-committers@lists.postgresql.org
  Subject: Re: pgsql: Avoid inserting PlaceHolderVars in cases where pre-v16 PG did no
  In-Reply-To: <E1sk4hb-0029Es-6A@gemulon.postgresql.org>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox