Horton

Re: Reduce "Var IS [NOT] NULL" quals during constant folding

Tom Lane <tgl@sss.pgh.pa.us>

View thread
From: Tom Lane <tgl@sss.pgh.pa.us>
To: Robert Haas <robertmhaas@gmail.com>
Cc: Richard Guo <guofenglinux@gmail.com>, David Rowley <dgrowleyml@gmail.com>, Tender Wang <tndrwang@gmail.com>, Pg Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2025-04-10T19:54:39Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Fix misuse of Relids for storing attribute numbers

  2. Reduce "Var IS [NOT] NULL" quals during constant folding

  3. Centralize collection of catalog info needed early in the planner

  4. Expand virtual generated columns before sublink pull-up

  5. Expand virtual generated columns in the planner

Robert Haas <robertmhaas@gmail.com> writes:
> OK. Maybe I shouldn't be worrying about the table_open() /
> table_close() here, because I see that you are right that
> has_subclass() is nearby, which admittedly does not involve opening
> the relation, but it does involve fetching from the syscache a tuple
> that we wouldn't need to fetch if we had a Relation available at that
> point. And I see now that expand_virtual_generated_columns() is also
> in that area and works similar to your proposed function in that it
> just opens and closes all the relations. Perhaps that's just the way
> we do things at this very early stage of the planner? But I feel like
> it might make sense to do some reorganization of this code, though, so
> that it more resembles the phase 1 and phase 2 as you describe them.

Indeed, I think those are hacks that we should get rid of, not
emulate.  Note in particular that expand_virtual_generated_columns
is new in v18 and has exactly zero credibility as precedent.  In fact,
I'm probably going to harass Peter about fixing it before v18 ships.
Randomly adding table_opens in the planner is not a route to high
planning performance.

> A related point that I'm noticing is that you record the not-NULL
> information in the RangeTblEntry.

Did we not just complain about that w.r.t. the v1 version of this
patch?  RangeTblEntry is not where to store this info.  We need
a new data structure, and IMO the data structure should be a hashtable
based on table OID, not relid.  That way we can amortize across
multiple references to the same table within a query.

			regards, tom lane