On Sat, 2022-11-26 at 23:22 -0500, Kirk Wolak wrote:
> Your feedback would be appreciated.
> --- a/doc/src/sgml/func.sgml
> +++ b/doc/src/sgml/func.sgml
> @@ -17625,6 +17625,11 @@ $.* ? (@ like_regex "^\\d+$")
> command.
> </para>
> <para>
> +<programlisting>
> +SELECT nextval('myseq'::regclass);
> +</programlisting>
> + </para>
> + <para>
The indentation of the <para> tags should be the same everywhere.
True, the misalignment was pre-existing here, but it would be better to
fix it than to repeat the mistake.
I think the cast to "regclass" should be only in the more elaborate
example at the end
> @@ -17669,7 +17674,6 @@ SELECT setval('myseq', 42, false);
> <lineannotation>Next <function>nextval</fu
> sequence.
> </para></entry>
> </row>
> -
> <row>
> <entry role="func_table_entry"><para role="func_signature">
> <indexterm>
I think the empty line is intentional and should stay. We have it
everywhere else, and it helps to read the source.
> @@ -17707,19 +17714,75 @@ SELECT setval('myseq', 42, false);
> <lineannotation>Next <function>nextval</fu
> identical to <function>currval</function>, except that instead
> of taking the sequence name as an argument it refers to whichever
> sequence <function>nextval</function> was most recently applied to
> - in the current session. It is an error to call
> - <function>lastval</function> if <function>nextval</function>
> - has not yet been called in the current session.
> + in the current session. (An error is reported if
> <function>nextval</function> has
> + never been called in this session.)
> +<programlisting>
> +SELECT lastval();
> +</programlisting>
I have no problem with changing the sentence to a parenthetical remark, although
I thing the original wording was fine. However, there is a trailing blank at
the
end of the line (and in the <programlisting> line as well). Please avoid these.
"git diff" highlights such training blanks.
I would not add a code sample in this case. The function has no arguments, so
what
cat we really learn from that? We don't have to have a code sample here just
because
we had one with all the other functions.
> </para>
> <para>
> This function requires <literal>USAGE</literal>
> or <literal>SELECT</literal> privilege on the last used sequence.
> </para></entry>
> </row>
> + <row>
> + <entry role="func_table_entry">
> + </entry>
> + </row>
> </tbody>
> </tgroup>
> </table>
This empty row should definitely go.
> + <para>Example
I think you should use the <title> tag to render that more conspicuous.
> +<programlisting>
> +CREATE SCHEMA play; <lineannotation>-- Create a
> play schema</lineannotation>
> [...]
Now I think that is taking it too far. Your code sample would be great for a
tutorial,
but it is too elaborate for the technical documentation. The example should
focus on
the sequence functions, but more than half of the code describes other parts of
PostgreSQL:
- schema and search_path
- psql commands
- INSERT ... RETURNING
- pg_get_serial_sequence()
I am not saying that this is bad material, but if anything, it should go to
chapter 3 (Advanced Features) of the tutorial (and then it could be even more
elaborate).
I am alright with having a CREATE TABLE with a DEFAULT and an INSERT or two;
whatever it takes to show the functions in a realistic scenario.
For example, you could INSERT a row that overrides the DEFAULT, then call
"setval()" to readjust the sequence.
Yours,
Laurenz Albe
