Re: Sequence Access Methods, round two - Mailing list pgsql-hackers

From Matthias van de Meent
Subject Re: Sequence Access Methods, round two
Date
Msg-id CAEze2WiMGNG9XK3NSUen-5BARhCnP=u=FXnf8pvpL2qDKeOsZg@mail.gmail.com
Whole thread Raw
In response to Re: Sequence Access Methods, round two  (Michael Paquier <michael@paquier.xyz>)
Responses Re: Sequence Access Methods, round two
List pgsql-hackers
On Mon, 26 Feb 2024 at 09:11, Michael Paquier <michael@paquier.xyz> wrote:
>
> On Thu, Feb 22, 2024 at 05:36:00PM +0100, Tomas Vondra wrote:
> > 0002, 0003
> > ------------
> > seems fine, cosmetic changes
>
> Thanks, I've applied these two for now.  I'll reply to the rest
> tomorrow or so.

Huh, that's surprising to me. I'd expected this to get at least a
final set of patches before they'd get committed. After a quick check
6e951bf seems fine, but I do have some nits on 449e798c:

> +/* ----------------
> + *        validate_relation_kind - check the relation's kind
> + *
> + *        Make sure relkind is from an index

Shouldn't this be "... from a sequence"?

> + * ----------------
> + */
> +static inline void
> +validate_relation_kind(Relation r)

Shouldn't this be a bit more descriptive than just
"validate_relation_kind"? I notice this is no different from how this
is handled in index.c and table.c, but I'm not a huge fan of shadowing
names, even with static inlines functions.

> -ERROR:  "serialtest1" is not a sequence
> +ERROR:  cannot open relation "serialtest1"
> +DETAIL:  This operation is not supported for tables.

We seem to lose some details here: We can most definitely open tables.
We just can't open them while treating them as sequences, which is not
mentioned in the error message.

Kind regards,

Matthias van de Meent
Neon (https://neon.tech)



pgsql-hackers by date:

Previous
From: Peter Eisentraut
Date:
Subject: Re: backend *.c #include cleanup (IWYU)
Next
From: jian he
Date:
Subject: Re: Improve readability by using designated initializers when possible