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 <vignes...@gmail.com> 于2022年11月25日周五 14:18写道：
> On Sun, 20 Nov 2022 at 09:29, vignesh C <vignes...@gmail.com> wrote:
> >
> > On Fri, 11 Nov 2022 at 11:03, Peter Smith <smithpb2...@gmail.com> wrote:
> > >
> > > On Fri, Nov 11, 2022 at 4:17 PM Peter Smith <smithpb2...@gmail.com>
> wrote:
> > > >
> > > > On Fri, Nov 11, 2022 at 4:09 PM Peter Smith <smithpb2...@gmail.com>
> wrote:
> > > > >
> > > > > On Fri, Nov 11, 2022 at 3:47 PM Peter Smith <smithpb2...@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
>
>
>
