Re: nextval parameter is not clear

On Thu, Nov 24, 2022 at 7:33 AM Kirk Wolak <wolakk(at)gmail(dot)com> wrote:

> On Thu, Nov 24, 2022 at 5:33 AM Laurenz Albe <laurenz(dot)albe(at)cybertec(dot)at>
> wrote:
>
>> On Thu, 2022-11-24 at 02:41 -0500, Kirk Wolak wrote:
>> > Alright, as I have the documentation build working, and a slightly
>> better stylesheet,
>> > the comments on the last block were not aligned.
>> > They are fixed now.
>> >
>> > Apologies for spamming this in... My first patch turned into 3
>> emails...
>>
>> That is no problem.
>>
>> I think the patch is fine. I like the addition of the "--" comments.
>> The one think that should be unified is that you include an explicit
>> cast to "regclass" everywhere, but the pre-existing example does not.
>>
>> I would omit that cast. It might confuse beginners (who are probably
>> satisfied to know that you have to use the sequence name as a string),
>> and it is explained in the last paragraph of the page anyway.
>>
>> Yours,
>> Laurenz Albe
>>
>
> Laurenz,
> First, thanks for the support..
>
> It's funny you mention that (with and without ::regclass). I went both
> ways on it.
> The reason I did not address it, was that I liked the variety because it
> sticks out.
> I thought about adding a comment like
> -- while you can omit the ::regclass casting, it is preferred
>
> The fact that it is different engages the brain of the reader whose
> pattern matching
> just got challenged. It almost begs them to test it out.
>
> I think it might deserve a comment, but I would really prefer to keep it
> in there.
>
>
I dislike having CREATE SEQUENCE in the table examples. Keep those focused
on only the syntax of the call expression. That requires removing the
nextval calls too and not showing the error path. I'd be fine with not
showing examples of code that would produce an error. (if we are going to
show the error path I'd suggest we show the actual error message produced)

At the bottom of the page I would add an extended example of how these
sequences can be used in practice.

CREATE TABLE seq_example ( id bigint PRIMARY KEY GENERATED BY DEFAULT AS
IDENTITY ) -- implicit sequence named seq_example_seq is created (or
whatever is generated...)
INSERT INTO seq_example RETURNING id; -- 1
SELECT currval('seq_example_seq'::regclass); -- 1
SELECT setval('seq_example_seq', 10);
INSERT INTO seq_example (id)
VALUES (nextval('seq_example_seq')) -- works as-is due to choosing BY
DEFAULT for the identity
RETURNING id; -- 11
SELECT lastval(); -- 11

I would remove the casting of the sequence name from the table examples and
just keep one in the main usage example. I agree that having both to make
the reader stop and think is a good idea.

David J.