public inbox for pgsql-general@postgresql.org  
help / color / mirror / Atom feed
From: Amit Kapila <amit.kapila16@gmail.com>
To: Japin Li <japinli@hotmail.com>
Cc: Zheng Li <zhengli10@gmail.com>
Cc: Alvaro Herrera <alvherre@alvh.no-ip.org>
Cc: Dilip Kumar <dilipbalaut@gmail.com>
Cc: rajesh.rs0541@gmail.com
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: Support logical replication of DDLs
Date: Thu, 7 Apr 2022 15:46:32 +0530
Message-ID: <CAA4eK1J4AekmEKgmfp6e-zZz4M02m7w7uxvC2tjqmjF-LDSGDA@mail.gmail.com> (raw)
In-Reply-To: <MEYP282MB166926E46397CBFC113B4A7EB6189@MEYP282MB1669.AUSP282.PROD.OUTLOOK.COM>
References: <CAAD30ULtoGp8L_GKbV15Wnm+X5r=SE7MOnYHuqBr396m26jJSA@mail.gmail.com>
	<202203162206.7spggyktx63e@alvherre.pgsql>
	<CAAD30UKRUusq8JyyHzAv71=ncN22OE8OkOOyAWvRHW3wXNjyyA@mail.gmail.com>
	<CAAD30UKTp87+kvGZYL3M2Suxq=WEvFUG24ZRT0yT9rqdkP=uMA@mail.gmail.com>
	<MEYP282MB1669863D5C31D7F6A1D996D8B6139@MEYP282MB1669.AUSP282.PROD.OUTLOOK.COM>
	<CAAD30UKc=GiGQzE8H7+Ofo18hwMOfK4qUm_KUyw6c09q4JvA5Q@mail.gmail.com>
	<MEYP282MB16691E383140844437FB0633B6139@MEYP282MB1669.AUSP282.PROD.OUTLOOK.COM>
	<CAAD30U+ZTBXLH0wWsW9+Zu2RECGKeaQNynLs7wKA0o86w8C-fw@mail.gmail.com>
	<MEYP282MB166926E46397CBFC113B4A7EB6189@MEYP282MB1669.AUSP282.PROD.OUTLOOK.COM>

On Wed, Mar 23, 2022 at 10:39 AM Japin Li <japinli@hotmail.com> wrote:
>
> Thanks for fixing this.  I rebase the patchset on current master (383f222119)
> and attach here for further review.
>

Some initial comments:
===================
1.
+/*
+ * Write logical decoding DDL message into XLog.
+ */
+XLogRecPtr
+LogLogicalDDLMessage(const char *prefix, Oid roleoid, const char *message,
+ size_t size, bool transactional)

I don't see anywhere the patch using a non-transactional message. Is
it required for any future usage? The non-transactional behavior has
some known consistency issues, see email [1].

2. For DDL replication, do we need to wait for a consistent point of
snapshot? For DMLs, that point is a convenient point to initialize
replication from, which is why we export a snapshot at that point,
which is used to read normal data. Do we have any similar needs for
DDL replication?

3. The patch seems to be creating an entry in pg_subscription_rel for
'create' message. Do we need some handling on Drop, if not, why? I
think adding some comments for this aspect would make it easier to
follow.

4. The handling related to partition tables seems missing because, on
the subscriber-side, it always creates a relation entry in
pg_subscription_rel which won't work. Check its interaction with
publish_via_partition_root.

[1] - https://www.postgresql.org/message-id/CAA4eK1KAFdQEULk%2B4C%3DieWA5UPSUtf_gtqKsFj9J9f2c%3D8hm4g%40ma...

-- 
With Regards,
Amit Kapila.





reply

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Reply to all the recipients using the --to and --cc options:
  reply via email

  To: pgsql-general@postgresql.org
  Cc: amit.kapila16@gmail.com, japinli@hotmail.com, zhengli10@gmail.com, alvherre@alvh.no-ip.org, dilipbalaut@gmail.com, rajesh.rs0541@gmail.com, pgsql-hackers@lists.postgresql.org
  Subject: Re: Support logical replication of DDLs
  In-Reply-To: <CAA4eK1J4AekmEKgmfp6e-zZz4M02m7w7uxvC2tjqmjF-LDSGDA@mail.gmail.com>

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox