Re: Sequence Access Methods, round two

On Mon, 26 Feb 2024 at 09:11, Michael Paquier <michael(at)paquier(dot)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)