Re: Support logical replication of DDLs

From: li jie <ggysxcq(at)gmail(dot)com>
To: vignesh C <vignesh21(at)gmail(dot)com>
Cc: Peter Smith <smithpb2250(at)gmail(dot)com>, Zheng Li <zhengli10(at)gmail(dot)com>, Ajin Cherian <itsajin(at)gmail(dot)com>, Dilip Kumar <dilipbalaut(at)gmail(dot)com>, Alvaro Herrera <alvherre(at)alvh(dot)no-ip(dot)org>, "houzj(dot)fnst(at)fujitsu(dot)com" <houzj(dot)fnst(at)fujitsu(dot)com>, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com>, Masahiko Sawada <sawada(dot)mshk(at)gmail(dot)com>, Japin Li <japinli(at)hotmail(dot)com>, rajesh singarapu <rajesh(dot)rs0541(at)gmail(dot)com>, PostgreSQL Hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Support logical replication of DDLs
Date: 2022-11-25 09:06:44
Message-ID: CAGfChW4gijmx19d0qwJaRctgVRmjN_YvyQTCs48TOFKTF_BGYQ@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-general pgsql-hackers

Hi Developer,

I have been following this patch for a long time.
Recently, I started to try to test it. I found several bugs
here and want to give you feedback.

1. CREATE TABLE LIKE
I found that this case may be repication incorrectly.
You can run the following SQL statement:
```
CREATE TABLE ctlt1 (a text CHECK (length(a) > 2) PRIMARY KEY, b text);
ALTER TABLE ctlt1 ALTER COLUMN a SET STORAGE MAIN;
ALTER TABLE ctlt1 ALTER COLUMN b SET STORAGE EXTERNAL;
CREATE TABLE ctlt1_like (LIKE ctlt1 INCLUDING ALL);
```
The ctlt1_like table will not be able to correct the replication.
I think this is because create table like statement is captured by
the event trigger to a create table statement and multiple alter table
statements.
There are some overlaps between them, and an error is reported when
downstream replication occurs.

2. ALTER TABLE (inherits)
case:
```
CREATE TABLE gtest30 (
a int,
b int GENERATED ALWAYS AS (a * 2) STORED
);
CREATE TABLE gtest30_1 () INHERITS (gtest30);
ALTER TABLE gtest30 ALTER COLUMN b DROP EXPRESSION;
```
After this case is executed in the publication, the following error occurs
in the subscription :

ERROR: column "b" of relation "gtest30" is not a stored generated column
STATEMENT: ALTER TABLE public.gtest30 ALTER COLUMN b DROP EXPRESSION, ALTER
COLUMN b DROP EXPRESSION

Obviously, the column modifications of the inherited table were also
captured,

and then deparse the wrong statement.

I believe that such errors may also occur in other alter table subcmd
scenarios where tables are inherited.

3. ALTER TABLE SET STATISTICS

case:

```
CREATE TABLE test_stat (a int);
ALTER TABLE test_stat ALTER a SET STATISTICS -1;

```

After this case is executed in the publication, the following error occurs
in the subscription :

syntax error at or near "4294967295" at character 60
STATEMENT: ALTER TABLE public.test_stat ALTER COLUMN a SET STATISTICS
4294967295

I guess this should be an overflow in the integer conversion process.

4. json null string coredump

case:

```
CREATE OR REPLACE FUNCTION test_ddl_deparse_full()
RETURNS event_trigger LANGUAGE plpgsql AS
$$
DECLARE
r record;
deparsed_json text;
BEGIN
FOR r IN SELECT * FROM pg_event_trigger_ddl_commands()
LOOP
deparsed_json = ddl_deparse_to_json(r.command);
RAISE NOTICE 'deparsed json: %', deparsed_json;
RAISE NOTICE 're-formed command: %',
ddl_deparse_expand_command(deparsed_json);
END LOOP;
END;
$$;

CREATE EVENT TRIGGER test_ddl_deparse_full
ON ddl_command_end EXECUTE PROCEDURE test_ddl_deparse_full();

CREATE SCHEMA AUTHORIZATION postgres;

```

If the preceding case is executed, coredump occurs,

which is related to null string and can be reproduced.

I hope these feedbacks can be helpful to you.

We sincerely wish you complete the ddl Logical replication feature.

Regards, Adger

vignesh C <vignesh21(at)gmail(dot)com> 于2022年11月25日周五 14:18写道:

> On Sun, 20 Nov 2022 at 09:29, vignesh C <vignesh21(at)gmail(dot)com> wrote:
> >
> > On Fri, 11 Nov 2022 at 11:03, Peter Smith <smithpb2250(at)gmail(dot)com> wrote:
> > >
> > > On Fri, Nov 11, 2022 at 4:17 PM Peter Smith <smithpb2250(at)gmail(dot)com>
> wrote:
> > > >
> > > > On Fri, Nov 11, 2022 at 4:09 PM Peter Smith <smithpb2250(at)gmail(dot)com>
> wrote:
> > > > >
> > > > > On Fri, Nov 11, 2022 at 3:47 PM Peter Smith <smithpb2250(at)gmail(dot)com>
> wrote:
> > > > > >
> > > > > > Here are more review comments for the v32-0001 file ddl_deparse.c
> > > > > >
> > > > > > *** NOTE - my review post became too big, so I split it into
> smaller parts.
> > > > >
> > > >
> > >
> > > THIS IS PART 4 OF 4.
> > >
> > > =======
> > >
> > > src/backend/commands/ddl_deparse.c
> >
> > Thanks for the comments, the attached v39 patch has the changes for the
> same.
>
> One comment:
> While fixing review comments, I found that default syntax is not
> handled for create domain:
> + /*
> + * Verbose syntax
> + *
> + * CREATE DOMAIN %{identity}D AS %{type}T %{not_null}s
> %{constraints}s
> + * %{collation}s
> + */
> + createDomain = new_objtree("CREATE");
> +
> + append_object_object(createDomain,
> + "DOMAIN %{identity}D AS",
> +
> new_objtree_for_qualname_id(TypeRelationId,
> +
> objectId));
> + append_object_object(createDomain,
> + "%{type}T",
> +
> new_objtree_for_type(typForm->typbasetype, typForm->typtypmod));
>
> Regards,
> Vignesh
>
>
>

In response to

Responses

Browse pgsql-general by date

  From Date Subject
Next Message Kirk Wolak 2022-11-25 09:11:37 Re: About row locking ordering
Previous Message vignesh C 2022-11-25 06:17:59 Re: Support logical replication of DDLs

Browse pgsql-hackers by date

  From Date Subject
Next Message John Naylor 2022-11-25 09:47:20 Re: [PoC] Improve dead tuple storage for lazy vacuum
Previous Message Etsuro Fujita 2022-11-25 08:59:22 Re: postgres_fdw: batch inserts vs. before row triggers