On Thu, Feb 19, 2026 at 4:48 PM Ashutosh Bapat <[email protected]> wrote: > > Hi Peter, > > On Wed, Feb 18, 2026 at 8:45 PM Peter Eisentraut <[email protected]> wrote: > > > > We have three different functions called validate_relation_kind(), namely in > > > > src/backend/access/index/indexam.c > > src/backend/access/sequence/sequence.c > > src/backend/access/table/table.c > > > > These all check which relkinds are permitted by index_open(), > > sequence_open(), and table_open(), respectively, which are each wrappers > > around relation_open() (which accepts any relkind). > > > > It's better to use a different name for each of them as done in > attached 0003. Same names can confuse code browsing tools like cscope > and human reader alike. > > > I always found the one in table.c a little too mysterious, because it > > just checks which relkinds it does *not* want, and so if you want to > > know whether a particular relkind is suitable for table_open(), you need > > to do additional research and check what all the possible relkinds are > > and so on, and there is no real explanation why those choices were made. > > I think it would be clearer and more robust and also more consistent > > with the other ones if we flipped that around and listed the ones that > > are acceptable and why. > > > > The || should be &&. The bug shows up as an initdb failure
Yes, I noticed the same issue, using || will always evaluate to true, which means it will always error out. > running bootstrap script ... 2026-02-19 14:06:43.411 IST [197482] > FATAL: cannot open relation "pg_type" > 2026-02-19 14:06:43.411 IST [197482] DETAIL: This operation is not > supported for tables. > > I think this is more future-proof. If a relkind gets added and needs > to be in this list, we will notice it from the error. I think we > should avoid mentioning specific relkinds in the comment as well since > that list will need to be updated as the set of relkinds changes. Just > mentioning the criteria should be enough. I have slightly improved the > comment in the attached 0003. The renaming looks reasonable to me. > > > Secondly, the sequence.c one was probably copied from the table.c one, > > but I think we can make the error message a bit more direct by just > > saying "... is not a sequence" instead of "cannot open relation". > > > > +1. > > > These are the two attached patches. This is just something I found > > while working on something else nearby. > > Attached are your two patches + bug fix in 0002 + my suggestions in 0003. > > -- > Best Wishes, > Ashutosh Bapat -- Regards Junwang Zhao
