Horton

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

Richard Guo <guofenglinux@gmail.com>

View thread
From: Richard Guo <guofenglinux@gmail.com>
To: David Rowley <dgrowleyml@gmail.com>
Cc: Tender Wang <tndrwang@gmail.com>, Tom Lane <tgl@sss.pgh.pa.us>, Robert Haas <robertmhaas@gmail.com>, Pg Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2025-03-27T07:10:35Z
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

On Wed, Mar 26, 2025 at 6:45 PM David Rowley <dgrowleyml@gmail.com> wrote:
> On Wed, 26 Mar 2025 at 19:31, Richard Guo <guofenglinux@gmail.com> wrote:
> > On Wed, Mar 26, 2025 at 3:06 PM Tender Wang <tndrwang@gmail.com> wrote:
> > > The comment about  notnullattnums in struct RangeTblEntry says that:
> > > * notnullattnums is zero-based set containing attnums of NOT NULL
> > > * columns.
> > >
> > > But in get_relation_notnullatts():
> > > rte->notnullattnums = bms_add_member(rte->notnullattnums,
> > >                                                                     i + 1);
> > >
> > > The notnullattnums seem to be 1-based.

> > This corresponds to the attribute numbers in Var nodes; you can
> > consider zero as representing a whole-row Var.

> Yeah, and a negative number is a system attribute, which the Bitmapset
> can't represent... The zero-based comment is meant to inform the
> reader that they don't need to offset by
> FirstLowInvalidHeapAttributeNumber when indexing the Bitmapset. If
> there's some confusion about that then maybe the wording could be
> improved. I used "zero-based" because I wanted to state what it was
> and that was the most brief terminology that I could think of to do
> that. The only other way I thought about was to say that "it's not
> offset by FirstLowInvalidHeapAttributeNumber", but I thought it was
> better to say what it is rather than what it isn't.
>
> I'm open to suggestions if people are confused about this.

I searched the current terminology used in code and can find "offset
by FirstLowInvalidHeapAttributeNumber", but not "not offset by
FirstLowInvalidHeapAttributeNumber".  I think "zero-based" should be
sufficient to indicate that this bitmapset is offset by zero, not by
FirstLowInvalidHeapAttributeNumber.  So I'm fine to go with
"zero-based".

I'm planning to push this patch soon, barring any objections.

Thanks
Richard