Re: "pgoutput" options missing on documentation

From: Emre Hasegeli <emre(at)hasegeli(dot)com>
To: Peter Smith <smithpb2250(at)gmail(dot)com>
Cc: pgsql-hackers(at)postgresql(dot)org, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>
Subject: Re: "pgoutput" options missing on documentation
Date: 2023-12-14 10:14:33
Message-ID: CAE2gYzwgR3ageRd+OOtfxWsF-3jLgqVQGpiW8zdU16uVEU4LYA@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

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

Good idea. I've done so.

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

It looks like they are called "options" in most places. I changed the
documentation to be consistent too.

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

Changed.

> 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

This is not going to be correct because not all options do require a
protocol version. "origin" is added in version 16, but doesn't check
for any "proto_version". Perhaps we should fix this too.

> 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

Done.

Thanks for the review.

The new versions are attached.

Attachment Content-Type Size
v01-0001-pgoutput-Improve-error-messages-for-options.patch application/octet-stream 8.6 KB
v01-0002-pgoutput-Document-missing-options.patch application/octet-stream 6.2 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Emre Hasegeli 2023-12-14 10:15:25 Re: "pgoutput" options missing on documentation
Previous Message Richard Guo 2023-12-14 10:02:08 Re: planner chooses incremental but not the best one