Hi Shubhab.
Here are some more review comments for the v23-0001.
======
011_generated.pl b/src/test/subscription/t/011_generated.pl
nitpick - renamed /regress_pub/regress_pub_tab1/ and
/regress_sub1/regress_sub1_tab1/
nitpick - typo /inital data /initial data/
nitpick - typo /snode_subscriber2/node_subscriber2/
nitpick - tweak the combo initial sync comments and messages
nitpick - /#Cleanup/# cleanup/
nitpick - tweak all the combo normal replication comments
nitpick - removed blank line at the end
~~~
1. Refactor tab_gen_to_missing initial sync tests.
I moved the tab_gen_to_missing initial sync for node_subscriber2 to be
back where all the other initial sync tests are done.
See the nitpicks patch file.
~~~
2. Refactor tab_nogen_to_gen initial sync tests
I moved all the tab_nogen_to_gen initial sync tests back to where the
other initial sync tests are done.
See the nitpicks patch file.
~~~
3. Added another test case:
Because the (current PG17) nogen-to-gen initial sync test case (with
copy_data=true) gives an ERROR, I have added another combination to
cover normal replication (e.g. using copy_data=false).
See the nitpicks patch file.
(This has exposed an inconsistency which IMO might be a PG17 bug. I
have included TAP test comments about this, and plan to post a
separate thread for it later).
~
4. GUC
Moving and adding more CREATE SUBSCRIPTION exceeded some default GUCs,
so extra configuration was needed.
See the nitpick patch file.
======
Kind Regards,
Peter Smith.
Fujitsu Australia
diff --git a/src/test/subscription/t/011_generated.pl
b/src/test/subscription/t/011_generated.pl
index 0b596b7..2be06c6 100644
--- a/src/test/subscription/t/011_generated.pl
+++ b/src/test/subscription/t/011_generated.pl
@@ -12,16 +12,25 @@ use Test::More;
my $node_publisher = PostgreSQL::Test::Cluster->new('publisher');
$node_publisher->init(allows_streaming => 'logical');
+$node_publisher->append_conf('postgresql.conf',
+ "max_wal_senders = 20
+ max_replication_slots = 20");
$node_publisher->start;
# All subscribers on this node will use parameter include_generated_columns =
false
my $node_subscriber = PostgreSQL::Test::Cluster->new('subscriber');
$node_subscriber->init;
+$node_subscriber->append_conf('postgresql.conf',
+ "max_logical_replication_workers = 20
+ max_worker_processes = 20");
$node_subscriber->start;
# All subscribers on this node will use parameter include_generated_columns =
true
my $node_subscriber2 = PostgreSQL::Test::Cluster->new('subscriber2');
$node_subscriber2->init;
+$node_subscriber2->append_conf('postgresql.conf',
+ "max_logical_replication_workers = 20
+ max_worker_processes = 20");
$node_subscriber2->start;
my $publisher_connstr = $node_publisher->connstr . ' dbname=postgres';
@@ -139,7 +148,7 @@ $node_publisher->safe_psql('postgres',
# pub_combo_gen_to_missing is not included in pub_combo, because some tests
give errors.
$node_publisher->safe_psql('postgres',
- "CREATE PUBLICATION regress_pub FOR TABLE tab1");
+ "CREATE PUBLICATION regress_pub_tab1 FOR TABLE tab1");
$node_publisher->safe_psql('postgres',
"CREATE PUBLICATION regress_pub_combo FOR TABLE tab_gen_to_gen,
tab_gen_to_nogen, tab_missing_to_gen"
);
@@ -157,10 +166,10 @@ $node_publisher->safe_psql('postgres',
#
# Note that all subscriptions created on node_subscriber2 use copy_data =
false,
# because copy_data = true with include_generated_columns is not yet supported.
-# For this reason, the expected inital data on snode_subscriber2 is always
empty.
+# For this reason, the expected inital data on node_subscriber2 is always
empty.
$node_subscriber->safe_psql('postgres',
- "CREATE SUBSCRIPTION regress_sub1 CONNECTION '$publisher_connstr'
PUBLICATION regress_pub"
+ "CREATE SUBSCRIPTION regress_sub1_tab1 CONNECTION '$publisher_connstr'
PUBLICATION regress_pub_tab1"
);
$node_subscriber->safe_psql('postgres',
"CREATE SUBSCRIPTION regress_sub1_combo CONNECTION '$publisher_connstr'
PUBLICATION regress_pub_combo"
@@ -168,11 +177,18 @@ $node_subscriber->safe_psql('postgres',
$node_subscriber->safe_psql('postgres',
"CREATE SUBSCRIPTION regress_sub1_combo_gen_to_missing CONNECTION
'$publisher_connstr' PUBLICATION regress_pub_combo_gen_to_missing"
);
+# Note, regress_sub1_combo_nogen_to_gen is not created here due to expected
errors. See later.
$node_subscriber2->safe_psql('postgres',
"CREATE SUBSCRIPTION regress_sub2_combo CONNECTION '$publisher_connstr'
PUBLICATION regress_pub_combo WITH (include_generated_columns = true, copy_data
= false)"
);
$node_subscriber2->safe_psql('postgres',
+ "CREATE SUBSCRIPTION regress_sub2_combo_gen_to_missing CONNECTION
'$publisher_connstr' PUBLICATION regress_pub_combo_gen_to_missing with
(include_generated_columns = true, copy_data = false)"
+);
+$node_subscriber2->safe_psql('postgres',
+ "CREATE SUBSCRIPTION regress_sub2_combo_nogen_to_gen CONNECTION
'$publisher_connstr' PUBLICATION regress_pub_combo_nogen_to_gen WITH
(include_generated_columns = true, copy_data = false)"
+);
+$node_subscriber2->safe_psql('postgres',
"CREATE SUBSCRIPTION regress_sub2_misc CONNECTION '$publisher_connstr'
PUBLICATION regress_pub_misc WITH (include_generated_columns = true, copy_data
= false)"
);
@@ -188,57 +204,82 @@ is( $result, qq(1|22
2|44
3|66), 'generated columns initial sync');
-# gen-to-gen
+#####################
+# TEST tab_gen_to_gen initial sync
+#####################
$result =
$node_subscriber->safe_psql('postgres', "SELECT a, b FROM tab_gen_to_gen");
is( $result, qq(1|22
2|44
-3|66), 'generated columns initial sync, when include_generated_columns=false'
+3|66), 'tab_gen_to_gen initial sync, when include_generated_columns=false'
);
$result =
$node_subscriber2->safe_psql('postgres', "SELECT a, b FROM tab_gen_to_gen");
is($result, qq(),
- 'generated columns initial sync, when include_generated_columns=true');
+ 'tab_gen_to_gen initial sync, when include_generated_columns=true');
-# gen-to-nogen
+#####################
+# TEST tab_gen_to_nogen initial sync
+#####################
$result = $node_subscriber->safe_psql('postgres',
"SELECT a, b FROM tab_gen_to_nogen");
is( $result, qq(1|
2|
-3|), 'generated columns initial sync, when include_generated_columns=false');
+3|), 'tab_gen_to_nogen, when include_generated_columns=false');
$result = $node_subscriber2->safe_psql('postgres',
"SELECT a, b FROM tab_gen_to_nogen");
is($result, qq(),
- 'generated columns initial sync, when include_generated_columns=true');
+ 'tab_gen_to_nogen initial sync, when include_generated_columns=true');
-# gen-to-missing
-# Note, node_subscriber2 is not subscribing to this yet. See later.
+#####################
+# TEST tab_gen_to_missing initial sync
+#####################
$result =
$node_subscriber->safe_psql('postgres', "SELECT a FROM tab_gen_to_missing");
is( $result, qq(1
2
-3), 'generated columns initial sync, when include_generated_columns=false');
+3), 'tab_gen_to_missing initial sync, when include_generated_columns=false');
+# Note, the following is expected to work only because copy_data = false
+$result =
+ $node_subscriber2->safe_psql('postgres', "SELECT a FROM tab_gen_to_missing");
+is( $result, qq(), 'tab_gen_to_missing initial sync, when
include_generated_columns=true');
-# missing-to-gen
+#####################
+# TEST tab_missing_to_gen initial sync
+#####################
$result = $node_subscriber->safe_psql('postgres',
"SELECT a, b FROM tab_missing_to_gen");
is( $result, qq(1|22
2|44
-3|66), 'generated columns initial sync, when include_generated_columns=false'
+3|66), 'tab_missing_to_gen initial sync, when include_generated_columns=false'
);
$result = $node_subscriber2->safe_psql('postgres',
"SELECT a, b FROM tab_missing_to_gen");
is($result, qq(),
- 'generated columns initial sync, when include_generated_columns=true');
+ 'tab_missing_to_gen initial sync, when include_generated_columns=true');
-# nogen-to-gen
-# Note, node_subscriber is not subscribing to this yet. See later
-# Note, node_subscriber2 is not subscribing to this yet. See later
+#####################
+# TEST tab_nogen_to_gen initial sync
+#####################
+# The subscription is created here, because it causes the tablesync worker to
restart repetitively.
+my $offset = -s $node_subscriber->logfile;
+$node_subscriber->safe_psql('postgres',
+ "CREATE SUBSCRIPTION regress_sub1_combo_nogen_to_gen CONNECTION
'$publisher_connstr' PUBLICATION regress_pub_combo_nogen_to_gen WITH
(include_generated_columns = false)"
+);
+$node_subscriber->wait_for_log(
+ qr/ERROR: ( [A-Z0-9]:)? column "b" is a generated column/, $offset);
+# Note, the following is expected to work only because copy_data = false
+$result = $node_subscriber2->safe_psql('postgres',
+ "SELECT a, b FROM tab_nogen_to_gen");
+is($result, qq(),
+ 'tab_nogen_to_gen initial sync, when include_generated_columns=true');
+# tab_order:
$result = $node_subscriber2->safe_psql('postgres',
"SELECT a, b, c FROM tab_order ORDER BY a");
is($result, qq(), 'generated column initial sync');
+# tab_alter:
$result = $node_subscriber2->safe_psql('postgres',
"SELECT a, b, c FROM tab_alter ORDER BY a");
is($result, qq(), 'unsubscribed table initial data');
@@ -249,7 +290,7 @@ $node_publisher->safe_psql('postgres', "INSERT INTO tab1
VALUES (4), (5)");
$node_publisher->safe_psql('postgres', "UPDATE tab1 SET a = 6 WHERE a = 5");
-$node_publisher->wait_for_catchup('regress_sub1');
+$node_publisher->wait_for_catchup('regress_sub1_tab1');
$result = $node_subscriber->safe_psql('postgres', "SELECT * FROM tab1");
is( $result, qq(1|22|
@@ -259,7 +300,7 @@ is( $result, qq(1|22|
6|132|), 'generated columns replicated');
#####################
-# TEST tab_gen_to_gen
+# TEST tab_gen_to_gen replication
#
# publisher-side has generated col 'b'.
# subscriber-side has generated col 'b', using a different computation.
@@ -298,7 +339,7 @@ is( $result, qq(4|88
);
#####################
-# TEST tab_gen_to_nogen
+# TEST tab_gen_to_nogen replication
#
# publisher-side has generated col 'b'.
# subscriber-side has non-generated col 'b'.
@@ -334,7 +375,7 @@ is( $result, qq(4|8
);
#####################
-# TEST tab_gen_to_missing
+# TEST tab_gen_to_missing replication
#
# publisher-side has generated col 'b'.
# subscriber-side col 'b' is missing.
@@ -360,21 +401,11 @@ is( $result, qq(1
# regress_sub2_combo_gen_to_missing: (include_generated_columns = true)
# Confirm that col 'b' is not replicated and it will throw an error.
my $offset2 = -s $node_subscriber2->logfile;
-
-# The subscription is created here, because it causes the tablesync worker to
restart repetitively.
-$node_subscriber2->safe_psql('postgres',
- "CREATE SUBSCRIPTION regress_sub2_combo_gen_to_missing CONNECTION
'$publisher_connstr' PUBLICATION regress_pub_combo_gen_to_missing with
(include_generated_columns = true, copy_data = false)"
-);
-
-# insert data
-$node_publisher->safe_psql('postgres',
- "INSERT INTO tab_gen_to_missing VALUES (6)");
-
$node_subscriber2->wait_for_log(
qr/ERROR: ( [A-Z0-9]+:)? logical replication target relation
"public.tab_gen_to_missing" is missing replicated column: "b"/,
$offset2);
-#Cleanup
+# cleanup
$node_subscriber->safe_psql('postgres',
"DROP SUBSCRIPTION regress_sub1_combo_gen_to_missing");
$node_subscriber2->safe_psql('postgres',
@@ -383,7 +414,7 @@ $node_publisher->safe_psql('postgres',
"DROP PUBLICATION regress_pub_combo_gen_to_missing");
#####################
-# TEST tab_missing_to_gen
+# TEST tab_missing_to_gen replication
#
# publisher-side col 'b' is missing.
# subscriber-side col 'b' is generated.
@@ -426,57 +457,62 @@ $node_subscriber2->safe_psql('postgres',
$node_publisher->safe_psql('postgres', "DROP PUBLICATION regress_pub_combo");
#####################
-# TEST tab_nogen_to_gen
+# TEST tab_nogen_to_gen replication
#
# publisher-side has non-generated col 'b'.
# subscriber-side has generated col 'b'.
#####################
+# When copy_data=true a COPY error occurred. Try again but with
copy_data=false.
+$node_subscriber->safe_psql('postgres',
+ "CREATE SUBSCRIPTION regress_sub1_combo_nogen_to_gen_nocopy CONNECTION
'$publisher_connstr' PUBLICATION regress_pub_combo_nogen_to_gen WITH
(include_generated_columns = false, copy_data = false)"
+);
+
# insert data
$node_publisher->safe_psql('postgres',
"INSERT INTO tab_nogen_to_gen VALUES (4), (5)");
# regress_sub1_combo_nogen_to_gen: (include_generated_columns = false)
-# Confirm that col 'b' is not replicated and it will throw a COPY error.
#
-# The subscription is created here, because it causes the tablesync worker to
restart repetitively.
-my $offset = -s $node_subscriber->logfile;
-$node_subscriber->safe_psql('postgres',
- "CREATE SUBSCRIPTION regress_sub1_combo_nogen_to_gen CONNECTION
'$publisher_connstr' PUBLICATION regress_pub_combo_nogen_to_gen WITH
(include_generated_columns = false)"
+# XXX
+# The test below shows that current PG17 behavior does not give an error,
+# But this conflicts with the copy_data=true behavior so it might be a PG17
bug.
+# Needs more study.
+$node_publisher->wait_for_catchup('regress_sub1_combo_nogen_to_gen_nocopy');
+$result =
+ $node_subscriber->safe_psql('postgres',
+ "SELECT a, b FROM tab_nogen_to_gen ORDER BY a");
+is($result, qq(4|88
+5|110),
+ 'confirm when publisher col is not generated, subscriber generated
columns are generated as normal'
);
-$node_subscriber->wait_for_log(
- qr/ERROR: ( [A-Z0-9]:)? column "b" is a generated column/, $offset);
# regress_sub2_combo_nogen_to_gen: (include_generated_columns = true)
+# When copy_data=false, no COPY error occurs.
+# The col 'b' is not replicated; the subscriber-side generated value is
inserted.
#
# XXX
-# when copy_data=false, no COPY error occurs.
-# the col 'b' is not replicated; the subscriber-side generated value is
inserted.
-$node_subscriber2->safe_psql('postgres',
- "CREATE SUBSCRIPTION regress_sub2_combo_nogen_to_gen CONNECTION
'$publisher_connstr' PUBLICATION regress_pub_combo_nogen_to_gen WITH
(include_generated_columns = true, copy_data = false)"
-);
-
-# insert data
-$node_publisher->safe_psql('postgres',
- "INSERT INTO tab_nogen_to_gen VALUES (6)");
-
+# It is correct for this to give the same result as above, but it needs more
+# study to determine if the above result was actually correct, or a PG17 bug.
$node_publisher->wait_for_catchup('regress_sub2_combo_nogen_to_gen');
$result =
$node_subscriber2->safe_psql('postgres',
"SELECT a, b FROM tab_nogen_to_gen ORDER BY a");
-is($result, qq(6|132),
+is($result, qq(4|88
+5|110),
'confirm when publisher col is not generated, subscriber generated
columns are generated as normal'
);
# cleanup
-
+$node_subscriber->safe_psql('postgres',
+ "DROP SUBSCRIPTION regress_sub1_combo_nogen_to_gen_nocopy");
$node_subscriber2->safe_psql('postgres',
"DROP SUBSCRIPTION regress_sub2_combo_nogen_to_gen");
$node_publisher->safe_psql('postgres',
"DROP PUBLICATION regress_pub_combo_nogen_to_gen");
#####################
-# TEST tab_order:
+# TEST tab_order replication
#
# publisher-side cols 'b' and 'c' are generated
# subscriber-side col 'b' is not generated and col 'c' is generated.
@@ -498,7 +534,7 @@ is( $result, qq(4|8|88
'replicate generated columns with different order on the subscriber');
#####################
-# TEST tab_alter
+# TEST tab_alter replication
#
# Add a new table to existing publication, then
# do ALTER SUBSCRIPTION ... REFRESH PUBLICATION
@@ -516,7 +552,7 @@ is( $result, qq(1||22
3||66), 'add new table to existing publication');
#####################
-# TEST tabl_alter
+# TEST tab_alter
#
# Drop the generated column's expression on subscriber side.
# This changes the generated column into a non-generated column.
@@ -547,7 +583,6 @@ $node_publisher->safe_psql('postgres', "DROP PUBLICATION
regress_pub_misc");
#####################
# try it with a subscriber-side trigger
-
$node_subscriber->safe_psql(
'postgres', q{
CREATE FUNCTION tab1_trigger_func() RETURNS trigger
@@ -568,7 +603,7 @@ $node_publisher->safe_psql('postgres', "INSERT INTO tab1
VALUES (7), (8)");
$node_publisher->safe_psql('postgres', "UPDATE tab1 SET a = 9 WHERE a = 7");
-$node_publisher->wait_for_catchup('regress_sub1');
+$node_publisher->wait_for_catchup('regress_sub1_tab1');
$result =
$node_subscriber->safe_psql('postgres', "SELECT * FROM tab1 ORDER BY 1");