public inbox for pgsql-general@postgresql.org
help / color / mirror / Atom feedFrom: Peter Smith <smithpb2250@gmail.com>
To: Ajin Cherian <itsajin@gmail.com>
Cc: vignesh C <vignesh21@gmail.com>
Cc: Zheng Li <zhengli10@gmail.com>
Cc: li jie <ggysxcq@gmail.com>
Cc: Dilip Kumar <dilipbalaut@gmail.com>
Cc: Alvaro Herrera <alvherre@alvh.no-ip.org>
Cc: houzj.fnst@fujitsu.com <houzj.fnst@fujitsu.com>
Cc: Amit Kapila <amit.kapila16@gmail.com>
Cc: Masahiko Sawada <sawada.mshk@gmail.com>
Cc: Japin Li <japinli@hotmail.com>
Cc: rajesh singarapu <rajesh.rs0541@gmail.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: Support logical replication of DDLs
Date: Wed, 15 Feb 2023 15:32:53 +1100
Message-ID: <CAHut+PusvDXTJa2drUPzePBBW0i9we8yns8Z4DU4b7JVs6Ew1Q@mail.gmail.com> (raw)
In-Reply-To: <CAFPTHDbZzCeYMPJn0iFuD_ggpY-0ZHfVBHgQ9VJ6v4dF59xang@mail.gmail.com>
References: <CAAD30UKg8rXeGM8Oy_MAmxKBL_K5DiHXdeNF=hUefcu1C_6VfQ@mail.gmail.com>
<20221006171601.6um4ey5idm4h62vf@alvherre.pgsql>
<CAAD30UJh+LcUND+zg_A5dbQ_Bi=m_n7qUfKrOwAx2v4jBKWvKQ@mail.gmail.com>
<CAFPTHDbVujj6C3TdMCmoBXovpc4=5Ow3i5M1_HNhmnqdiA5qSA@mail.gmail.com>
<CAAD30UKCCpMTZd4WSSxYu-qRAEFd+0kjXTgje+EXHdo1JTkz0g@mail.gmail.com>
<CALDaNm3F9GvQ9bPrxobhqkUKP3HmrRZGCU3EX0xt3=Ef0-Reaw@mail.gmail.com>
<CAFPTHDY2O42ouQFMeEbPt51CWQ=zyzYhgK6B9basyd5PLaOv0A@mail.gmail.com>
<CAFPTHDZmS2PrOyJx8OAe+Nt-Fx4-GZetJatqCEJhDNXA2orwCg@mail.gmail.com>
<CAFiTN-tWkJW-JEGANkK+O3KXUjv_Yb5Tb+r83ujbognHG_brTA@mail.gmail.com>
<CALDaNm08gZq9a7xnsbaJMmHmi29_kbEuyShHHfxAKLXPh6btWQ@mail.gmail.com>
<CAFPTHDbGRpXOF-j+oemm=xkzjrh-ZnqPV7SMS_+QBPYdUwhWxg@mail.gmail.com>
<CAAD30UL=wV7pvdwJ6w5UQUc3urHs6W2vnomK2MKUyek3rzsKcA@mail.gmail.com>
<CAHut+Psm+9q++y8b70QTeBeZiYcfDNtc_obxSwhxukp0MFhnRA@mail.gmail.com>
<CAHut+PsO0dwWoB4B6L3Ucd6D8ckgdXY2Sd3JK7F_3wLsXU7ZAg@mail.gmail.com>
<CAHut+Puxo_kq2toicNK_BQdeccK3REGW-Xv8tVauFvTNku6V-w@mail.gmail.com>
<CAHut+Pu6H6hY0JJNNRCRmFpM_3817z=0xjm-_ibP+cL85pBOpQ@mail.gmail.com>
<CALDaNm3q_2Qfo4XbFkBT8mPz_ALS2MyuR=isP7_8Pz27JnLHiw@mail.gmail.com>
<CAGfChW5Qbb8WKznzVqp+cyTqkzLt_vEeSe14td14et6HjnH7qw@mail.gmail.com>
<CAAD30UKwHmLo1YyjB1j_vCEth0+OS9hkLr9Nrrsw5uwEuOs=+g@mail.gmail.com>
<CALDaNm2V6YL6H4P9ZT95Ua_RDJaeDTUf6V0UDfrz4_vxhM5pMg@mail.gmail.com>
<CAAD30ULrZB-RNmuD3NMr1jGNUt15ZpPgFdFRX53HbcAB76hefw@mail.gmail.com>
<CAAD30UL0EmSGk58eZxxYzFTQ5EqSpX6d+OcX5MuE0j0r2ZgyDA@mail.gmail.com>
<CAAD30ULAVzCnNOsnHwZWoeWfaYr3yDtM-7TnkMJf9HTrW7HVCQ@mail.gmail.com>
<CAAD30U+fqaaD6533-eiaWVHpUaBNBCEvqyXOT_ow1B--Aa_jOQ@mail.gmail.com>
<CALDaNm0mnkHXnQ7zMDxY6=ZtWn8A1iKLKGhEtqRVC9c3gc8YCw@mail.gmail.com>
<CAFPTHDY_9_xd8JEckB=Au2ALPnx+hfmn9ztutVw4aVwZxLhrNQ@mail.gmail.com>
<CALDaNm3YEFNwFkkbzS1J8NiHKX6LWQ_h3CA+CPtkUqAZgD8q+Q@mail.gmail.com>
<CAFPTHDY_4koZ135_Bub_=HPBxpG1u+XBEv8zspwXW7rAQps6eA@mail.gmail.com>
<CALDaNm0dOwbPVrkqw9OLN=TVFdo5aroUNwdjG5SAcKQg93-g0w@mail.gmail.com>
<CAHut+PtH9S+3-TGPa4_uipBP-4-Kksx-FrEkg9ZtK0FphVRA5A@mail.gmail.com>
<CAFPTHDbZzCeYMPJn0iFuD_ggpY-0ZHfVBHgQ9VJ6v4dF59xang@mail.gmail.com>
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:
> >
> > Here are some review comments for patch v63-0001.
> >
> > ======
> > src/backend/catalog/aclchk.c
> >
> > 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.
> > ======
> > src/backend/utils/adt/regproc.c
> >
> > 9.
> > +
> > +/*
> > + * Append the parenthesized arguments of the given pg_proc row into the output
> > + * buffer. force_qualify indicates whether to schema-qualify type names
> > + * regardless of visibility.
> > + */
> > +static void
> > +format_procedure_args_internal(Form_pg_proc procform, StringInfo buf,
> > + bool force_qualify)
> > +{
> > + int i;
> > + char* (*func[2])(Oid) = {format_type_be, format_type_be_qualified};
> > +
> > + appendStringInfoChar(buf, '(');
> > + for (i = 0; i < procform->pronargs; i++)
> > + {
> > + Oid thisargtype = procform->proargtypes.values[i];
> > + char *argtype = NULL;
> > +
> > + if (i > 0)
> > + appendStringInfoChar(buf, ',');
> > +
> > + argtype = func[force_qualify](thisargtype);
> > + appendStringInfoString(buf, argtype);
> > + pfree(argtype);
> > + }
> > + appendStringInfoChar(buf, ')');
> > +}
> >
> > 9a.
> > Assign argtype = NULL looks redundant because it will always be
> > overwritten anyhow.
> >
>
> changed this.
>
> > ~
> >
> > 9b.
> > I understand why this function was put here beside the other static
> > functions in "Support Routines" but IMO it really belongs nearby (i.e.
> > directly above) the only caller (format_procedure_args). Keeping both
> > those functional together will improve the readability of both, and
> > will also remove the need to have the static forward declaration.
> >
There was no reply for 9b. Was it accidentally overlooked, or just
chose not to do it?
------
[1] https://www.postgresql.org/message-id/20230213090752.27ftbb6byiw3qcbl%40alvherre.pgsql
Kind Regards,
Peter Smith.
Fujitsu Australia
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: smithpb2250@gmail.com, itsajin@gmail.com, vignesh21@gmail.com, zhengli10@gmail.com, ggysxcq@gmail.com, dilipbalaut@gmail.com, alvherre@alvh.no-ip.org, houzj.fnst@fujitsu.com, amit.kapila16@gmail.com, sawada.mshk@gmail.com, japinli@hotmail.com, rajesh.rs0541@gmail.com, pgsql-hackers@lists.postgresql.org
Subject: Re: Support logical replication of DDLs
In-Reply-To: <CAHut+PusvDXTJa2drUPzePBBW0i9we8yns8Z4DU4b7JVs6Ew1Q@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