Re: Support logical replication of DDLs

From: Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>
To: Peter Smith <smithpb2250(at)gmail(dot)com>
Cc: Ajin Cherian <itsajin(at)gmail(dot)com>, vignesh C <vignesh21(at)gmail(dot)com>, Zheng Li <zhengli10(at)gmail(dot)com>, li jie <ggysxcq(at)gmail(dot)com>, Dilip Kumar <dilipbalaut(at)gmail(dot)com>, "houzj(dot)fnst(at)fujitsu(dot)com" <houzj(dot)fnst(at)fujitsu(dot)com>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, Japin Li <japinli(at)hotmail(dot)com>, rajesh singarapu <rajesh(dot)rs0541(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Support logical replication of DDLs
Date: 2023-02-15 08:32:01
Message-ID: 20230215083201.6rdeixnkihotwu7a@alvherre.pgsql
Views: Raw Message | Whole Thread | Download mbox | Resend email
Lists: pgsql-general pgsql-hackers

On 2023-Feb-15, Peter Smith wrote:

> On Thu, Feb 9, 2023 at 8:55 PM Ajin Cherian <itsajin(at)gmail(dot)com> wrote:
> >
> > On Fri, Feb 3, 2023 at 11:41 AM Peter Smith <smithpb2250(at)gmail(dot)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.


Álvaro Herrera 48°01'N 7°57'E —

In response to


Browse pgsql-general by date

  From Date Subject
Next Message Laurenz Albe 2023-02-15 08:46:23 Re: Multi-column index: Which column order
Previous Message Amit Kapila 2023-02-15 06:50:59 Re: Support logical replication of DDLs

Browse pgsql-hackers by date

  From Date Subject
Next Message Maxim Ivanov 2023-02-15 08:57:31 Use of additional index columns in rows filtering
Previous Message Drouvot, Bertrand 2023-02-15 08:21:48 Re: Reconcile stats in find_tabstat_entry() and get rid of PgStat_BackendFunctionEntry