RE: Support logical replication of DDLs

On Monday, April 10, 2023 7:20 PM Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
>
> On Fri, Apr 7, 2023 at 8:52 AM houzj(dot)fnst(at)fujitsu(dot)com
> <houzj(dot)fnst(at)fujitsu(dot)com> wrote:
> >
> > Sorry, there was a miss when rebasing the patch which could cause the
> > CFbot to fail and here is the correct patch set.
> >
>
> I see the following note in the patch: "Note: For ATTACH/DETACH PARTITION,
> we haven't added extra logic on the subscriber to handle the case where the
> table on the publisher is a PARTITIONED TABLE while the target table on the
> subscriber side is a NORMAL table. We will research this more and improve it
> later." and wonder what should we do about this. I can think of the following
> possibilities: (a) Convert a non-partitioned table to a partitioned one and then
> attach the partition; (b) Add the partition as a separate new table; (c) give an
> error that table types mismatch. For Detach partition, I don't see much
> possibility than giving an error that no such partition exists or something like
> that. Even for the Attach operation, I prefer (c) as the other options don't seem
> logical to me and may add more complexity to this work.
>
> Thoughts?

I also think option (c) makes sense and is same as the latest patch's behavior.

Attach the new version patch set which include the following changes:

- Disallow using non-immutable function when executing ALTER TABLE TYPE/ADD
COLUMN DEFALUT command which could cause table rewrite, Because it could
result in different data between publisher and subscriber. The previous
design of rewrite command is to first replicate the DDL command and then
convert the incoming rewrite insert to updates and replicate them to
subscriber to keep the data consistent, but this doesn't work if the column
type of replica identity column is changed which means we cannot find the old
value in the rewrite inserts.
- Don't log DROP if it's not an original deletion target.
- pg_dump handling of internal event triggers. Now it goes by.
pg_event_trigger's new column (evtisinternal) instead of name parsing.
- Reattach the test_deparser patches and split them into table, index and other
part. (TODO: need to write tests about the replication of index command)
- Fix a bug that the expression used in "alter type using expr" was not schema
qualified during deparsing.
- Restrict Alter (rename) event trigger operations on internally created event
triggers. We allow Alter enable/disable internal event trigger, because it's
necessary to enable the event trigger on replica mode so that it can be fired
in apply worker.
- Address comments from Amit[1][2].
- Address comments from Wang[3].
- Fix typos.

Thanks Shveta for helping address comments and improve the pg_dump handling.

In this version, to make the patch set easier for review and rebase, I didn't
include the patch related to the deparsing and replication of the rest ddl
commands(objects other than table and index). We can add it back later when the
main patches get into shape.

[1]https://www.postgresql.org/message-id/CAA4eK1JJYkwk1rz0O2J6OUK8qb3bZV5P7RwK933DKFkgu56nXQ%40mail.gmail.com
[2]https://www.postgresql.org/message-id/CAA4eK1Lmifb6-JeCiZFQisu2JTVGokvSLFEmx-cchpLLyKc8TA%40mail.gmail.com
[3]https://www.postgresql.org/message-id/OS3PR01MB62759906D280A4EEC42570D89E9A9%40OS3PR01MB6275.jpnprd01.prod.outlook.com

Best Regards,
Hou zj