MIME-Version: 1.0
References: 
 <CALDaNm3zSxK1g0UTo4RjPR3JEc-vyPzANOnKEKjiW88xtUUdzA@mail.gmail.com>
 <CAJpy0uCGnRQnpNVQaApRxqN=9t5VpHRk5b+Mfdf3cqCEW=s+4A@mail.gmail.com>
 <OSZPR01MB63102C42A24D59FACF6D9CD6FD4A9@OSZPR01MB6310.jpnprd01.prod.outlook.com>
 <CAJpy0uCbwqWj+p_yj1AHyiufzAUv_H29qOaztAXxFoTqZ9WcAw@mail.gmail.com>
 <OSZPR01MB6310E0509F229AF2A9414BB1FD499@OSZPR01MB6310.jpnprd01.prod.outlook.com>
 <CAJpy0uBwCZCniPR6vh26L+wpSf4xzUN8omUa9DzF-x1CAud_xA@mail.gmail.com>
 <CAA4eK1LOr+2O+_pWKTaa0y9vbW6whfm-8-fuBvnS6OBiaR+7TA@mail.gmail.com>
In-Reply-To: 
 <CAA4eK1LOr+2O+_pWKTaa0y9vbW6whfm-8-fuBvnS6OBiaR+7TA@mail.gmail.com>
From: Amit Kapila <amit.kapila16@gmail.com>
Date: Thu, 8 Jun 2023 10:35:54 +0530
Message-ID: 
 <CAA4eK1LF3EaCSj5iqO0oT1k3ew7YnQbbKEgbzORDAdvdtd+r7w@mail.gmail.com>
Subject: Re: Support logical replication of DDLs
To: shveta malik <shveta.malik@gmail.com>
Cc: "Yu Shi (Fujitsu)" <shiy.fnst@fujitsu.com>,
 vignesh C <vignesh21@gmail.com>,
	"Zhijie Hou (Fujitsu)" <houzj.fnst@fujitsu.com>,
 Ajin Cherian <itsajin@gmail.com>,
	"Wei Wang (Fujitsu)" <wangw.fnst@fujitsu.com>,
 Runqi Tian <runqidev@gmail.com>,
	Peter Smith <smithpb2250@gmail.com>, Tom Lane <tgl@sss.pgh.pa.us>,
 li jie <ggysxcq@gmail.com>,
	Dilip Kumar <dilipbalaut@gmail.com>,
 Alvaro Herrera <alvherre@alvh.no-ip.org>,
	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>
Content-Type: text/plain; charset="UTF-8"
Content-Transfer-Encoding: quoted-printable
Archived-At: 
 <https://www.postgresql.org/message-id/CAA4eK1LF3EaCSj5iqO0oT1k3ew7YnQbbKEgbzORDAdvdtd%2Br7w%40mail.gmail.com>
Precedence: bulk

On Tue, Jun 6, 2023 at 4:26=E2=80=AFPM Amit Kapila <amit.kapila16@gmail.com=
> wrote:
>
>
> Few assorted comments:
> =3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D
>

Few comments on 0008* patch:
=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=3D=
=3D=3D=3D=3D=3D
1. After 0008*, deparse_CreateStmt(), forms dpcontext before forming
identity whereas it is required only after it. It may not matter
practically but it is better to do the work when it is required. Also,
before 0008, it appears to be formed after identity, so not sure if
the change in 0008 is intentional, if so, then please let me know the
reason, and may be it is better to add a comment for the same.

2. Similarly, what is need to separately do insert_identity_object()
in deparse_CreateStmt() instead of directly doing something like
new_objtree_for_qualname() as we are doing in 0001 patch? For this, I
guess you need objtype handling in new_jsonb_VA().

3.
/*
* It will be of array type for multi-columns table, so lets begin
* an arrayobject. deparse_TableElems_ToJsonb() will add elements
* to it.
*/
pushJsonbValue(&state, WJB_BEGIN_ARRAY, NULL);

deparse_TableElems_ToJsonb(state, relation, node->tableElts, dpcontext,
   false, /* not typed table */
   false); /* not composite */
deparse_Constraints_ToJsonb(state, objectId);

pushJsonbValue(&state, WJB_END_ARRAY, NULL);

This part of the code is repeated in deparse_CreateStmt(). Can we move
this to a separate function?

4.
 * Note we ignore constraints in the parse node here; they are extracted fr=
om
 * system catalogs instead.
 */

static void
deparse_TableElems_ToJsonb(JsonbParseState *state, Relation relation,

An extra empty line between the comments end and function appears unnecessa=
ry.

5.
+ /* creat name and type elements for column */

/creat/create

--=20
With Regards,
Amit Kapila.