Re: Minor documentation error regarding streaming replication protocol

2020-12-18 Thread Kyotaro Horiguchi
At Fri, 18 Dec 2020 10:18:45 +0900, Michael Paquier wrote in > On Thu, Dec 17, 2020 at 03:13:25PM -0800, Jeff Davis wrote: > > On second thought, I'm going to retract this patch. The docs[1] say: > > > > "You can, if you like, add comments to a history file to record your > > own notes about

Re: Minor documentation error regarding streaming replication protocol

2020-12-17 Thread Michael Paquier
On Thu, Dec 17, 2020 at 03:13:25PM -0800, Jeff Davis wrote: > On second thought, I'm going to retract this patch. The docs[1] say: > > "You can, if you like, add comments to a history file to record your > own notes about how and why this particular timeline was created. Such > comments will be

Re: Minor documentation error regarding streaming replication protocol

2020-12-17 Thread Jeff Davis
On Tue, 2020-12-15 at 12:54 -0800, Andres Freund wrote: > Hi, > > On 2020-12-15 01:22:44 -0800, Jeff Davis wrote: > > Attached a simple patch that enforces an all-ASCII restore point > > name > > rather than documenting the possibility of non-ASCII text. > > +1 On second thought, I'm going to

Re: Minor documentation error regarding streaming replication protocol

2020-12-16 Thread Brar Piening
Jeff Davis wrote: Attached a simple patch that enforces an all-ASCII restore point name rather than documenting the possibility of non-ASCII text. +1 This is probably the best solution because it gives guarantees to the client without causing compatibility issues with old clients. Thanks!

Re: Minor documentation error regarding streaming replication protocol

2020-12-15 Thread Michael Paquier
On Tue, Dec 15, 2020 at 12:54:51PM -0800, Andres Freund wrote: > Minor nit: I'd put this into common/string.[ch], besides > pg_clean_ascii(). Probably renaming it to pg_is_ascii(). Yeah. There is already one pg_is_ascii_string() in saslprep.c, is_all_ascii() in collationcmds.c and

Re: Minor documentation error regarding streaming replication protocol

2020-12-15 Thread Bruce Momjian
On Tue, Dec 15, 2020 at 01:22:44AM -0800, Jeff Davis wrote: > On Thu, 2020-12-03 at 13:14 -0500, Bruce Momjian wrote: > > Andres objected (in a separate conversation) to forcing a binary- > > > format > > > value on a client that didn't ask for one. He suggested that we > > > mandate > > > that

Re: Minor documentation error regarding streaming replication protocol

2020-12-15 Thread Andres Freund
Hi, On 2020-12-15 01:22:44 -0800, Jeff Davis wrote: > Attached a simple patch that enforces an all-ASCII restore point name > rather than documenting the possibility of non-ASCII text. +1 > diff --git a/src/backend/access/transam/xlogfuncs.c > b/src/backend/access/transam/xlogfuncs.c > index

Re: Minor documentation error regarding streaming replication protocol

2020-12-15 Thread Jeff Davis
On Thu, 2020-12-03 at 13:14 -0500, Bruce Momjian wrote: > Andres objected (in a separate conversation) to forcing a binary- > > format > > value on a client that didn't ask for one. He suggested that we > > mandate > > that the data is ASCII-only (for both filename and content), > > closing > >

Re: Minor documentation error regarding streaming replication protocol

2020-12-07 Thread Jeff Davis
On Thu, 2020-12-03 at 13:14 -0500, Bruce Momjian wrote: > How do we mandate that? Just mention it in the docs and C comments? Can't we just throw an error in pg_create_restore_point() if any high bits are set? And perhaps error or Assert at a later point in case we change the code in such a way

Re: Minor documentation error regarding streaming replication protocol

2020-12-03 Thread Bruce Momjian
On Thu, Dec 3, 2020 at 09:04:21AM -0800, Jeff Davis wrote: > On Wed, 2020-12-02 at 15:16 -0500, Bruce Momjian wrote: > > Yes, we could, but I thought the format code was not something we set > > at > > this level. Looking at byteasend() it is true it just sends the > > bytes. > > It can be set

Re: Minor documentation error regarding streaming replication protocol

2020-12-03 Thread Jeff Davis
On Wed, 2020-12-02 at 15:16 -0500, Bruce Momjian wrote: > Yes, we could, but I thought the format code was not something we set > at > this level. Looking at byteasend() it is true it just sends the > bytes. It can be set along with the type. Attached an example. Andres objected (in a separate

Re: Minor documentation error regarding streaming replication protocol

2020-12-02 Thread Bruce Momjian
On Tue, Dec 1, 2020 at 10:16:31AM -0800, Jeff Davis wrote: > On Thu, 2020-10-08 at 23:17 -0400, Bruce Momjian wrote: > > As I said before, it is more raw bytes, but > > we > > don't have an SQL data type for that. > > Sorry to jump in to this thread late. > > Can't we just set both "filename"

Re: Minor documentation error regarding streaming replication protocol

2020-12-01 Thread Jeff Davis
On Thu, 2020-10-08 at 23:17 -0400, Bruce Momjian wrote: > As I said before, it is more raw bytes, but > we > don't have an SQL data type for that. Sorry to jump in to this thread late. Can't we just set both "filename" and "content" to be BYTEA, but then set the format code to 1 (indicating

Re: Aw: Re: Minor documentation error regarding streaming replication protocol

2020-11-12 Thread Bruce Momjian
On Thu, Oct 15, 2020 at 12:43:22PM -0400, Bruce Momjian wrote: > On Thu, Oct 15, 2020 at 12:01:21PM -0400, Tom Lane wrote: > > Bruce Momjian writes: > > > We would want the timeline history file contents label changed from > > > BYTEA to TEXT in the docs changed for all supported versions, add a

Re: Aw: Re: Minor documentation error regarding streaming replication protocol

2020-10-15 Thread Bruce Momjian
On Thu, Oct 15, 2020 at 12:01:21PM -0400, Tom Lane wrote: > Bruce Momjian writes: > > We would want the timeline history file contents label changed from > > BYTEA to TEXT in the docs changed for all supported versions, add a C > > comment to all backbranches that BYTEA is the same as TEXT

Re: Aw: Re: Minor documentation error regarding streaming replication protocol

2020-10-15 Thread Tom Lane
Bruce Momjian writes: > We would want the timeline history file contents label changed from > BYTEA to TEXT in the docs changed for all supported versions, add a C > comment to all backbranches that BYTEA is the same as TEXT protocol > fields, and change the C code to return TEXT IN PG 14. Is

Aw: Re: Re: Minor documentation error regarding streaming replication protocol

2020-10-15 Thread Brar Piening
Tom Lane wrote: > Well, let's label it as text and then in the description > of the message, note that the field contains the raw file contents, > whose encoding is unknown to the server. +1 This would definitely have solved my initial issue and looks like a sane way forward without

Re: Aw: Re: Minor documentation error regarding streaming replication protocol

2020-10-15 Thread Bruce Momjian
On Thu, Oct 15, 2020 at 11:18:33AM -0400, Tom Lane wrote: > Bruce Momjian writes: > > On Thu, Oct 15, 2020 at 10:03:46AM -0400, Tom Lane wrote: > >> I don't really see why our only documentation choices are "text" or > >> "bytea". Can't we just write "(raw file contents)" or the like? > > >

Re: Aw: Re: Minor documentation error regarding streaming replication protocol

2020-10-15 Thread Tom Lane
Bruce Momjian writes: > On Thu, Oct 15, 2020 at 10:03:46AM -0400, Tom Lane wrote: >> I don't really see why our only documentation choices are "text" or >> "bytea". Can't we just write "(raw file contents)" or the like? > Agreed. The reason they are those labels is that the C code has to >

Re: Aw: Re: Minor documentation error regarding streaming replication protocol

2020-10-15 Thread Bruce Momjian
On Thu, Oct 15, 2020 at 10:03:46AM -0400, Tom Lane wrote: > Brar Piening writes: > > Bruce Momjian wrote: > >> I did some more research on this. It turns out timeline 'content' is > >> the only BYTEA listed in the protocol docs, even though it just passes C > >> strings to pq_sendbytes(), just

Re: Aw: Re: Minor documentation error regarding streaming replication protocol

2020-10-15 Thread Fujii Masao
On 2020/10/15 23:03, Tom Lane wrote: Brar Piening writes: Bruce Momjian wrote: I did some more research on this. It turns out timeline 'content' is the only BYTEA listed in the protocol docs, even though it just passes C strings to pq_sendbytes(), just like many other fields like the

Re: Aw: Re: Minor documentation error regarding streaming replication protocol

2020-10-15 Thread Tom Lane
Brar Piening writes: > Bruce Momjian wrote: >> I did some more research on this. It turns out timeline 'content' is >> the only BYTEA listed in the protocol docs, even though it just passes C >> strings to pq_sendbytes(), just like many other fields like the field >> above it, the timeline

Aw: Re: Minor documentation error regarding streaming replication protocol

2020-10-15 Thread Brar Piening
Bruce Momjian wrote: >> Good point. The reporter was assuming the data would come to the client >> in the bytea output format specified by the GUC, e.g. \x..., so that >> doesn't happen either. As I said before, it is more raw bytes, but we >> don't have an SQL data type for that. > I did some

Re: Minor documentation error regarding streaming replication protocol

2020-10-14 Thread Bruce Momjian
On Wed, Oct 14, 2020 at 05:10:30PM -0400, Tom Lane wrote: > Bruce Momjian writes: > > Patch attached. I would like to backpatch this to all supported > > versions so we are consistent and people don't think different PG > > versions use different return values for this. Is that safe? Looking >

Re: Minor documentation error regarding streaming replication protocol

2020-10-14 Thread Tom Lane
Bruce Momjian writes: > Patch attached. I would like to backpatch this to all supported > versions so we are consistent and people don't think different PG > versions use different return values for this. Is that safe? Looking > at the uses of this in our code, it seems so. We aren't doing

Re: Minor documentation error regarding streaming replication protocol

2020-10-14 Thread Bruce Momjian
On Thu, Oct 8, 2020 at 11:17:59PM -0400, Bruce Momjian wrote: > Good point. The reporter was assuming the data would come to the client > in the bytea output format specified by the GUC, e.g. \x..., so that > doesn't happen either. As I said before, it is more raw bytes, but we > don't have an

Re: Minor documentation error regarding streaming replication protocol

2020-10-08 Thread Bruce Momjian
On Fri, Oct 9, 2020 at 08:52:50AM +0900, Michael Paquier wrote: > On Thu, Oct 08, 2020 at 04:23:06PM -0400, Bruce Momjian wrote: > > I have looked at this. It seems SendTimeLineHistory() is sending raw > > bytes from the history file, with no encoding conversion, and > > ReceiveXlogStream() is

Re: Minor documentation error regarding streaming replication protocol

2020-10-08 Thread Michael Paquier
On Thu, Oct 08, 2020 at 04:23:06PM -0400, Bruce Momjian wrote: > I have looked at this. It seems SendTimeLineHistory() is sending raw > bytes from the history file, with no encoding conversion, and > ReceiveXlogStream() is receiving it, again assuming it is just plain > text. I am not sure we

Re: Minor documentation error regarding streaming replication protocol

2020-10-08 Thread Bruce Momjian
On Sun, Sep 13, 2020 at 10:25:01PM +0200, Brar Piening wrote: > While implementing streaming replication client functionality for Npgsql > I stumbled upon a minor documentation error at > https://www.postgresql.org/docs/current/protocol-replication.html > > The "content" return value for the

Minor documentation error regarding streaming replication protocol

2020-09-13 Thread Brar Piening
While implementing streaming replication client functionality for Npgsql I stumbled upon a minor documentation error at https://www.postgresql.org/docs/current/protocol-replication.html The "content" return value for the TIMELINE_HISTORYcommand is advertised as bytea while it is in fact raw ASCII

Re: Minor documentation error regarding streaming replication protocol

2020-09-13 Thread Brar Piening
Brar Piening wrote: > This is certainly a minor problem After thinking about it a little more I think that at least the fact that even the row description message advertises the content as bytea could be considered as a bug - no?