On Thu, Feb 19, 2026 at 4:48 PM Ashutosh Bapat
<ashutosh.bapat.oss@gmail.com> wrote:
>
> Hi Peter,
>
> On Wed, Feb 18, 2026 at 8:45 PM Peter Eisentraut <peter@eisentraut.org> 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
