Horton

Re: Support logical replication of DDLs

Tom Lane <tgl@sss.pgh.pa.us>

View thread
From: Tom Lane <tgl@sss.pgh.pa.us>
To: vignesh C <vignesh21@gmail.com>
Cc: Ajin Cherian <itsajin@gmail.com>, "houzj.fnst@fujitsu.com" <houzj.fnst@fujitsu.com>, "wangw.fnst@fujitsu.com" <wangw.fnst@fujitsu.com>, Runqi Tian <runqidev@gmail.com>, Peter Smith <smithpb2250@gmail.com>, li jie <ggysxcq@gmail.com>, Dilip Kumar <dilipbalaut@gmail.com>, Alvaro Herrera <alvherre@alvh.no-ip.org>, Amit Kapila <amit.kapila16@gmail.com>, Masahiko Sawada <sawada.mshk@gmail.com>, Japin Li <japinli@hotmail.com>, rajesh singarapu <rajesh.rs0541@gmail.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>, Zheng Li <zhengli10@gmail.com>
Date: 2023-03-26T21:21:58Z
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.

vignesh C <vignesh21@gmail.com> writes:
> [ YA patch set ]

I spent some time looking through this thread to try to get a sense
of the state of things, and I came away quite depressed.  The patchset
has ballooned to over 2MB, which is a couple orders of magnitude
larger than anyone could hope to meaningfully review from scratch.
Despite that, it seems that there are fundamental semantics issues
remaining, not to mention clear-and-present security dangers, not
to mention TODO comments all over the code.

I'm also less than sold on the technical details, specifically
the notion of "let's translate utility parse trees into JSON and
send that down the wire".  You can probably make that work for now,
but I wonder if it will be any more robust against cross-version
changes than just shipping the outfuncs.c representation.  (Perhaps
it can be made more robust than the raw parse trees, but I see no
evidence that anyone's thought much about how.)

And TBH, I don't think that I quite believe the premise in the
first place.  The whole point of using logical rather than physical
replication is that the subscriber installation(s) aren't exactly like
the publisher.  Given that, how can we expect that automated DDL
replication is going to do the right thing often enough to be a useful
tool rather than a disastrous foot-gun?  The more you expand the scope
of what gets replicated, the worse that problem becomes --- for
example, I don't buy for one second that "let's replicate roles"
is a credible solution for the problems that come from the roles
not being the same on publisher and subscriber.

I'm not sure how we get from here to a committable and useful feature,
but I don't think we're close to that yet, and I'm not sure that minor
iterations on a 2MB patchset will accomplish much.  I'm afraid that
a whole lot of work is going to end up going down the drain, which
would be a shame because surely there are use-cases here.

I suggest taking a couple of steps back from the minutiae of the
patch, and spending some hard effort thinking about how the thing
would be controlled in a useful fashion (that is, a real design for
the filtering that was mentioned at the very outset), and about the
security issues, and about how we could get to a committable patch.

If you ask me, a committable initial patch could be at most about a
tenth the size of what's here, which means that the functionality
goals for the first version have to be stripped way back.

[ Now, where did I put my flameproof vest? ]

			regards, tom lane