Re: Support logical replication of DDLs

Please find the attached patch set which addresses the following comments.

On Mon, Dec 19, 2022 at 5:02 AM li jie <ggysxcq(at)gmail(dot)com> wrote:
>
> I have presented some comments below:
>
> 1. AT_AddColumn
>
> > + tmpobj = new_objtree_VA("ADD %{objtype}s %{definition}s", 3,
> [ IF NOT EXISTS ] is missing here.
Fixed.

> 2. AT_DropColumn
> > + tmpobj = new_objtree_VA("DROP %{objtype}s %{column}I", 3,
> [ IF EXISTS ] is missing here.
Fixed.

> 3. AT_DropConstraint
> > + tmpobj = new_objtree_VA("DROP CONSTRAINT %{constraint}I", 2,
> [ IF EXISTS ] is missing here.
Fixed.

> 4. AT_DetachPartition
> > + tmpobj = new_objtree_VA("DETACH PARTITION %{partition_identity}D", 2,
> [ CONCURRENTLY | FINALIZE ] is missing here.
Fixed.

> 5. deparse_CreateSeqStmt
> > + ret = new_objtree_VA("CREATE %{persistence}s SEQUENCE %{identity}D %{definition: }s", 3,
> [ IF NOT EXISTS ] is missing here.
Fixed.

> 6. deparse_IndexStmt
> > + ret = new_objtree_VA("CREATE %{unique}s INDEX %{concurrently}s %{if_not_exists}s %{name}I ON %{table}D USING %{index_am}s (%{definition}s)", 7,
> [ INCLUDE ] and [ ONLY ] are missing here.
Fixed and added a test case in
src/test/modules/test_ddl_deparse_regress/sql/create_table.sql.

> 7. deparse_RuleStmt
> > + foreach(cell, actions)
> > + list = lappend(list, new_string_object(lfirst(cell)));
>
> if (actions == NIL)
> list = lappend(list, new_string_object("NOTHING"));
> else
> {
> foreach(cell, actions)
> list = lappend(list, new_string_object(lfirst(cell)));
> }
Fixed.

> 8. AT_AddIndexConstraint
> > + tmpobj = new_objtree_VA("ADD CONSTRAINT %{name}I %{constraint_type}s USING INDEX %{index_name}I %{deferrable}s %{init_deferred}s", 6,
> > + "type", ObjTypeString, "add constraint using index",
> > + "name", ObjTypeString, get_constraint_name(constrOid),
> > + "constraint_type", ObjTypeString,
> > + istmt->deferrable ? "DEFERRABLE" : "NOT DEFERRABLE",
>
> "constraint_type", ObjTypeString,
> istmt->primary ? "PRIMARY KEY" : "UNIQUE",
Fixed.

Regards,
Zane