Re: some validate_relation_kind() tidying

From: Ashutosh Bapat <ashutosh(dot)bapat(dot)oss(at)gmail(dot)com>
To: Peter Eisentraut <peter(at)eisentraut(dot)org>
Cc: pgsql-hackers <pgsql-hackers(at)postgresql(dot)org>
Subject: Re: some validate_relation_kind() tidying
Date: 2026-02-19 08:48:22
Message-ID: CAExHW5v9LTRuqqjS-N5g0n+cwGJLSf9+73ACgy4vFVzgzFb36A@mail.gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi Peter,

On Wed, Feb 18, 2026 at 8:45 PM Peter Eisentraut <peter(at)eisentraut(dot)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
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.

> 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

Attachment Content-Type Size
v20260219-0001-Change-error-message-for-sequence-validate.patch text/x-patch 2.2 KB
v20260219-0002-Flip-logic-in-table-validate_relation_kind.patch text/x-patch 2.0 KB
v20260219-0003-Rename-validate_relation_kind.patch text/x-patch 5.5 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Peter Eisentraut 2026-02-19 08:54:30 Use standard C23 and C++ attributes if available
Previous Message Richard Guo 2026-02-19 08:40:08 Remove obsolete SAMESIGN macro