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

Junwang Zhao <zhjwpku(at)gmail(dot)com> 于2025年9月7日周日 19:12写道：

> Hi,
>
> On Thu, Aug 21, 2025 at 9:07 AM Richard Guo <guofenglinux(at)gmail(dot)com>
> wrote:
> >
> > On Wed, Aug 20, 2025 at 11:11 PM Nathan Bossart
> > <nathandbossart(at)gmail(dot)com> wrote:
> > > On Wed, Aug 20, 2025 at 10:29:03AM +0900, Richard Guo wrote:
> > > > On Wed, Aug 20, 2025 at 2:38 AM Nathan Bossart <
> nathandbossart(at)gmail(dot)com> wrote:
> > > >> There is still an open item for this one, but it's not clear
> whether we are
> > > >> planning to do anything about this for v18, especially since nobody
> has
> > > >> shown measurable performance impact. Does anyone want to argue for
> > > >> addressing this for v18, or shall we close the open item as "Won't
> Fix"?
> >
> > > > I don't think we're likely to do anything about this for v18.
> > > > Actually, I still doubt that the extra table_open call brings any
> > > > measurable performance impact, especially since the lock is already
> > > > held and the relation is likely already present in the relcache.
> > > >
> > > > Also, I still don't think moving the expansion of virtual generated
> > > > columns to the rewriter (as Tom proposed) is a better idea. It
> turned
> > > > out to have several problems that need to be fixed with the help of
> > > > PHVs, which is why we moved the expansion into the planner.
> >
> > > Okay. I have marked the v18 open item as "Won't Fix".
> >
> > Thank you for helping with this.
> >
> > Thanks
> > Richard
> >
> >
>
> While reading this thread, I found that it uses *Relids* to collect NOT
> NULL
> attribute numbers, I think this might be an oversight, since ISTM that
> Relids is used to represent the index of the relation in the range table.
>
> I searched the code base and it seems nowhere to use Relids to represent
> attribute numbers, and there is a *notnullattnums* field in RelOptInfo:
>
> /* zero-based set containing attnums of NOT NULL columns */
> Bitmapset *notnullattnums;
>
> So I think it would be better to be consistent, anyway I post a trivial
> patch
> if the community agrees with me.
>
> --
> Regards
> Junwang Zhao
>

+1
From the code readability perspective, Bitmapset* seems better.
--
Thanks,
Tender Wang