Horton

Re: Allow logical replication to copy tables in binary format

Amit Kapila <amit.kapila16@gmail.com>

View thread
From: Amit Kapila <amit.kapila16@gmail.com>
To: Peter Smith <smithpb2250@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-16T02:49:54Z
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 Thu, Mar 16, 2023 at 5:59 AM Peter Smith <smithpb2250@gmail.com> wrote:
>
> 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.
>

I find this needlessly verbose.

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

Okay, then let's go with that.

-- 
With Regards,
Amit Kapila.