Horton

Re: Allow logical replication to copy tables in binary format

Melih Mutlu <m.melihmutlu@gmail.com>

View thread
From: Melih Mutlu <m.melihmutlu@gmail.com>
To: vignesh C <vignesh21@gmail.com>, Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>, Jelte Fennema <postgres@jeltef.nl>
Cc: "Takamichi Osumi (Fujitsu)" <osumi.takamichi@fujitsu.com>, Amit Kapila <amit.kapila16@gmail.com>, pgsql-hackers <pgsql-hackers@postgresql.org>, "Hayato Kuroda (Fujitsu)" <kuroda.hayato@fujitsu.com>, "shiy.fnst@fujitsu.com" <shiy.fnst@fujitsu.com>, Euler Taveira <euler@eulerto.com>
Date: 2023-02-28T14:20:47Z
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.

Attachments

Hi,

Attached v11.

vignesh C <vignesh21@gmail.com>, 28 Şub 2023 Sal, 13:59 tarihinde şunu
yazdı:

> Thanks for the patch, Few comments:
> 1) Are primary key required for the tables, if not required we could
> remove the primary key which will speed up the test by not creating
> the indexes and inserting in the indexes. Even if required just create
> for one of the tables:
>

I think that having a primary key in tables for logical replication tests
is good for checking if log. rep. duplicates any row. Other tests also have
primary keys in almost all tables.

Bharath Rupireddy <bharath.rupireddyforpostgres@gmail.com>, 28 Şub 2023
Sal, 15:27 tarihinde şunu yazdı:

> 1.
> +      <para>
> +       If true, initial data synchronization will be performed in binary
> format
> +      </para></entry>
> It's not just the initial table sync right? The table sync can happen
> at any other point of time when ALTER SUBSCRIPTION ... REFRESH
> PUBLICATION WITH (copy = true) is run.
> How about - "If true, the subscriber requests publication for
> pre-existing data in binary format"?
>

I changed it as you suggested.
I sometimes feel like the phrase "initial sync" is used for initial sync of
a table, not a subscription. Or table syncs triggered by ALTER SUBSCRIPTION
are ignored in some places where "initial sync" is used.

2.
> Perhaps, this should cover the recommended cases for enabling this new
> option - something like below (may not need to have exact wording, but
> the recommended cases?):
> "It is recommended to enable this option only when 1) the column data
> types have appropriate binary send/receive functions, 2) not
> replicating between different major versions or different platforms,
> 3) both publisher and subscriber tables have the exact same column
> types (not when replicating from smallint to int or numeric to int8
> and so on), 4) both publisher and subscriber supports COPY with binary
> option, otherwise the table copy can fail."
>

I added a line stating that binary format is less portable across machine
architectures and versions as stated in COPY [1].
I don't think we should add line saying "recommended", but state the
restrictions clearly instead. It's also similar in COPY docs as well.
I think the explanation now covers all your points, right?

Jelte Fennema <postgres@jeltef.nl>, 28 Şub 2023 Sal, 16:25 tarihinde şunu
yazdı:

> > 3. I think the newly added tests must verify if the binary COPY is
> > picked up when enabled. Perhaps, looking at the publisher's server log
> > for 'COPY ... WITH BINARY format'? Maybe it's an overkill, otherwise,
> > we have no way of testing that the option took effect.
>
> Another way to test that BINARY is enabled could be to trigger one
> of the failure cases.


Yes, there is already a failure case for binary copy which resolves with
swithcing binary_copy to false.
But I also added checks for publisher logs now too.


[1] https://www.postgresql.org/docs/devel/sql-copy.html

Thanks,
-- 
Melih Mutlu
Microsoft