Horton

Re: Support logical replication of DDLs

Ajin Cherian <itsajin@gmail.com>

View thread
From: Ajin Cherian <itsajin@gmail.com>
To: Alvaro Herrera <alvherre@alvh.no-ip.org>
Cc: 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>, "houzj.fnst@fujitsu.com" <houzj.fnst@fujitsu.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: 2023-02-16T07:54:15Z
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 Thu, Feb 9, 2023 at 9:55 PM Alvaro Herrera <alvherre@alvh.no-ip.org> wrote:
>
> I happened to notice that MINVFUNC in 0003 displays like this
>       "fmt": "MINVFUNC==%{type}T",
> in some cases; this appears in the JSON that's emitted by the regression
> tests at some point.  How can we detect this kind of thing, so that
> these mistakes become self-evident?  I thought the intention of the
> regress module was to run the deparsed code, so the syntax error should
> have become obvious.
>
> ...
>
> Oh, I see the problem.  There are two 'fmt' lines for that clause (and
> many others), one of which is used when the clause is not present.  So
> there's never a syntax error, because this one never expands other than
> to empty.
>
> AFAICS this defeats the purpose of the 'present' field.  I mean, if the
> clause is never to deparse, then why have it there in the first place?
> If we want to have it, then it has to be correct.
>
>
> I think we should design the code to avoid the repetition, because that
> has an inherent risk of typo bugs and such.  Maybe we should set forth
> policy that each 'fmt' string should appear in the source code only
> once.  So instead of this
>
> +       /* MINVFUNC */
> +       if (OidIsValid(agg->aggminvtransfn))
> +               tmp = new_objtree_VA("MINVFUNC=%{type}T", 1,
> +                                    "type", ObjTypeObject,
> +                                    new_objtree_for_qualname_id(ProcedureRelationId,
> +                                                                agg->aggminvtransfn));
> +       else
> +       {
> +               tmp = new_objtree("MINVFUNC==%{type}T");
> +               append_bool_object(tmp, "present", false);
> +       }
>
> we would have something like
>
>    tmp = new_objtree("MINVFUNC=%{type}T");
>    if (OidIsValid(agg->aggminvtransfn))
>    {
>       append_bool_object(tmp, "present", true);
>       append...(tmp, "type", new_objtree_for_qualname_id(ProcedureRelationId, agg->aggminvtransfn));
>    }
>    else
>    {
>       append_bool_object(tmp, "present", false);
>    }
>
I have addressed this by changing similar occurrences accordingly for
instances with  bool "present" - true and false.
This has also changed the test output and I have changed the test patch as well.
Mu changes are in patch 0003 and patch 0004.

regards,
Ajin Cherian
Fujitsu Australia