Обсуждение: Add small detail to RAISE statement descripton

Hi,

There is a slight syntax inaccuracy in the description of the RAISE
statement - assignments of parameter values in USING.
(Chapter 43. PL/pgSQL — SQL Procedural Language / 43.9 Errors and
Messages / 43.9.1 Reporting Errors and Messages):

"...You can attach additional information to the error report by
writing USING followed by option = expression items..."
It should, apparently, be like this: "...option { = | := } expression..."

The patch corrects this little omission. Attached: fix_doc_raise.patch

Regards, Igor Gnatyuk

On Tue, May 14, 2024 at 11:09 PM Igor Gnatyuk <ig953or@gmail.com> wrote:
>
> Hi,
>
> There is a slight syntax inaccuracy in the description of the RAISE
> statement - assignments of parameter values in USING.
> (Chapter 43. PL/pgSQL — SQL Procedural Language / 43.9 Errors and
> Messages / 43.9.1 Reporting Errors and Messages):
>
> "...You can attach additional information to the error report by
> writing USING followed by option = expression items..."
> It should, apparently, be like this: "...option { = | := } expression..."
>
> The patch corrects this little omission. Attached: fix_doc_raise.patch
>
> Regards, Igor Gnatyuk


while at it,
I found out there is no brief explanation of:
<replaceable class="parameter">condition_name</replaceable>
and
<replaceable class="parameter">sqlstate</replaceable>
should we add it?

one more minor thing.

    You can attach additional information to the error report by writing
    <literal>USING</literal> followed by <replaceable
    class="parameter">option</replaceable> { = | := } <replaceable
    class="parameter">expression</replaceable> items.  Each

we are not in <synopsis>, maybe

    You can attach additional information to the error report by writing
    <literal>USING</literal> followed by <replaceable
    class="parameter">option</replaceable> = <replaceable
    class="parameter">expression</replaceable> or
     <replaceable class="parameter">option</replaceable> :=
<replaceable class="parameter">expression</replaceable>
    items

will make it more clear.

Hi.

Thank you for your letter. I tried to take your comments into account:
I added explanations to the condition_name, sqlstate options and
slightly changed
the explanations for USING. Attached you will find new patch version.
Thank you for your help.

Ragards, Igor Gnatyuk


ср, 15 мая 2024 г. в 12:18, jian he <jian.universality@gmail.com>:
>
> On Tue, May 14, 2024 at 11:09 PM Igor Gnatyuk <ig953or@gmail.com> wrote:
> >
> > Hi,
> >
> > There is a slight syntax inaccuracy in the description of the RAISE
> > statement - assignments of parameter values in USING.
> > (Chapter 43. PL/pgSQL — SQL Procedural Language / 43.9 Errors and
> > Messages / 43.9.1 Reporting Errors and Messages):
> >
> > "...You can attach additional information to the error report by
> > writing USING followed by option = expression items..."
> > It should, apparently, be like this: "...option { = | := } expression..."
> >
> > The patch corrects this little omission. Attached: fix_doc_raise.patch
> >
> > Regards, Igor Gnatyuk
>
>
> while at it,
> I found out there is no brief explanation of:
> <replaceable class="parameter">condition_name</replaceable>
> and
> <replaceable class="parameter">sqlstate</replaceable>
> should we add it?
>
> one more minor thing.
>
>     You can attach additional information to the error report by writing
>     <literal>USING</literal> followed by <replaceable
>     class="parameter">option</replaceable> { = | := } <replaceable
>     class="parameter">expression</replaceable> items.  Each
>
> we are not in <synopsis>, maybe
>
>     You can attach additional information to the error report by writing
>     <literal>USING</literal> followed by <replaceable
>     class="parameter">option</replaceable> = <replaceable
>     class="parameter">expression</replaceable> or
>      <replaceable class="parameter">option</replaceable> :=
> <replaceable class="parameter">expression</replaceable>
>     items
>
> will make it more clear.

On Fri, May 17, 2024 at 4:39 PM Igor Gnatyuk <ig953or@gmail.com> wrote:
>
> Hi.
>
> Thank you for your letter. I tried to take your comments into account:
> I added explanations to the condition_name, sqlstate options and
> slightly changed
> the explanations for USING. Attached you will find new patch version.
> Thank you for your help.
>
> Ragards, Igor Gnatyuk

 there occurrence of sqlcode:
+     <replaceable class="parameter">sqlcode</replaceable>
should be:
     <replaceable class="parameter">sqlstate</replaceable>


+    <para>
+     In the syntax of <command>RAISE</command> command with the
+     <replaceable class="parameter">condition_name</replaceable> or
+     <replaceable class="parameter">sqlcode</replaceable> options
+     <literal>USING</literal> clause can be used to supply a custom
error message,
+     detail, or hint. A variation of the example above:
+ <programlisting>
+ RAISE unique_violation USING MESSAGE = 'Duplicate user ID: ' || user_id;
+ </programlisting>
+    </para>

>>  options
>>     <literal>USING</literal> clause can be used to supply a custom error message,
>>     detail, or hint.

"options" should be "optional"?
I am not sure we need to explicitly say, "error message, error detail,
error hint"?

Hi.

Thanks for the comments. I fixed my mistakes by specifying sqlcode
instead of sqlstate and changed the phrase
with an explicit listing of options in USING. In addition, I moved the
paragraph with it below.
Please check out the new patch version in the attachment.

Regards, Igor Gnatyuk

вс, 19 мая 2024 г. в 11:43, jian he <jian.universality@gmail.com>:
>
> On Fri, May 17, 2024 at 4:39 PM Igor Gnatyuk <ig953or@gmail.com> wrote:
> >
> > Hi.
> >
> > Thank you for your letter. I tried to take your comments into account:
> > I added explanations to the condition_name, sqlstate options and
> > slightly changed
> > the explanations for USING. Attached you will find new patch version.
> > Thank you for your help.
> >
> > Ragards, Igor Gnatyuk
>
>  there occurrence of sqlcode:
> +     <replaceable class="parameter">sqlcode</replaceable>
> should be:
>      <replaceable class="parameter">sqlstate</replaceable>
>
>
> +    <para>
> +     In the syntax of <command>RAISE</command> command with the
> +     <replaceable class="parameter">condition_name</replaceable> or
> +     <replaceable class="parameter">sqlcode</replaceable> options
> +     <literal>USING</literal> clause can be used to supply a custom
> error message,
> +     detail, or hint. A variation of the example above:
> + <programlisting>
> + RAISE unique_violation USING MESSAGE = 'Duplicate user ID: ' || user_id;
> + </programlisting>
> +    </para>
>
> >>  options
> >>     <literal>USING</literal> clause can be used to supply a custom error message,
> >>     detail, or hint.
>
> "options" should be "optional"?
> I am not sure we need to explicitly say, "error message, error detail,
> error hint"?

On Wed, May 22, 2024 at 4:18 PM Igor Gnatyuk <ig953or@gmail.com> wrote:
>
> Hi.
>
> Thanks for the comments. I fixed my mistakes by specifying sqlcode
> instead of sqlstate and changed the phrase
> with an explicit listing of options in USING. In addition, I moved the
> paragraph with it below.
> Please check out the new patch version in the attachment.
>

after git apply

jian@jian:~/Desktop/pg_src/src7/postgres$ git apply
$PATCHES/fix_doc_raise_v3.patch
jian@jian:~/Desktop/pg_src/src7/postgres$ git diff
diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml
old mode 100644
new mode 100755

-----------------
i guess, that means your patch (fix_doc_raise_v3) has some structural problem.

so I wrote it based on fix_doc_raise_v2.patch.
please check attached.


Feel free to change it.

Hi,

thanks for the response and for your corrections to the patch.
But I checked my changes by running make html, make postgres-A4.pdf,
make postgres-US.pdf and viewing
the results: everything is OK.

Message
'old mode 100644
new mode 100755'
this means, imho, that access rights were changed during changes to
the file. I fixed it in the patch
fix_doc_raise_v3-bios.patch. Please check it out.

Thanks again for your comments.

ср, 22 мая 2024 г. в 14:13, jian he <jian.universality@gmail.com>:
>
> On Wed, May 22, 2024 at 4:18 PM Igor Gnatyuk <ig953or@gmail.com> wrote:
> >
> > Hi.
> >
> > Thanks for the comments. I fixed my mistakes by specifying sqlcode
> > instead of sqlstate and changed the phrase
> > with an explicit listing of options in USING. In addition, I moved the
> > paragraph with it below.
> > Please check out the new patch version in the attachment.
> >
>
> after git apply
>
> jian@jian:~/Desktop/pg_src/src7/postgres$ git apply
> $PATCHES/fix_doc_raise_v3.patch
> jian@jian:~/Desktop/pg_src/src7/postgres$ git diff
> diff --git a/doc/src/sgml/plpgsql.sgml b/doc/src/sgml/plpgsql.sgml
> old mode 100644
> new mode 100755
>
> -----------------
> i guess, that means your patch (fix_doc_raise_v3) has some structural problem.
>
> so I wrote it based on fix_doc_raise_v2.patch.
> please check attached.
>
>
> Feel free to change it.

On Wed, May 22, 2024 at 11:35 PM Igor Gnatyuk <ig953or@gmail.com> wrote:
>
> Hi,
>
> thanks for the response and for your corrections to the patch.
> But I checked my changes by running make html, make postgres-A4.pdf,
> make postgres-US.pdf and viewing
> the results: everything is OK.
>
> Message
> 'old mode 100644
> new mode 100755'
> this means, imho, that access rights were changed during changes to
> the file. I fixed it in the patch
> fix_doc_raise_v3-bios.patch. Please check it out.
>

looks good to me.

On Wed, 2024-05-22 at 18:34 +0300, Igor Gnatyuk wrote:
> fix_doc_raise_v3-bios.patch. Please check it out.

I think the patch is fine.

> +   <para>
> +    <replaceable class="parameter">condition_name</replaceable> and
> +    <replaceable class="parameter">sqlstate</replaceable> specify
> +    error condition name and the five-character SQLSTATE code respectively.
> +    See <xref linkend="errcodes-appendix"/> for more information.
> +   </para>

There should be a comma before "respectively".

>      You can attach additional information to the error report by writing
>      <literal>USING</literal> followed by <replaceable
>      class="parameter">option</replaceable> = <replaceable
> -    class="parameter">expression</replaceable> items.  Each
> +    class="parameter">expression</replaceable> or
> +    <replaceable class="parameter">option</replaceable> :=
> +    <replaceable class="parameter">expression</replaceable>
> +    items, where
>      <replaceable class="parameter">expression</replaceable> can be any
>      string-valued expression.

I think that is unnecessarily verbose.  The original wording was fine;
everybody can see from the syntax diagram that you can also use :=

But I won't fight over it.

> +    In the <command>RAISE</command> command syntax with
> +    <replaceable class="parameter">condition_name</replaceable> or
> +    <replaceable class="parameter">sqlstate</replaceable> you can
> +    additionally use the <literal>USING</literal> clause too.
> +    A variation of the example above:

I think that the final sentence should be more complete.
Suggestions:

  Here is a variation of the above example:

  A variation of the above example is:

Yours,
Laurenz Albe

Hi, thanks for the review.
I tried to take your comments into account:

- added a missing comma
- still left a verbose version ;)
- made the last sentence more complete (according to the 2nd option).

Attached you will find new patch version fix_doc_raise_v4.patch,
please check it out.
Thanks again for review and comments.

Regards, Igor Gnatyuk

ср, 10 июл. 2024 г. в 23:36, Laurenz Albe <laurenz.albe@cybertec.at>:
>
> On Wed, 2024-05-22 at 18:34 +0300, Igor Gnatyuk wrote:
> > fix_doc_raise_v3-bios.patch. Please check it out.
>
> I think the patch is fine.
>
> > +   <para>
> > +    <replaceable class="parameter">condition_name</replaceable> and
> > +    <replaceable class="parameter">sqlstate</replaceable> specify
> > +    error condition name and the five-character SQLSTATE code respectively.
> > +    See <xref linkend="errcodes-appendix"/> for more information.
> > +   </para>
>
> There should be a comma before "respectively".
>
> >      You can attach additional information to the error report by writing
> >      <literal>USING</literal> followed by <replaceable
> >      class="parameter">option</replaceable> = <replaceable
> > -    class="parameter">expression</replaceable> items.  Each
> > +    class="parameter">expression</replaceable> or
> > +    <replaceable class="parameter">option</replaceable> :=
> > +    <replaceable class="parameter">expression</replaceable>
> > +    items, where
> >      <replaceable class="parameter">expression</replaceable> can be any
> >      string-valued expression.
>
> I think that is unnecessarily verbose.  The original wording was fine;
> everybody can see from the syntax diagram that you can also use :=
>
> But I won't fight over it.
>
> > +    In the <command>RAISE</command> command syntax with
> > +    <replaceable class="parameter">condition_name</replaceable> or
> > +    <replaceable class="parameter">sqlstate</replaceable> you can
> > +    additionally use the <literal>USING</literal> clause too.
> > +    A variation of the example above:
>
> I think that the final sentence should be more complete.
> Suggestions:
>
>   Here is a variation of the above example:
>
>   A variation of the above example is:
>
> Yours,
> Laurenz Albe

On Mon, 2024-07-15 at 21:30 +0300, Igor Gnatyuk wrote:
> I tried to take your comments into account:
>
> - added a missing comma
> - still left a verbose version ;)
> - made the last sentence more complete (according to the 2nd option).
>
> Attached you will find new patch version fix_doc_raise_v4.patch,
> please check it out.

Thanks.  I have marked the patch as "ready for committer".

Yours,
Laurenz Albe

Laurenz Albe <laurenz.albe@cybertec.at> writes:
> On Mon, 2024-07-15 at 21:30 +0300, Igor Gnatyuk wrote:
>> Attached you will find new patch version fix_doc_raise_v4.patch,
>> please check it out.

> Thanks.  I have marked the patch as "ready for committer".

I fooled with the wording a bit more and pushed it.
Thanks for the submission!

            regards, tom lane