Horton

Re: Allow logical replication to copy tables in binary format

Peter Smith <smithpb2250@gmail.com>

View thread
From: Peter Smith <smithpb2250@gmail.com>
To: Amit Kapila <amit.kapila16@gmail.com>
Cc: Melih Mutlu <m.melihmutlu@gmail.com>, Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>, "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>, Dilip Kumar <dilipbalaut@gmail.com>, Euler Taveira <euler@eulerto.com>, "Takamichi Osumi (Fujitsu)" <osumi.takamichi@fujitsu.com>, pgsql-hackers <pgsql-hackers@postgresql.org>, "shiy.fnst@fujitsu.com" <shiy.fnst@fujitsu.com>
Date: 2023-03-16T00:29:22Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Allow logical replication to copy tables in binary format.

  2. Common function for percent placeholder replacement

  3. Doc: add XML ID attributes to <sectN> and <varlistentry> tags.

  4. Re-order disable_on_error in tab-complete.

  5. Make subscription tests pass with log_error_verbosity=verbose

  6. Weaken type-OID-matching checks in array_recv and record_recv.

  7. Allow logical replication to transfer data in binary format.

On Wed, Mar 15, 2023 at 5:31 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
>
> On Wed, Mar 15, 2023 at 11:52 AM Peter Smith <smithpb2250@gmail.com> wrote:
> >
> > ======
> > src/backend/replication/logical/tablesync.c
> >
> > 5.
> > +
> > + /*
> > + * If the publisher version is earlier than v14, it COPY command doesn't
> > + * support the binary option.
> > + */
> > + if (walrcv_server_version(LogRepWorkerWalRcvConn) >= 140000 &&
> > + MySubscription->binary)
> > + {
> > + appendStringInfo(&cmd, " WITH (FORMAT binary)");
> > + options = lappend(options, makeDefElem("format", (Node *)
> > makeString("binary"), -1));
> > + }
> >
> > Sorry, I gave a poor review comment for this previously. Now I have
> > revisited all the thread discussions about version checking. I feel
> > that some explanation should be given in the code comment so that
> > future readers of this code can understand why you decided to use v14
> > checking.
> >
> > Something like this:
> >
> > SUGGESTION
> > If the publisher version is earlier than v14, we use text format COPY.
> >
>
> I think this isn't explicit that we supported the binary format since
> v14. So, I would prefer my version of the comment as suggested in the
> previous email.
>

Hmm, but my *full* suggestion was bigger than what is misquoted above,
and it certainly did say " logical replication binary mode transfer
was not introduced until v14".

SUGGESTION
If the publisher version is earlier than v14, we use text format COPY.
Note - In fact COPY syntax "WITH (FORMAT binary)" has existed since
v9, but since the logical replication binary mode transfer was not
introduced until v14 it was decided to check using the later version.

~~

Anyway, the shortened comment as in the latest v15 patch is fine by me too.

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