Re: Support logical replication of DDLs

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@gmail.com> 于2022年11月25日周五 14:18写道：

> On Sun, 20 Nov 2022 at 09:29, vignesh C <vignesh21@gmail.com> wrote:
> >
> > On Fri, 11 Nov 2022 at 11:03, Peter Smith <smithpb2250@gmail.com> wrote:
> > >
> > > On Fri, Nov 11, 2022 at 4:17 PM Peter Smith <smithpb2250@gmail.com>
> wrote:
> > > >
> > > > On Fri, Nov 11, 2022 at 4:09 PM Peter Smith <smithpb2250@gmail.com>
> wrote:
> > > > >
> > > > > On Fri, Nov 11, 2022 at 3:47 PM Peter Smith <smithpb2250@gmail.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
>
>
>