Horton

RE: Support logical replication of DDLs

Zhijie Hou (Fujitsu) <houzj.fnst@fujitsu.com>

View thread
From: "houzj.fnst@fujitsu.com" <houzj.fnst@fujitsu.com>
To: Alvaro Herrera <alvherre@alvh.no-ip.org>, Amit Kapila <amit.kapila16@gmail.com>
Cc: Peter Smith <smithpb2250@gmail.com>, Ajin Cherian <itsajin@gmail.com>, vignesh C <vignesh21@gmail.com>, Zheng Li <zhengli10@gmail.com>, li jie <ggysxcq@gmail.com>, Dilip Kumar <dilipbalaut@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>
Date: 2023-02-16T13:15:28Z
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.

On Wednesday, February 15, 2023 5:51 PM Amit Kapila <amit.kapila16@gmail.com> wrote:
> 
> On Wed, Feb 15, 2023 at 2:02 PM Alvaro Herrera <alvherre@alvh.no-ip.org>
> wrote:
> >
> > On 2023-Feb-15, Peter Smith wrote:
> >
> > > On Thu, Feb 9, 2023 at 8:55 PM Ajin Cherian <itsajin@gmail.com> wrote:
> > > >
> > > > On Fri, Feb 3, 2023 at 11:41 AM Peter Smith <smithpb2250@gmail.com>
> wrote:
> >
> > > > > 3. ExecuteGrantStmt
> > > > >
> > > > > + /* Copy the grantor id needed for DDL deparsing of Grant */
> > > > > + istmt.grantor_uid = grantor;
> > > > >
> > > > > SUGGESTION (comment)
> > > > > Copy the grantor id to the parsetree, needed for DDL deparsing
> > > > > of Grant
> > > >
> > > > didn't change this, as Alvaro said this was not a parsetree.
> > >
> > > Perhaps there is more to do here? Sorry, I did not understand the
> > > details of Alvaro's post [1], but I did not recognize the difference
> > > between ExecuteGrantStmt and ExecSecLabelStmt so it was my
> > > impression either one or both of these places are either wrongly
> > > commented, or maybe are doing something that should not be done.
> >
> > These two cases are different.  In ExecGrantStmt we're adding the
> > grantor OID to the InternalGrant struct, which is not a parse node, so
> > there's no strong reason not to modify it (and also the suggested
> > comment change is factually wrong).  I don't know if the idea is
> > great, but at least I see no strong objection.
> >
> > In the other case, as I said in [1], the patch proposes to edit the
> > parse node to add the grantor, but I think a better idea might be to
> > change the signature to ExecSecLabelStmt(SecLabelStmt *stmt,
> > ObjectAddress *provider) so that the function can set the provider
> > there; and caller passes &secondaryObject, which is the method we
> > adopted for this kind of thing.
> >
> 
> +1, that is a better approach to make the required change in
> ExecSecLabelStmt().

I did some research for this.

The provider seems not a database object, user needs to register a provider via
C ode via register_label_provider. And ObjectAddress only have three
fields(classId, objectId, objectSubId), so it seems hard to set the provider with name to
a ObjectAddress. We cannot get the correct provider name from the catalog as
well because there could be multiple providers for the same db object.

So, if we don't want to modify the parsetree. Maybe we can introduce a function
GetDefaultProvider() which can be used if user doesn't specify the provider in
the DDL command. GetDefaultProvider will return the first provider in the
providers list as is done in ExecSecLabelStmt(). What do you think ?

Best Regards,
Hou zj