Re: "pgoutput" options missing on documentation

From: Peter Smith <smithpb2250(at)gmail(dot)com>
To: emre(at)hasegeli(dot)com
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: "pgoutput" options missing on documentation
Date: 2023-12-14 07:53:56
Message-ID: CAHut+Ps7JSd7QqXcU0EmFRna9W1QTaCuMme_-kGCc6Kzxqw_oQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi, here are some initial review comments.

//////

Patch v00-0001

1.
+
+ /* Check required parameters */
+ if (!protocol_version_given)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("proto_version parameter missing")));
+ if (!publication_names_given)
+ ereport(ERROR,
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("publication_names parameter missing")));

To reduce translation efforts, perhaps it is better to arrange for
these to share a common message.

For example,

ereport(ERROR,
errcode(ERRCODE_INVALID_PARAMETER_VALUE),
/* translator: %s is a pgoutput option */
errmsg("missing pgoutput option: %s", "proto_version"));

~

ereport(ERROR,
errcode(ERRCODE_INVALID_PARAMETER_VALUE),
/* translator: %s is a pgoutput option */
errmsg("missing pgoutput option: %s", "publication_names"));

Also, I am unsure whether to call these "parameters" or "options" -- I
wanted to call them parameters like in the documentation, but every
other message in this function refers to "options", so in my example,
I mimicked the nearby code YMMV.

//////

Patch v00-0002

2.
- The logical replication <literal>START_REPLICATION</literal> command
- accepts following parameters:
+ The standard logical decoding plugin (<literal>pgoutput</literal>) accepts
+ following parameters with <literal>START_REPLICATION</literal> command:

Since the previous line already said pgoutput is the standard decoding
plugin, maybe it's not necessary to repeat that.

SUGGESTION
Using the <literal>START_REPLICATION</literal> command,
<literal>pgoutput</literal>) accepts the following parameters:

~~~

3.
I noticed in the protocol message formats docs [1] that those messages
are grouped according to the protocol version that supports them.
Perhaps this page should be arranged similarly for these parameters?

For example, document the parameters in the order they were introduced.

SUGGESTION

-proto_version
...
-publication_names
...
-binary
...
-messages
...

Since protocol version 2:

-streaming (boolean)
...

Since protocol version 3:

-two_phase
...

Since protocol version 4:

-streaming (boolean/parallel)
...
-origin
...

~~~

4.
+ Boolean parameter to use the binary transfer mode. This is faster
+ than the text mode, but slightly less robust

SUGGESTION
Boolean parameter to use binary transfer mode. Binary mode is faster
than the text mode but slightly less robust

======
[1] https://www.postgresql.org/docs/current/protocol-logicalrep-message-formats.html

Kind Regards,
Peter Smith.
Fujitsu Australia

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Amit Langote 2023-12-14 08:04:46 Re: remaining sql/json patches
Previous Message Dilip Kumar 2023-12-14 07:07:07 Re: logical decoding and replication of sequences, take 2