Horton

Re: Support logical replication of DDLs

Amit Kapila <amit.kapila16@gmail.com>

View thread
From: Amit Kapila <amit.kapila16@gmail.com>
To: Masahiko Sawada <sawada.mshk@gmail.com>
Cc: Ajin Cherian <itsajin@gmail.com>, Peter Smith <smithpb2250@gmail.com>, vignesh C <vignesh21@gmail.com>, Zheng Li <zhengli10@gmail.com>, li jie <ggysxcq@gmail.com>, Dilip Kumar <dilipbalaut@gmail.com>, Alvaro Herrera <alvherre@alvh.no-ip.org>, "houzj.fnst@fujitsu.com" <houzj.fnst@fujitsu.com>, Japin Li <japinli@hotmail.com>, rajesh singarapu <rajesh.rs0541@gmail.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2023-02-15T05:57:04Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Add a run_as_owner option to subscriptions.

  2. Refactor pgoutput_change().

  3. Print the correct aliases for DML target tables in ruleutils.

  4. Fix object identity string for transforms

  5. Add grantable MAINTAIN privilege and pg_maintain role.

  6. Get rid of recursion-marker values in enum AlterTableType

  7. Release cache tuple when no longer needed

  8. Empty search_path in logical replication apply worker and walsender.

  9. Refactor format_type APIs to be more modular

  10. Use wrappers of PG_DETOAST_DATUM_PACKED() more.

On Fri, Feb 10, 2023 at 8:23 PM Masahiko Sawada <sawada.mshk@gmail.com> wrote:
>
> On Thu, Feb 9, 2023 at 6:55 PM Ajin Cherian <itsajin@gmail.com> wrote:
> >
> (v67)
>
> I have some questions about adding the infrastructure for DDL deparsing.
>
> Apart from the changes made by 0001 patch to add infrastructure for
> DDL deparsing, 0002 patch seems to add some variables that are not
> used in 0002 patch:
>
> @@ -2055,6 +2055,7 @@ typedef struct AlterTableStmt
>         List       *cmds;                       /* list of subcommands */
>         ObjectType      objtype;                /* type of object */
>         bool            missing_ok;             /* skip error if table
> missing */
> +       bool        table_like;         /* internally generated for
> TableLikeClause */
>  } AlterTableStmt;
>
> @@ -39,6 +40,7 @@ typedef struct CollectedATSubcmd
>  {
>         ObjectAddress address;          /* affected column,
> constraint, index, ... */
>         Node       *parsetree;
> +       char       *usingexpr;
>  } CollectedATSubcmd;
>
>  typedef struct CollectedCommand
> @@ -62,6 +64,7 @@ typedef struct CollectedCommand
>                 {
>                         Oid                     objectId;
>                         Oid                     classId;
> +                       bool            rewrite;
>                         List       *subcmds;
>                 }                       alterTable;
>
> These three variables are used in 0006 patch.
>

Hmm, then it should be better to move these to 0006 patch.

> Looking at 0006 patch (Support DDL replication), it seems to me that
> it includes not only DDL replication support but also changes for the
> event trigger. For instance, the patch adds
> EventTriggerAlterTypeStart() and EventTriggerAlterTypeEnd(). If these
> changes are required for DDL deparse, should we include them in 0001
> patch? Perhaps the same is true for
> EventTriggerCollectCreatePublication() and friends. IIUC the DDL
> deparse and DDL replication are independent features, so I think 0006
> patch should not include any changes for DDL deparse infrastructure.
>

AFAICS, these are required for DDL replication, so not sure moving
them would be of any help.

> Also, 0003 and 0006 patches introduce SCT_Create/AlterPublication and
> change DDL deparse so that it deparse CREATE/ALTER PUBLICATION in a
> different way from other simple commands. Is there any reason for
> that? I mean, since EventTriggerCollectCreatePublication() collects
> the information from the parse tree, I wonder if we could use
> SCT_Simple for them.
>

Right, I think we should try a bit harder to avoid adding new
CollectedCommandTypes. Is there a reason that we can't retrieve the
additional information required to form the command string from system
catalogs or parsetree?

-- 
With Regards,
Amit Kapila.