On Fri, 17 Mar 2023 at 17:55, Melih Mutlu <m.melihmu...@gmail.com> wrote:
>
> Hi,
>
> Sharing v17.
>
> Amit Kapila <amit.kapil...@gmail.com>, 17 Mar 2023 Cum, 03:02 tarihinde şunu
> yazdı:
>>
>> I think to reduce the risk of breakage, let's change the check to
>> >=v16. Also, accordingly, update the doc and commit message.
>
>
> Done.
>
> Peter Smith <smithpb2...@gmail.com>, 17 Mar 2023 Cum, 04:58 tarihinde şunu
> yazdı:
>>
>> IMO the sentence "However, logical replication in binary format is
>> more restrictive." should just be plain text.
>
>
> Done.
>
> shiy.f...@fujitsu.com <shiy.f...@fujitsu.com>, 17 Mar 2023 Cum, 05:26
> tarihinde şunu yazdı:
>>
>> It looks that you forgot to pass `offset` into wait_for_log().
>
>
> Yes, I somehow didn't include those lines into the patch. Thanks for
> noticing. Fixed them now.
Thanks for the updated patch, few comments:
1) Currently we refer the link to the beginning of create subscription
page, this can be changed to refer to binary option contents in create
subscription:
+ <para>
+ See the <literal>binary</literal> option of
+ <link linkend="sql-createsubscription"><command>CREATE
SUBSCRIPTION</command></link>
+ for details about copying pre-existing data in binary format.
+ </para>
2) Running pgperltidy shows the test script 014_binary.pl could be
slightly improved as in the attachment.
Regards,
Vignesh
diff --git a/doc/src/sgml/logical-replication.sgml b/doc/src/sgml/logical-replication.sgml
index bad25e54cd..9c96f900b4 100644
--- a/doc/src/sgml/logical-replication.sgml
+++ b/doc/src/sgml/logical-replication.sgml
@@ -253,7 +253,7 @@
will be filled with the default value as specified in the definition of the
target table. However, logical replication in <literal>binary</literal>
format is more restrictive. See the <literal>binary</literal> option of
- <link linkend="sql-createsubscription"><command>CREATE SUBSCRIPTION</command></link>
+ <link linkend="binary-option-create-subscription"><command>CREATE SUBSCRIPTION</command></link>
for details.
</para>
diff --git a/doc/src/sgml/ref/alter_subscription.sgml b/doc/src/sgml/ref/alter_subscription.sgml
index ccd4ed6f59..fb0ddd2d7f 100644
--- a/doc/src/sgml/ref/alter_subscription.sgml
+++ b/doc/src/sgml/ref/alter_subscription.sgml
@@ -179,7 +179,7 @@ ALTER SUBSCRIPTION <replaceable class="parameter">name</replaceable> RENAME TO <
</para>
<para>
See the <literal>binary</literal> option of
- <link linkend="sql-createsubscription"><command>CREATE SUBSCRIPTION</command></link>
+ <link linkend="binary-option-create-subscription"><command>CREATE SUBSCRIPTION</command></link>
for details about copying pre-existing data in binary format.
</para>
</listitem>
diff --git a/doc/src/sgml/ref/create_subscription.sgml b/doc/src/sgml/ref/create_subscription.sgml
index e0be2b586f..edf52fa7d4 100644
--- a/doc/src/sgml/ref/create_subscription.sgml
+++ b/doc/src/sgml/ref/create_subscription.sgml
@@ -185,7 +185,7 @@ CREATE SUBSCRIPTION <replaceable class="parameter">subscription_name</replaceabl
<variablelist>
- <varlistentry>
+ <varlistentry id="binary-option-create-subscription" xreflabel="binary">
<term><literal>binary</literal> (<type>boolean</type>)</term>
<listitem>
<para>
diff --git a/src/test/subscription/t/014_binary.pl b/src/test/subscription/t/014_binary.pl
index d20f94b8d3..f5fa5976e1 100644
--- a/src/test/subscription/t/014_binary.pl
+++ b/src/test/subscription/t/014_binary.pl
@@ -57,16 +57,18 @@ $node_publisher->safe_psql(
my $publisher_connstring = $node_publisher->connstr . ' dbname=postgres';
$node_subscriber->safe_psql('postgres',
- "CREATE SUBSCRIPTION tsub CONNECTION '$publisher_connstring' "
+ "CREATE SUBSCRIPTION tsub CONNECTION '$publisher_connstring' "
. "PUBLICATION tpub WITH (slot_name = tpub_slot, binary = true)");
# Ensure the COPY command is executed in binary format on the publisher
-$node_publisher->wait_for_log(qr/LOG: ( [A-Z0-9]+:)? statement: COPY (.+)? TO STDOUT WITH \(FORMAT binary\)/);
+$node_publisher->wait_for_log(
+ qr/LOG: ( [A-Z0-9]+:)? statement: COPY (.+)? TO STDOUT WITH \(FORMAT binary\)/
+);
# Ensure nodes are in sync with each other
$node_subscriber->wait_for_subscription_sync($node_publisher, 'tsub');
-my $sync_check = qq(
+my $sync_check = qq(
SELECT a, b, c, d FROM test_numerical ORDER BY a;
SELECT a, b, c FROM test_arrays ORDER BY a;
);
@@ -207,10 +209,12 @@ $node_publisher->safe_psql(
my $offset = -s $node_subscriber->logfile;
# Refresh the publication to trigger the tablesync
-$node_subscriber->safe_psql('postgres', "ALTER SUBSCRIPTION tsub REFRESH PUBLICATION");
+$node_subscriber->safe_psql('postgres',
+ "ALTER SUBSCRIPTION tsub REFRESH PUBLICATION");
# It should fail
-$node_subscriber->wait_for_log(qr/ERROR: ( [A-Z0-9]+:)? no binary input function available for type/,
+$node_subscriber->wait_for_log(
+ qr/ERROR: ( [A-Z0-9]+:)? no binary input function available for type/,
$offset);
# Create and set send/rcv functions for the custom type
@@ -231,9 +235,10 @@ $node_subscriber->safe_psql('postgres', $ddl);
$node_subscriber->wait_for_subscription_sync($node_publisher, 'tsub');
# Check the synced data on the subscriber
-$result = $node_subscriber->safe_psql('postgres', 'SELECT a FROM test_myvarchar;');
+$result =
+ $node_subscriber->safe_psql('postgres', 'SELECT a FROM test_myvarchar;');
-is( $result, 'a', 'check synced data on subscriber with custom type');
+is($result, 'a', 'check synced data on subscriber with custom type');
# -----------------------------------------------------
# Test mismatched column types with/without binary mode
@@ -241,7 +246,7 @@ is( $result, 'a', 'check synced data on subscriber with custom type');
# Test syncing tables with mismatching column types
$node_publisher->safe_psql(
- 'postgres', qq(
+ 'postgres', qq(
CREATE TABLE public.test_mismatching_types (
a bigint PRIMARY KEY
);
@@ -253,7 +258,7 @@ $node_publisher->safe_psql(
$offset = -s $node_subscriber->logfile;
$node_subscriber->safe_psql(
- 'postgres', qq(
+ 'postgres', qq(
CREATE TABLE public.test_mismatching_types (
a int PRIMARY KEY
);
@@ -261,23 +266,26 @@ $node_subscriber->safe_psql(
));
# Cannot sync due to type mismatch
-$node_subscriber->wait_for_log(qr/ERROR: ( [A-Z0-9]+:)? incorrect binary data format/);
+$node_subscriber->wait_for_log(
+ qr/ERROR: ( [A-Z0-9]+:)? incorrect binary data format/);
# Check the publisher log from now on.
$offset = -s $node_publisher->logfile;
# Setting binary to false should allow syncing
$node_subscriber->safe_psql(
- 'postgres', qq(
+ 'postgres', qq(
ALTER SUBSCRIPTION tsub SET (binary = false);));
# Ensure the COPY command is executed in text format on the publisher
-$node_publisher->wait_for_log(qr/LOG: ( [A-Z0-9]+:)? statement: COPY (.+)? TO STDOUT\n/);
+$node_publisher->wait_for_log(
+ qr/LOG: ( [A-Z0-9]+:)? statement: COPY (.+)? TO STDOUT\n/);
$node_subscriber->wait_for_subscription_sync($node_publisher, 'tsub');
# Check the synced data on the subscriber
-$result = $node_subscriber->safe_psql('postgres', 'SELECT a FROM test_mismatching_types ORDER BY a;');
+$result = $node_subscriber->safe_psql('postgres',
+ 'SELECT a FROM test_mismatching_types ORDER BY a;');
is( $result, '1
2', 'check synced data on subscriber with binary = false');
