[COMMITTERS] pgsql: In the planner,delete joinaliasvars lists after we're done with - Mailing list pgsql-committers

From Tom Lane
Subject [COMMITTERS] pgsql: In the planner,delete joinaliasvars lists after we're done with
Date
Msg-id E1e77uV-0002qw-KC@gemulon.postgresql.org
Whole thread Raw
List pgsql-committers
In the planner, delete joinaliasvars lists after we're done with them.

Although joinaliasvars lists coming out of the parser are quite simple,
those lists can contain arbitrarily complex expressions after subquery
pullup.  We do not perform expression preprocessing on them, meaning that
expressions in those lists will not meet the expectations of later phases
of the planner (for example, that they do not contain SubLinks).  This had
been thought pretty harmless, since we don't intentionally touch those
lists in later phases --- but Andreas Seltenreich found a case in which
adjust_appendrel_attrs() could recurse into a joinaliasvars list and then
die on its assertion that it never sees a SubLink.  We considered a couple
of localized fixes to prevent that specific case from looking at the
joinaliasvars lists, but really this seems like a generic hazard for all
expression processing in the planner.  Therefore, probably the best answer
is to delete the joinaliasvars lists from the parsetree at the end of
expression preprocessing, so that there are no reachable expressions that
haven't been through preprocessing.

The case Andreas found seems to be harmless in non-Assert builds, and so
far there are no field reports suggesting that there are user-visible
effects in other cases.  I considered back-patching this anyway, but
it turns out that Andreas' test doesn't fail at all in 9.4-9.6, because
in those versions adjust_appendrel_attrs contains code (added in commit
842faa714 and removed again in commit 215b43cdc) to process SubLinks
rather than complain about them.  Barring discovery of another path by
which unprocessed joinaliasvars lists can cause trouble, the most
prudent compromise seems to be to patch this into v10 but not further.

Patch by me, with thanks to Amit Langote for initial investigation
and review.

Discussion: https://postgr.es/m/87r2tvt9f1.fsf@ansel.ydns.eu

Branch
------
REL_10_STABLE

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

Modified Files
--------------
src/backend/optimizer/plan/planner.c    | 32 +++++++++++++++++++++++++++-----
src/test/regress/expected/subselect.out | 14 ++++++++++++++
src/test/regress/sql/subselect.sql      | 18 ++++++++++++++++++
3 files changed, 59 insertions(+), 5 deletions(-)


--
Sent via pgsql-committers mailing list (pgsql-committers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-committers

pgsql-committers by date:

Previous
From: Peter Geoghegan
Date:
Subject: Re: [COMMITTERS] pgsql: Fix traversal of half-frozen update chains
Next
From: Andrew Dunstan
Date:
Subject: [COMMITTERS] pgsql: Process variadic arguments consistently in json functions