Re: Allow logical replication to copy tables in binary format - Mailing list pgsql-hackers

(1) whitespace issues

(2) Suggestion to update another general description about the subscription

Kindly have a look at doc/src/sgml/logical-replication.sgml.

"The data types of the columns do not need to match,
as long as the text representation of the data can be converted to the target type.
For example, you can replicate from a column of type integer to a column of type bigint."

With the patch, I think we have an impact about those descriptions
since enabling the binary option for a subscription and executing the
initial synchronization requires the same data types for binary format.

I suggest that we update those descriptions as well.

(3) shouldn't test that we fail expectedly with binary copy for different types ?

How about having a test that we correctly fail with different data types
between the publisher and the subscriber, for instance ?

(4) Error message of the binary format copy

I've gotten below message from data type contradiction (between integer and bigint).
Probably, this is unclear for the users to understand the direct cause
and needs to be adjusted ?
This might be a similar comment Euler mentioned in [1].

2022-09-16 11:54:54.835 UTC [4570] ERROR:  insufficient data left in message
2022-09-16 11:54:54.835 UTC [4570] CONTEXT:  COPY tab, line 1, column id

(5) Minor adjustment of the test comment in 002_types.pl.

+is( $result, $sync_result, 'check initial sync on subscriber');
+is( $result_binary, $sync_result, 'check initial sync on subscriber in binary');

 # Insert initial test data

There are two same comments which say "Insert initial test data" in this file.
We need to update them, one for the initial table sync and
the other for the application of changes.

I agree with the direction to support binary copy for v16 and later.

IIUC, the binary format replication with different data types fails even during apply phase on HEAD.
I thought that means, the upgrade concern only applies to a scenario that the user executes
only initial table synchronizations between the publisher and subscriber
and doesn't replicate any data at apply phase after that. I would say
this isn't a valid scenario and your proposal makes sense.