Horton

Re: Support logical replication of DDLs

Zheng Li <zhengli10@gmail.com>

View thread
From: Zheng Li <zhengli10@gmail.com>
To: Japin Li <japinli@hotmail.com>
Cc: Alvaro Herrera <alvherre@alvh.no-ip.org>, Dilip Kumar <dilipbalaut@gmail.com>, rajesh.rs0541@gmail.com, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2022-03-18T17:25:09Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Add a run_as_owner option to subscriptions.

  2. Refactor pgoutput_change().

  3. Print the correct aliases for DML target tables in ruleutils.

  4. Fix object identity string for transforms

  5. Add grantable MAINTAIN privilege and pg_maintain role.

  6. Get rid of recursion-marker values in enum AlterTableType

  7. Release cache tuple when no longer needed

  8. Empty search_path in logical replication apply worker and walsender.

  9. Refactor format_type APIs to be more modular

  10. Use wrappers of PG_DETOAST_DATUM_PACKED() more.

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