Add small detail to RAISE statement descripton

Started by Igor Gnatyukalmost 2 years ago12 messagesdocs
Jump to latest
#1Igor Gnatyuk
ig953or@gmail.com

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

Attachments:

fix_doc_raise.patchtext/x-patch; charset=US-ASCII; name=fix_doc_raise.patchDownload+10-10
#2jian he
jian.universality@gmail.com
In reply to: Igor Gnatyuk (#1)
Re: Add small detail to RAISE statement descripton

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.

#3Igor Gnatyuk
ig953or@gmail.com
In reply to: jian he (#2)
Re: Add small detail to RAISE statement descripton

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>:

Show quoted text

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.

Attachments:

fix_doc_raise_v2.patchtext/x-patch; charset=US-ASCII; name=fix_doc_raise_v2.patchDownload+40-28
#4jian he
jian.universality@gmail.com
In reply to: Igor Gnatyuk (#3)
Re: Add small detail to RAISE statement descripton

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"?

#5Igor Gnatyuk
ig953or@gmail.com
In reply to: jian he (#4)
Re: Add small detail to RAISE statement descripton

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>:

Show quoted text

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"?

Attachments:

fix_doc_raise_v3.patchtext/x-patch; charset=US-ASCII; name=fix_doc_raise_v3.patchDownload+43-27
#6jian he
jian.universality@gmail.com
In reply to: Igor Gnatyuk (#5)
Re: Add small detail to RAISE statement descripton

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.

Attachments:

v4-0001-fix-plpgsql-raise-command-doc-issue.patchtext/x-patch; charset=US-ASCII; name=v4-0001-fix-plpgsql-raise-command-doc-issue.patchDownload+24-6
#7Igor Gnatyuk
ig953or@gmail.com
In reply to: jian he (#6)
Re: Add small detail to RAISE statement descripton

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>:

Show quoted text

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.

Attachments:

fix_doc_raise_v3-bis.patchtext/x-patch; charset=US-ASCII; name=fix_doc_raise_v3-bis.patchDownload+43-27
#8jian he
jian.universality@gmail.com
In reply to: Igor Gnatyuk (#7)
Re: Add small detail to RAISE statement descripton

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.

#9Laurenz Albe
laurenz.albe@cybertec.at
In reply to: Igor Gnatyuk (#7)
Re: Add small detail to RAISE statement descripton

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

#10Igor Gnatyuk
ig953or@gmail.com
In reply to: Laurenz Albe (#9)
Re: Add small detail to RAISE statement descripton

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>:

Show quoted text

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

Attachments:

fix_doc_raise_v4.patchtext/x-patch; charset=US-ASCII; name=fix_doc_raise_v4.patchDownload+43-27
#11Laurenz Albe
laurenz.albe@cybertec.at
In reply to: Igor Gnatyuk (#10)
Re: Add small detail to RAISE statement descripton

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

#12Tom Lane
tgl@sss.pgh.pa.us
In reply to: Laurenz Albe (#11)
Re: Add small detail to RAISE statement descripton

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