On Thu, Aug 21, 2025 at 9:07 AM Richard Guo <guofenglinux@gmail.com> wrote: > > On Wed, Aug 20, 2025 at 11:11 PM Nathan Bossart > <nathandbossart@gmail.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@gmail.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.
