Re: Support logical replication of DDLs - Mailing list pgsql-hackers

From Amit Kapila
Subject Re: Support logical replication of DDLs
Date
Msg-id CAA4eK1K88SMoBq=DRA4XU-F3FG6qyzCjGMMKsPpcRBPRcrELrw@mail.gmail.com
Whole thread Raw
In response to RE: Support logical replication of DDLs  ("houzj.fnst@fujitsu.com" <houzj.fnst@fujitsu.com>)
Responses Re: Support logical replication of DDLs
List pgsql-hackers
On Tue, Jun 21, 2022 at 5:49 PM houzj.fnst@fujitsu.com
<houzj.fnst@fujitsu.com> wrote:
>
> On Monday, June 20, 2022 11:32 AM houzj.fnst@fujitsu.com <houzj.fnst@fujitsu.com> wrote:
> >
>
> Attach the new version patch set which added support for CREATE/DROP/ATER
> Sequence and CREATE/DROP Schema ddl commands which are provided by Ajin
> Cherian off list.
>

Few questions/comments on v9-0001-Functions-to-deparse-DDL-commands
===========================================================
1.
+/*
+ * Similar to format_type_internal, except we return each bit of information
+ * separately:
...
...
+static void
+format_type_detailed(Oid type_oid, int32 typemod,
+ Oid *nspid, char **typname, char **typemodstr,
+ bool *typarray)

The function mentioned in the comments seems to be changed to
format_type_extended in commit a26116c6. If so, change it accordingly.

2. It is not clear to me why format_type_detailed needs to use
'peculiar_typmod' label and then goto statement? In
format_type_extended, we have similar code but without using the goto
statement, so can't we use a similar way here as well?

3.
format_type_detailed()
{
...
+ typeform->typstorage != 'p')

It is better to replace the constant 'p' with TYPSTORAGE_PLAIN.

4. It seems to me that the handling of some of the built-in types is
different between format_type_detailed and format_type_extended. Can
we add some comments to explain the same?

5.
+static ObjTree *
+deparse_CreateStmt(Oid objectId, Node *parsetree)
{
...
+ tmp = new_objtree_VA("TABLESPACE %{tablespace}I", 0);
+ if (node->tablespacename)
+ append_string_object(tmp, "tablespace", node->tablespacename);
+ else
+ {
+ append_null_object(tmp, "tablespace");
+ append_bool_object(tmp, "present", false);
+ }
+ append_object_object(createStmt, "tablespace", tmp);
...
}

Why do we need to append the objects (tablespace, with clause, etc.)
when they are not present in the actual CREATE TABLE statement? The
reason to ask this is that this makes the string that we want to send
downstream much longer than the actual statement given by the user on
the publisher.

-- 
With Regards,
Amit Kapila.



pgsql-hackers by date:

Previous
From: Yura Sokolov
Date:
Subject: Re: BufferAlloc: don't take two simultaneous locks
Next
From: Marcos Pegoraro
Date:
Subject: better error description on logical replication