Horton

Re: Support logical replication of DDLs

vignesh C <vignesh21@gmail.com>

View thread
From: vignesh C <vignesh21@gmail.com>
To: Dilip Kumar <dilipbalaut@gmail.com>
Cc: Ajin Cherian <itsajin@gmail.com>, Zheng Li <zhengli10@gmail.com>, Alvaro Herrera <alvherre@alvh.no-ip.org>, "houzj.fnst@fujitsu.com" <houzj.fnst@fujitsu.com>, Peter Smith <smithpb2250@gmail.com>, Amit Kapila <amit.kapila16@gmail.com>, Masahiko Sawada <sawada.mshk@gmail.com>, Japin Li <japinli@hotmail.com>, rajesh singarapu <rajesh.rs0541@gmail.com>, PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Date: 2022-10-19T05:48:05Z
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.

Attachments

On Wed, 12 Oct 2022 at 11:38, Dilip Kumar <dilipbalaut@gmail.com> wrote:
>
> On Tue, Oct 11, 2022 at 7:00 PM Ajin Cherian <itsajin@gmail.com> wrote:
> >
>
> I was going through 0001 patch and I have a few comments/suggestions.
>
> 1.
>
> @@ -6001,7 +6001,7 @@ getObjectIdentityParts(const ObjectAddress *object,
>                  transformType = format_type_be_qualified(transform->trftype);
>                  transformLang = get_language_name(transform->trflang, false);
>
> -                appendStringInfo(&buffer, "for %s on language %s",
> +                appendStringInfo(&buffer, "for %s language %s",
>                                   transformType,
>                                   transformLang);
>
>
> How is this change related to this patch?

This change is required for ddl of transform commands, we have added a
note for the same in the patch:
Removed an undesirable 'on' from the identity string for TRANSFORM in
getObjectIdentityParts. This is needed to make deparse of DROP
TRANSFORM command work since 'on' is not present in the current DROP
TRANSFORM syntax. For example, the correct syntax is
drop transform trf for int language sql;
instead of
drop transform trf for int on language sql;

> 2.
> +typedef struct ObjTree
> +{
> +    slist_head    params;
> +    int            numParams;
> +    StringInfo    fmtinfo;
> +    bool        present;
> +} ObjTree;
> +
> +typedef struct ObjElem
> +{
> +    char       *name;
> +    ObjType        objtype;
> +
> +    union
> +    {
> +        bool        boolean;
> +        char       *string;
> +        int64        integer;
> +        float8        flt;
> +        ObjTree       *object;
> +        List       *array;
> +    } value;
> +    slist_node    node;
> +} ObjElem;
>
> It would be good to explain these structure members from readability pov.

Modified

> 3.
>
> +
> +bool verbose = true;
> +
>
> I do not understand the usage of such global variables.  Even if it is
> required, add some comments to explain the purpose of it.

Modified

>
> 4.
> +/*
> + * Given a CollectedCommand, return a JSON representation of it.
> + *
> + * The command is expanded fully, so that there are no ambiguities even in the
> + * face of search_path changes.
> + */
> +Datum
> +ddl_deparse_to_json(PG_FUNCTION_ARGS)
> +{
>
> It will be nice to have a test case for this utility function.

We are discussing how to test in a separate thread at [1]. We will
implement accordingly once it is concluded. This comment will be
handled at that time.
[1] - https://www.postgresql.org/message-id/flat/CAH8n8_jMTunxxtP4L-3tc%3DGNamg%3Dmg1X%3DtgHr9CqqjjzFLwQng%40mail.gmail.com

This patch also includes changes for replication of:
CREATE/ALTER/DROP STATISTICS
and pgindent fixes for the ddl replication code.
Thanks for the comments, the attached v30 patch has the changes for the same.

Regards,
Vignesh