BUG #14258: Documentation pl/pgsql

Started by Дилян Палаузовover 9 years ago6 messagesbugs
Jump to latest
#1Дилян Палаузов
dpa-postgres@aegee.org

The following bug has been logged on the website:

Bug reference: 14258
Logged by: Dilian Palauzov
Email address: dpa-postgres@aegee.org
PostgreSQL version: 9.5.3
Operating system: any
Description:

Hello,

in doc/src/sgml/plpgsql.sgml please replace:

--- a/doc/src/sgml/plpgsql.sgml
+++ b/doc/src/sgml/plpgsql.sgml
@@ -328,7 +328,7 @@ arow RECORD;
<para>
The general syntax of a variable declaration is:
<synopsis>
-<replaceable>name</replaceable> <optional> CONSTANT </optional>
<replaceable>type</replaceable> <optional> COLLATE
<replaceable>collation_name</replaceable> </optional> <optional> NOT NULL
</optional> <optional> { DEFAULT | := | = }
<replaceable>expression</replaceable> </optional>;
+<replaceable>name</replaceable> <optional> CONSTANT </optional>
<replaceable>type</replaceable> <optional> COLLATE
<replaceable>collation_name</replaceable> </optional> <optional>  <optional>
NOT NULL </optional>{ DEFAULT | := | = }
<replaceable>expression</replaceable> </optional>;
</synopsis>
The <literal>DEFAULT</> clause, if given, specifies the initial value
assigned
to the variable when the block is entered.  If the <literal>DEFAULT</>
clause

because NOT NULL can be used only with {DEFAULT | := | =}. Keep in mind
that the mentioned line is so long, that it does not fit in the PDF/A4
documentation
(https://www.postgresql.org/files/documentation/pdf/9.5/postgresql-9.5-A4.pdf,
page 1063/1138).

For GET [CURRENT] DIAGNOSITCS (plpgsql.sgml, line 1441) the documentation
within pgpgsql.sgml is unclear, what is CURRENT supposed to do.

In <sect2 id="plpgsql-conditionals">, 1834 is written

IF ... THEN

CASE ... WHEN ... END CASE

More consistent would be to write IF ... THEN ... END IF

In Trigger Procedures, Triggers on Data Changes is stated, that the return
value of a row-level trigger is ignored. Does such a trigger have to
execute explicitly RETURN, contrary to functions returning void?

In the example afterwards "A PL/pgSQL Trigger Procedure For Auditing", in
process_emp_audit() as AFTER trigger, the comment states, that the return
value is ignored, but why does the function have four return statements
instead of just one?

In subsection "Triggers on Events" please replace "is called as a event
trigger" with "is called as AN event trigger".

--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs

#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Дилян Палаузов (#1)
Re: BUG #14258: Documentation pl/pgsql

dpa-postgres@aegee.org writes:

in doc/src/sgml/plpgsql.sgml please replace:
...
because NOT NULL can be used only with {DEFAULT | := | =}.

I'm disinclined to do that; it's not required by the plpgsql grammar,
and in principle it might not be required at runtime either. In
particular, if the variable is of a domain type it would be sensible for
plpgsql to adopt the domain's default value if any.

For GET [CURRENT] DIAGNOSITCS (plpgsql.sgml, line 1441) the documentation
within pgpgsql.sgml is unclear, what is CURRENT supposed to do.

Hmm, it's just a noise word, but I agree the docs ought to say that.

In <sect2 id="plpgsql-conditionals">, 1834 is written
IF ... THEN
CASE ... WHEN ... END CASE
More consistent would be to write IF ... THEN ... END IF

Makes sense.

In Trigger Procedures, Triggers on Data Changes is stated, that the return
value of a row-level trigger is ignored. Does such a trigger have to
execute explicitly RETURN, contrary to functions returning void?

Yes, I believe so; the documentation for RETURN does not make any
exception for triggers.

In the example afterwards "A PL/pgSQL Trigger Procedure For Auditing", in
process_emp_audit() as AFTER trigger, the comment states, that the return
value is ignored, but why does the function have four return statements
instead of just one?

[ shrug... ] Seems like reasonable coding style to me; for one thing,
it would make it easier to copy-and-paste the example as a basis for a
BEFORE trigger. I probably wouldn't have written it that way myself,
but I don't feel a need to change it.

In subsection "Triggers on Events" please replace "is called as a event
trigger" with "is called as AN event trigger".

Good catch, looks like there's a couple of those...

Thanks for the notes!

regards, tom lane

--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs

#3Дилян Палаузов
dpa-postgres@aegee.org
In reply to: Tom Lane (#2)
Re: BUG #14258: Documentation pl/pgsql

Hello Tom,

in you answer you have not tackled the argument, that the line with NOT NULL {DEFAULT | := | = } is soo long, that it does fit in its current form in the PDF provided documentation: https://www.postgresql.org/files/documentation/pdf/9.5/postgresql-9.5-A4.pdf, page logical 1063(physical 1138).

On which occasions is the online documentation regenerated (in terms of https://www.postgresql.org/docs/9.5/static/, https://www.postgresql.org/docs/9.4/static/ and https://www.postgresql.org/files/documentation/pdf/9.5/postgresql-9.5-A4.pdf ) from the .sgml files?

Greetings
Dilian

On 07/18/2016 09:29 PM, Tom Lane wrote:

dpa-postgres@aegee.org writes:

in doc/src/sgml/plpgsql.sgml please replace:
...
because NOT NULL can be used only with {DEFAULT | := | =}.

I'm disinclined to do that; it's not required by the plpgsql grammar,
and in principle it might not be required at runtime either. In
particular, if the variable is of a domain type it would be sensible for
plpgsql to adopt the domain's default value if any.

For GET [CURRENT] DIAGNOSITCS (plpgsql.sgml, line 1441) the documentation
within pgpgsql.sgml is unclear, what is CURRENT supposed to do.

Hmm, it's just a noise word, but I agree the docs ought to say that.

In <sect2 id="plpgsql-conditionals">, 1834 is written
IF ... THEN
CASE ... WHEN ... END CASE
More consistent would be to write IF ... THEN ... END IF

Makes sense.

In Trigger Procedures, Triggers on Data Changes is stated, that the return
value of a row-level trigger is ignored. Does such a trigger have to
execute explicitly RETURN, contrary to functions returning void?

Yes, I believe so; the documentation for RETURN does not make any
exception for triggers.

In the example afterwards "A PL/pgSQL Trigger Procedure For Auditing", in
process_emp_audit() as AFTER trigger, the comment states, that the return
value is ignored, but why does the function have four return statements
instead of just one?

[ shrug... ] Seems like reasonable coding style to me; for one thing,
it would make it easier to copy-and-paste the example as a basis for a
BEFORE trigger. I probably wouldn't have written it that way myself,
but I don't feel a need to change it.

In subsection "Triggers on Events" please replace "is called as a event
trigger" with "is called as AN event trigger".

Good catch, looks like there's a couple of those...

Thanks for the notes!

regards, tom lane

--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs

#4David G. Johnston
david.g.johnston@gmail.com
In reply to: Дилян Палаузов (#3)
Re: BUG #14258: Documentation pl/pgsql

On Thu, Jul 21, 2016 at 5:08 PM, Дилян Палаузов <dpa-postgres@aegee.org>
wrote:

Hello Tom,

in you answer you have not tackled the argument, that the line with NOT
NULL {DEFAULT | := | = } is soo long, that it does fit in its current form
in the PDF provided documentation:
https://www.postgresql.org/files/documentation/pdf/9.5/postgresql-9.5-A4.pdf,
page logical 1063(physical 1138).

​Please don't top-post; and trim unnecessary content from the quoted text
in the process.

​The failure for the PDF file to not wrap "code/syntax" blocks is not
limited to this one instance. The page for VACUUM (logical 1681, physical
1756) exhibits the same problem. I found this by happenstance during a
cursory skim of the PDF, there is at least one more instance in one of the
program syntax definitions but I gave up trying to re-locate it.

For the pl/pgsql page I could see trying to shorten it via:
expression -> expr
collation_name -> collation

That recovers 11 of the 9 required characters.

VACUUM is a bit tougher, we need at least 14 characters and there are only
two labels

The only cosmetic way to do this is to define

<OPTION> ::= { FULL | FREEZE | VERBOSE | ANALYZE | DISABLE_PAGE_SKIPPING }

VACUUM [ ( OPTION [, ...] ) ] [ table_name [ (column_name [, ...] ) ] ]

Non-cosmetically...

VACUUM [ ( { FULL | FREEZE | VERBOSE | ANALYZE | DISABLE_PAGE_SKIPPING } [,
...] ) ] [ tbl [ (col [, ...] ) ] ]

I'm hoping there is some kind of warning output during the PDF build
process that can help locate these kinds of problems. The only way to
solve them is to do what I did above - write an alternate form that is
shorter, ideally by changing labels (i.e., <replaceable>).

I don't know whether it is considered acceptable, or structurally correct,
to define "OPTION" that way but you get the idea. There is probably a more
idiomatic way to do that since long command sequences are not uncommon in
the docs and we seem to generally handle them well. Would like to give
someone more experienced a chance to comment before I go digging any deeper.

David J.

#5Tom Lane
tgl@sss.pgh.pa.us
In reply to: David G. Johnston (#4)
Re: BUG #14258: Documentation pl/pgsql

"David G. Johnston" <david.g.johnston@gmail.com> writes:

I'm hoping there is some kind of warning output during the PDF build
process that can help locate these kinds of problems.

Yeah, look for "Overfull \hbox" in the TeX log. Unfortunately, the
warnings reference line numbers in TeX's input (the tex-pdf file,
which isn't even saved after the make is done :-(). So it's a pain
to work backwards to where the problem is in the SGML docs. But you
can if you're determined.

By my count, there are 3021 such warnings generated in current HEAD docs.
A lot of them are small enough to be negligible, but a lot are not.
There's about 450 that are for more than 72 points (an inch) of overflow.

I do not see a lot of value in attacking such problems one instance at a
time.

regards, tom lane

--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs

#6Tom Lane
tgl@sss.pgh.pa.us
In reply to: Дилян Палаузов (#3)
Re: BUG #14258: Documentation pl/pgsql

=?UTF-8?B?0JTQuNC70Y/QvSDQn9Cw0LvQsNGD0LfQvtCy?= <dpa-postgres@aegee.org> writes:

On which occasions is the online documentation regenerated (in terms of https://www.postgresql.org/docs/9.5/static/, https://www.postgresql.org/docs/9.4/static/ and https://www.postgresql.org/files/documentation/pdf/9.5/postgresql-9.5-A4.pdf ) from the .sgml files?

I believe the web team updates those pages whenever a new minor release
occurs.

regards, tom lane

--
Sent via pgsql-bugs mailing list (pgsql-bugs@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-bugs