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

From: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>
To: Robert Haas <robertmhaas(at)gmail(dot)com>
Cc: Richard Guo <guofenglinux(at)gmail(dot)com>, David Rowley <dgrowleyml(at)gmail(dot)com>, Tender Wang <tndrwang(at)gmail(dot)com>, Pg Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Reduce "Var IS [NOT] NULL" quals during constant folding
Date: 2025-04-10 19:54:39
Message-ID: 602561.1744314879@sss.pgh.pa.us
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Robert Haas <robertmhaas(at)gmail(dot)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

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andres Freund 2025-04-10 20:05:42 Re: pgsql: Add function to get memory context stats for processes
Previous Message Robert Haas 2025-04-10 19:45:28 Re: Reduce "Var IS [NOT] NULL" quals during constant folding