Hello, Thanks for the quick review!
> 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. git apply works for me. I'm not sure why git am complains. I also created a git branch to make code sharing easier, please try this out: https://github.com/zli236/postgres/tree/ddl_replication > 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. Agreed. Fixed in the latest version. > I also think that ddl = '' isn't a good way to disable DDL replication, how > about using ddl = 'none' or something else? The syntax follows the existing convention of the WITH publish option. For example in CREATE PUBLICATION mypub WITH (publish = '') it means don't publish any DML change. So I'd leave it as is for now. > The test_decoding test case cannot work as expected, see below: ..... > DDL message: transactional: 1 prefix: role: redacted, search_path: > "$user", public, sz: 47 content:CREATE TABLE tab1 (id serial 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 don't understand the question, do you have an example of when the test doesn't work? It runs fine for me. > 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 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) This is fixed in the latest version. I need to remind myself to run make check-world in the future. Regards, Zheng Li
