Horton

Thread

Commits

  1. doc: Replace reference to ERRCODE_RAISE_EXCEPTION by "raise_exception"

  2. Improve the protocol message descriptions for 2PC logical replication.

  3. doc: Fix typos in protocol.sgml

  4. Fix inconsistencies and typos in the tree, take 10

  1. Typo in "43.9.1. Reporting Errors and Messages"?

    The Post Office <noreply@postgresql.org> — 2022-07-31T23:37:03Z

    The following documentation comment has been logged on the website:

Page: https://www.postgresql.org/docs/14/plpgsql-errors-and-messages.html
Description:

Towards the end of the "43.9.1. Reporting Errors and Messages" section (here
https://www.postgresql.org/docs/current/plpgsql-errors-and-messages.html#PLPGSQL-STATEMENTS-RAISE)
we have the following sentence:

> If no condition name nor SQLSTATE is specified in a RAISE EXCEPTION
command, the default is to use ERRCODE_RAISE_EXCEPTION (P0001).

Looking at the list of error codes (here
https://www.postgresql.org/docs/current/errcodes-appendix.html) I think the
"ERRCODE_RAISE_EXCEPTION (P0001)" is a typo and should remove "ERRCODE_" and
simply read "RAISE_EXCEPTION (P0001)" or perhaps "ERRCODE =
'RAISE_EXCEPTION'" since that's how the default behaviour would be written
in a RAISE statement.

Many thanks,
Eric Mutta.

  2. PostgreSQL 15 minor fixes in protocol.sgml

    Ekaterina Kiryanova <e.kiryanova@postgrespro.ru> — 2022-08-01T20:00:20Z

    Hello!

I've come across some typos in protocol.sgml for PostgreSQL 15 so please 
have a look at the attached patch.

I didn't include it in the patch but I also suggest removing single 
quotes around 'method' for the COMPRESSION option to help avoid 
confusion. (All the supported compression methods consist of a single 
word so in my opinion there is no need to use quotes in this case.)
-- <term><literal>COMPRESSION</literal> 
<replaceable>'method'</replaceable></term>

I've also noticed that there are two ways to describe an option: "If set 
to true" / "If true". As far as I know, the option here is specified by 
its name rather than being explicitly set to true so "if true" seems to 
be more correct, and this could be a slight improvement for this page. 
Please correct me if I'm wrong.

Another point worth mentioning is that only this file contains the 
phrase "two-phase transaction". I believe that "two-phase commit 
transaction" or "transaction prepared for two-phase commit" depending on 
the situation would be better wording.

And finally, could you please clarify this part?
-- The end LSN of the prepare transaction.
Is it a typo of "prepared transaction"? Or is it the LSN of the 
transaction for Prepare?
If it's the latter, perhaps it'd make more sense to capitalize it.

-- 
Best regards,
Ekaterina Kiryanova
Technical Writer
Postgres Professional
the Russian PostgreSQL Company

  3. Re: PostgreSQL 15 minor fixes in protocol.sgml

    Michael Paquier <michael@paquier.xyz> — 2022-08-02T10:58:10Z

    On Mon, Aug 01, 2022 at 11:00:20PM +0300, Ekaterina Kiryanova wrote:
> I didn't include it in the patch but I also suggest removing single quotes
> around 'method' for the COMPRESSION option to help avoid confusion. (All the
> supported compression methods consist of a single word so in my opinion
> there is no need to use quotes in this case.)
> -- <term><literal>COMPRESSION</literal>
> <replaceable>'method'</replaceable></term>

Other options use quotes as well in their description in this area.

> I've also noticed that there are two ways to describe an option: "If set to
> true" / "If true". As far as I know, the option here is specified by its
> name rather than being explicitly set to true so "if true" seems to be more
> correct, and this could be a slight improvement for this page. Please
> correct me if I'm wrong.

Both sound pretty much the same to me.

> Another point worth mentioning is that only this file contains the phrase
> "two-phase transaction". I believe that "two-phase commit transaction" or
> "transaction prepared for two-phase commit" depending on the situation would
> be better wording.

"Prepare for two-phase commit" may be clearer?

> And finally, could you please clarify this part?
> -- The end LSN of the prepare transaction.
> Is it a typo of "prepared transaction"? Or is it the LSN of the transaction
> for Prepare?
> If it's the latter, perhaps it'd make more sense to capitalize it.

Hmm.  The internals of 63cf61c refer to a "STREAM PREPARE", still the
protocol docs are quite messy ("prepare", "prepare timestamp", etc.)
so more consistency would be appropriate, it seems.  Amit?

The part for the protocol messages with 2PC and logical replication
could use a larger rework.  I have left these for now, and fixed the
rest of the typos you have found.
--
Michael

  4. Re: Typo in "43.9.1. Reporting Errors and Messages"?

    Euler Taveira <euler@eulerto.com> — 2022-08-02T12:49:47Z

    On Sun, Jul 31, 2022, at 8:37 PM, PG Doc comments form wrote:
> Towards the end of the "43.9.1. Reporting Errors and Messages" section (here
> https://www.postgresql.org/docs/current/plpgsql-errors-and-messages.html#PLPGSQL-STATEMENTS-RAISE)
> we have the following sentence:
> 
> > If no condition name nor SQLSTATE is specified in a RAISE EXCEPTION
> command, the default is to use ERRCODE_RAISE_EXCEPTION (P0001).
> 
> Looking at the list of error codes (here
> https://www.postgresql.org/docs/current/errcodes-appendix.html) I think the
> "ERRCODE_RAISE_EXCEPTION (P0001)" is a typo and should remove "ERRCODE_" and
> simply read "RAISE_EXCEPTION (P0001)" or perhaps "ERRCODE =
> 'RAISE_EXCEPTION'" since that's how the default behaviour would be written
> in a RAISE statement.
It is referring to the internal constant (see src/backend/utils/errcodes.h). It
was like you are proposing and it was changed in
66bde49d96a9ddacc49dcbdf1b47b5bd6e31ead5. Reading the original thread, there is
no explanation why it was changed. Refer to internal names is not good for a
user-oriented text. I think it would be better to use the condition name (in
lowercase) like it is referred to in [1]. I mean, change
ERRCODE_RAISE_EXCEPTION to raise_exception.

[1] https://www.postgresql.org/docs/current/errcodes-appendix.html


--
Euler Taveira
EDB   https://www.enterprisedb.com/

  5. Re: PostgreSQL 15 minor fixes in protocol.sgml

    Amit Kapila <amit.kapila16@gmail.com> — 2022-08-03T03:57:58Z

    On Tue, Aug 2, 2022 at 4:28 PM Michael Paquier <michael@paquier.xyz> wrote:
>
> On Mon, Aug 01, 2022 at 11:00:20PM +0300, Ekaterina Kiryanova wrote:
>
> > Another point worth mentioning is that only this file contains the phrase
> > "two-phase transaction". I believe that "two-phase commit transaction" or
> > "transaction prepared for two-phase commit" depending on the situation would
> > be better wording.
>
> "Prepare for two-phase commit" may be clearer?
>

I think we can use just "Prepared transaction" instead. So, the
message "The user defined GID of the two-phase transaction." can be
changed to "The user defined GID of the prepared transaction.".
Similarly, the message "Identifies the message as a two-phase prepared
transaction message." could be changed to: "Identifies the message as
a prepared transaction message."

> > And finally, could you please clarify this part?
> > -- The end LSN of the prepare transaction.
> > Is it a typo of "prepared transaction"?

I think in this case it should be a "prepared transaction".


Thanks for the report and Thanks Michael for including me. I am just
redirecting it to -hackers so that others involved in this feature
also can share their views.

-- 
With Regards,
Amit Kapila.

  6. Re: PostgreSQL 15 minor fixes in protocol.sgml

    Peter Smith <smithpb2250@gmail.com> — 2022-08-03T05:26:25Z

    On Wed, Aug 3, 2022 at 1:58 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Tue, Aug 2, 2022 at 4:28 PM Michael Paquier <michael@paquier.xyz> wrote:
> >
> > On Mon, Aug 01, 2022 at 11:00:20PM +0300, Ekaterina Kiryanova wrote:
> >
> > > Another point worth mentioning is that only this file contains the phrase
> > > "two-phase transaction". I believe that "two-phase commit transaction" or
> > > "transaction prepared for two-phase commit" depending on the situation would
> > > be better wording.
> >
> > "Prepare for two-phase commit" may be clearer?
> >
>
> I think we can use just "Prepared transaction" instead. So, the
> message "The user defined GID of the two-phase transaction." can be
> changed to "The user defined GID of the prepared transaction.".
> Similarly, the message "Identifies the message as a two-phase prepared
> transaction message." could be changed to: "Identifies the message as
> a prepared transaction message."
>
> > > And finally, could you please clarify this part?
> > > -- The end LSN of the prepare transaction.
> > > Is it a typo of "prepared transaction"?
>
> I think in this case it should be a "prepared transaction".
>
>
> Thanks for the report and Thanks Michael for including me. I am just
> redirecting it to -hackers so that others involved in this feature
> also can share their views.
>

PSA a patch to modify the descriptions as suggested by Amit.

------
Kind Regards,
Peter Smith.
Fujitsu Australia

  7. Re: PostgreSQL 15 minor fixes in protocol.sgml

    Alvaro Herrera <alvherre@alvh.no-ip.org> — 2022-08-03T10:53:15Z

    On 2022-Aug-03, Amit Kapila wrote:

> Thanks for the report and Thanks Michael for including me. I am just
> redirecting it to -hackers so that others involved in this feature
> also can share their views.

I'm sorry, but our policy is that crossposts are not allowed.  I think
this policy is bad, precisely because it prevents legitimate cases like
this one; but it is what it is.

I think we should change the policy, not back to allow indiscriminate
cross-posting, but to allow some limited form of it.  For example I
think pg-bugs+pg-hackers and pg-docs+pg-hackers should be allowed
combinations.  Just saying.

-- 
Álvaro Herrera        Breisgau, Deutschland  —  https://www.EnterpriseDB.com/

  8. Re: PostgreSQL 15 minor fixes in protocol.sgml

    Amit Kapila <amit.kapila16@gmail.com> — 2022-08-04T03:04:47Z

    On Wed, Aug 3, 2022 at 10:56 AM Peter Smith <smithpb2250@gmail.com> wrote:
>
> PSA a patch to modify the descriptions as suggested by Amit.
>

*
<para>
-         The end LSN of the commit prepared transaction.
+         The end LSN of the commit of the prepared transaction.
...
...
-         Identifies the message as the commit of a two-phase
transaction message.
+         Identifies the message as the commit of a prepared
transaction message.

In the above messages, we can even directly say "commit prepared
transaction" but as you have written appears clear to me.

*
For timestamp, related messages, we have three different messages:
Commit timestamp of the transaction. The value is in number of
microseconds since PostgreSQL epoch (2000-01-01).
Prepare timestamp of the transaction. The value is in number of
microseconds since PostgreSQL epoch (2000-01-01).
Rollback timestamp of the transaction. The value is in number of
microseconds since PostgreSQL epoch (2000-01-01).

We can improve by saying "Timestamp of prepared transaction" for the
second one but it will make it bit inconsistent with others, so not
sure if changing it makes sense or if there is a better way to change
all the three messages.

Thoughts?


-- 
With Regards,
Amit Kapila.

  9. Re: PostgreSQL 15 minor fixes in protocol.sgml

    Amit Kapila <amit.kapila16@gmail.com> — 2022-08-04T03:06:51Z

    On Wed, Aug 3, 2022 at 4:23 PM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
>
> On 2022-Aug-03, Amit Kapila wrote:
>
> > Thanks for the report and Thanks Michael for including me. I am just
> > redirecting it to -hackers so that others involved in this feature
> > also can share their views.
>
> I'm sorry, but our policy is that crossposts are not allowed.  I think
> this policy is bad, precisely because it prevents legitimate cases like
> this one; but it is what it is.
>
> I think we should change the policy, not back to allow indiscriminate
> cross-posting, but to allow some limited form of it.  For example I
> think pg-bugs+pg-hackers and pg-docs+pg-hackers should be allowed
> combinations.  Just saying.
>

+1.

-- 
With Regards,
Amit Kapila.

  10. Re: PostgreSQL 15 minor fixes in protocol.sgml

    Peter Smith <smithpb2250@gmail.com> — 2022-08-09T01:05:24Z

    On Thu, Aug 4, 2022 at 1:04 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Wed, Aug 3, 2022 at 10:56 AM Peter Smith <smithpb2250@gmail.com> wrote:
> >
> > PSA a patch to modify the descriptions as suggested by Amit.
> >
>
> *
> <para>
> -         The end LSN of the commit prepared transaction.
> +         The end LSN of the commit of the prepared transaction.
> ...
> ...
> -         Identifies the message as the commit of a two-phase
> transaction message.
> +         Identifies the message as the commit of a prepared
> transaction message.
>
> In the above messages, we can even directly say "commit prepared
> transaction" but as you have written appears clear to me.
>
> *
> For timestamp, related messages, we have three different messages:
> Commit timestamp of the transaction. The value is in number of
> microseconds since PostgreSQL epoch (2000-01-01).
> Prepare timestamp of the transaction. The value is in number of
> microseconds since PostgreSQL epoch (2000-01-01).
> Rollback timestamp of the transaction. The value is in number of
> microseconds since PostgreSQL epoch (2000-01-01).
>
> We can improve by saying "Timestamp of prepared transaction" for the
> second one but it will make it bit inconsistent with others, so not
> sure if changing it makes sense or if there is a better way to change
> all the three messages.
>
> Thoughts?
>

There was no feedback for Amit's previous post [1], so I am just
attaching the same [2] patch again, but this time for both HEAD and
REL_15_STABLE.

------
[1] https://www.postgresql.org/message-id/CAA4eK1LHSDb3KVRZZnYeBF0-SodMKYP%3DV%2B2VmrVBvRNK%3Dej1Tw%40mail.gmail.com
[2] https://www.postgresql.org/message-id/CAHut%2BPs8TLKFL0P4ghgERdTcDeB4y61zWm128524h88BhnYmfA%40mail.gmail.com

Kind Regards,
Peter Smith.
Fujitsu Australia

  11. Re: Typo in "43.9.1. Reporting Errors and Messages"?

    Bruce Momjian <bruce@momjian.us> — 2023-10-31T14:52:12Z

    On Tue, Aug  2, 2022 at 09:49:47AM -0300, Euler Taveira wrote:
> On Sun, Jul 31, 2022, at 8:37 PM, PG Doc comments form wrote:
> 
>     Towards the end of the "43.9.1. Reporting Errors and Messages" section
>     (here
>     https://www.postgresql.org/docs/current/plpgsql-errors-and-messages.html#
>     PLPGSQL-STATEMENTS-RAISE)
>     we have the following sentence:
> 
>     > If no condition name nor SQLSTATE is specified in a RAISE EXCEPTION
>     command, the default is to use ERRCODE_RAISE_EXCEPTION (P0001).
> 
>     Looking at the list of error codes (here
>     https://www.postgresql.org/docs/current/errcodes-appendix.html) I think the
>     "ERRCODE_RAISE_EXCEPTION (P0001)" is a typo and should remove "ERRCODE_"
>     and
>     simply read "RAISE_EXCEPTION (P0001)" or perhaps "ERRCODE =
>     'RAISE_EXCEPTION'" since that's how the default behaviour would be written
>     in a RAISE statement.
> 
> It is referring to the internal constant (see src/backend/utils/errcodes.h). It
> was like you are proposing and it was changed in
> 66bde49d96a9ddacc49dcbdf1b47b5bd6e31ead5. Reading the original thread, there is
> no explanation why it was changed. Refer to internal names is not good for a
> user-oriented text. I think it would be better to use the condition name (in
> lowercase) like it is referred to in [1]. I mean, change
> ERRCODE_RAISE_EXCEPTION to raise_exception.
> 
> [1] https://www.postgresql.org/docs/current/errcodes-appendix.html

Alexander, Michael, can you explain why this commit removed ERRCODE_:

	commit 66bde49d96
	Author: Michael Paquier <michael@paquier.xyz>
	Date:   Tue Aug 13 13:53:41 2019 +0900
	
	    Fix inconsistencies and typos in the tree, take 10
	
	    This addresses some issues with unnecessary code comments, fixes various
	    typos in docs and comments, and removes some orphaned structures and
	    definitions.
	
	    Author: Alexander Lakhin
	    Discussion: https://postgr.es/m/9aabc775-5494-b372-8bcb-4dfc0bd37c68@gmail.com


-- 
  Bruce Momjian  <bruce@momjian.us>        https://momjian.us
  EDB                                      https://enterprisedb.com

  Only you can decide what is important to you.

  12. Re: Typo in "43.9.1. Reporting Errors and Messages"?

    Alexander Law <exclusion@gmail.com> — 2023-10-31T18:00:00Z

    Hi Bruce,

31.10.2023 17:52, Bruce Momjian wrote:
>
>> It is referring to the internal constant (see src/backend/utils/errcodes.h). It
>> was like you are proposing and it was changed in
>> 66bde49d96a9ddacc49dcbdf1b47b5bd6e31ead5. Reading the original thread, there is
>> no explanation why it was changed. Refer to internal names is not good for a
>> user-oriented text. I think it would be better to use the condition name (in
>> lowercase) like it is referred to in [1]. I mean, change
>> ERRCODE_RAISE_EXCEPTION to raise_exception.
>>
>> [1] https://www.postgresql.org/docs/current/errcodes-appendix.html
> Alexander, Michael, can you explain why this commit removed ERRCODE_:
>
> 	commit 66bde49d96

I don't remember details, but I think the primary reason for the change
was that "RAISE_EXCEPTION" occurred in the whole tree only once (before
66bde49d96). Now I see, that I had chosen the wrong replacement — I agree
with Euler, change to "raise_exception" would be more appropriate.

(I've found a similar mention of ERRCODE_xxx in btree.sgml:
   Before doing so, the function should check the sign
   of <replaceable>offset</replaceable>: if it is less than zero, raise
   error <literal>ERRCODE_INVALID_PRECEDING_OR_FOLLOWING_SIZE</literal> (22013)
   with error text like <quote>invalid preceding or following size in window
   function</quote>.
but I think that's okay here, because that identifier supposed to be used
as-is in ereport/elog.)

Best regards,
Alexander

  13. Re: Typo in "43.9.1. Reporting Errors and Messages"?

    Michael Paquier <michael@paquier.xyz> — 2023-11-01T00:18:47Z

    On Tue, Oct 31, 2023 at 09:00:00PM +0300, Alexander Lakhin wrote:
> I don't remember details, but I think the primary reason for the change
> was that "RAISE_EXCEPTION" occurred in the whole tree only once (before
> 66bde49d96). Now I see, that I had chosen the wrong replacement — I agree
> with Euler, change to "raise_exception" would be more appropriate.

Indeed, it looks like the origin of the confusion is the casing here,
so changing to "raise_exception" like in the appendix sounds good to
me:
https://www.postgresql.org/docs/devel/errcodes-appendix.html

So you mean something like the attached then?

> (I've found a similar mention of ERRCODE_xxx in btree.sgml:
>   Before doing so, the function should check the sign
>   of <replaceable>offset</replaceable>: if it is less than zero, raise
>   error <literal>ERRCODE_INVALID_PRECEDING_OR_FOLLOWING_SIZE</literal> (22013)
>   with error text like <quote>invalid preceding or following size in window
>   function</quote>.
> but I think that's okay here, because that identifier supposed to be used
> as-is in ereport/elog.)

Yep, still this one is not that old (0a459cec96d3).
--
Michael

  14. Re: Typo in "43.9.1. Reporting Errors and Messages"?

    Michael Paquier <michael@paquier.xyz> — 2023-11-01T22:35:22Z

    On Wed, Nov 01, 2023 at 09:18:47AM +0900, Michael Paquier wrote:
> So you mean something like the attached then?

Fixed that with f8b96c211da0 down to 11, in time for next week's
release set.
--
Michael