Re: Support logical replication of DDLs - Mailing list pgsql-hackers

On Fri, 18 Mar 2022 at 08:18, Zheng Li <zhengli10@gmail.com> wrote:
> Hello,
>
> Attached please find the broken down patch set. Also fixed the failing
> TAP tests Japin reported.
>


Here are some comments for the new patches:

Seems like you forget initializing the *ddl_level_given after entering the
parse_publication_options(), see [1].


+           if (*ddl_level_given)
+               ereport(ERROR,
+                       (errcode(ERRCODE_SYNTAX_ERROR),
+                        errmsg("conflicting or redundant options")));

We can use the errorConflictingDefElem() to replace the ereport() to make the
error message more readable.

I also think that ddl = '' isn't a good way to disable DDL replication, how
about using ddl = 'none' or something else?

The test_decoding test case cannot work as expected, see below:

diff -U3 /home/px/Codes/postgresql/contrib/test_decoding/expected/ddlmessages.out
/home/px/Codes/postgresql/Debug/contrib/test_decoding/results/ddlmessages.out
--- /home/px/Codes/postgresql/contrib/test_decoding/expected/ddlmessages.out    2022-03-18 08:46:57.653922671 +0800
+++ /home/px/Codes/postgresql/Debug/contrib/test_decoding/results/ddlmessages.out       2022-03-18 17:34:33.411563601
+0800
@@ -25,8 +25,8 @@
 SELECT pg_drop_replication_slot('regression_slot');
 DROP TABLE tab1;
 DROP publication mypub;
-                                                                       data

----------------------------------------------------------------------------------------------------------------------------------------------------
+                                                                     data

+-----------------------------------------------------------------------------------------------------------------------------------------------
  DDL message: transactional: 1 prefix:  role: redacted, search_path: "$user", public, sz: 47 content:CREATE TABLE tab1
(idserial unique, data int);
 
  BEGIN
  sequence public.tab1_id_seq: transactional:1 last_value: 1 log_cnt: 0 is_called:0

Since the DDL message contains the username, and we try to replace the username with 'redacted' to avoid this problem,

    \o | sed 's/role.*search_path/role: redacted, search_path/g'

However, the title and dash lines may have different lengths for different
usernames.  To avoid this problem, how about using a specified username when
initializing the database for this regression test?

I try to disable the ddlmessage regression test,

make[2]: Entering directory '/home/px/Codes/postgresql/Debug/src/bin/pg_dump'
rm -rf '/home/px/Codes/postgresql/Debug/src/bin/pg_dump'/tmp_check && /usr/bin/mkdir -p
'/home/px/Codes/postgresql/Debug/src/bin/pg_dump'/tmp_check&& cd /home/px/Codes/postgresql/Debug/../src/bin/pg_dump
 
 && TESTDIR='/home/px/Codes/postgresql/Debug/src/bin/pg_dump'
PATH="/home/px/Codes/postgresql/Debug/tmp_install/home/px/Codes/postgresql/Debug/pg/bin:/home/px/Codes/postgresql/Debug/src/bin/pg_dump:$PATH"
 LD_LIBRARY_PATH="/home/px/Codes/postgresql/Debug/tmp_install/home/px/Codes/postgresql/Debug/pg/lib:$LD_LIBRARY_PATH"
PGPORT='65432'PG_REGRESS='/home/px/Codes/postgresql/Debug/src/bin/pg_dump/../../../s
 
rc/test/regress/pg_regress' /usr/bin/prove -I /home/px/Codes/postgresql/Debug/../src/test/perl/ -I
/home/px/Codes/postgresql/Debug/../src/bin/pg_dump t/*.pl
 
t/001_basic.pl ................ ok
t/002_pg_dump.pl .............. 13/?
#   Failed test 'binary_upgrade: should dump CREATE PUBLICATION pub1'
#   at t/002_pg_dump.pl line 3916.
# Review binary_upgrade results in /home/px/Codes/postgresql/Debug/src/bin/pg_dump/tmp_check/tmp_test_jgRI

#   Failed test 'binary_upgrade: should dump CREATE PUBLICATION pub2'
#   at t/002_pg_dump.pl line 3916.
# Review binary_upgrade results in /home/px/Codes/postgresql/Debug/src/bin/pg_dump/tmp_check/tmp_test_jgRI

#   Failed test 'binary_upgrade: should dump CREATE PUBLICATION pub3'
#   at t/002_pg_dump.pl line 3916.
# Review binary_upgrade results in /home/px/Codes/postgresql/Debug/src/bin/pg_dump/tmp_check/tmp_test_jgRI

#   Failed test 'binary_upgrade: should dump CREATE PUBLICATION pub4'
#   at t/002_pg_dump.pl line 3916.
# Review binary_upgrade results in /home/px/Codes/postgresql/Debug/src/bin/pg_dump/tmp_check/tmp_test_jgRI
t/002_pg_dump.pl .............. 240/?
[...]
# Review section_post_data results in /home/px/Codes/postgresql/Debug/src/bin/pg_dump/tmp_check/tmp_test_jgRI
t/002_pg_dump.pl .............. 7258/? # Looks like you failed 84 tests of 7709.
t/002_pg_dump.pl .............. Dubious, test returned 84 (wstat 21504, 0x5400)
Failed 84/7709 subtests
t/003_pg_dump_with_server.pl .. ok
t/010_dump_connstr.pl ......... ok

Test Summary Report
-------------------
t/002_pg_dump.pl            (Wstat: 21504 Tests: 7709 Failed: 84)
  Failed tests:  136-139, 362-365, 588-591, 1040-1043, 1492-1495
                1719-1722, 1946-1949, 2177-2180, 2407-2410
                2633-2636, 2859-2862, 3085-3088, 3537-3540
                3763-3766, 3989-3992, 4215-4218, 4441-4444
                5119-5122, 5345-5348, 6702-6705, 7154-7157
  Non-zero exit status: 84
Files=4, Tests=7808, 26 wallclock secs ( 0.46 usr  0.05 sys +  7.98 cusr  2.80 csys = 11.29 CPU)
Result: FAIL
make[2]: *** [Makefile:55: check] Error 1
make[2]: Leaving directory '/home/px/Codes/postgresql/Debug/src/bin/pg_dump'
make[1]: *** [Makefile:43: check-pg_dump-recurse] Error 2
make[1]: Leaving directory '/home/px/Codes/postgresql/Debug/src/bin'
make: *** [GNUmakefile:71: check-world-src/bin-recurse] Error 2


And, when I try to use git am to apply the patch, it complains:

    $ git am ~/0001-syntax-pg_publication-pg_dump-ddl_replication.patch
    Patch format detection failed.


[1]
https://www.postgresql.org/message-id/MEYP282MB1669DDF788C623B7F8B64C2EB6139%40MEYP282MB1669.AUSP282.PROD.OUTLOOK.COM

--
Regrads,
Japin Li.
ChengDu WenWu Information Technology Co.,Ltd.