On Tue, 20 Dec 2022 at 02:56, Zheng Li <zhengli10@gmail.com> wrote:
>
> Attaching v54 patch set which adds support for:
> - CREATE/ALTER/DROP FOREIGN TABLE
> - IMPORT FOREIGN SCHEMA, this is captured and replicated as individual
> CREATE FOREIGN TABLE command for each FOREIGN TABLE in the SCHEMA.
>
> Note:
> DROP FOREIGN TABLE ft1 also generates:
> DROP type IF EXISTS ft1;
> and
> DROP type IF EXISTS ft1[];
> These two dropped objects are also captured and replicated to the
> subscriber along with the DROP FOREIGN TABLE command which aren't
> necessary.
>
> In addition, the patch fixed a bug in deparse_CreateSchemaStmt which
> causes a quoted identifier to fail in replication, for example:
> CREATE SCHEMA "S 2"; is replicated as CREATE SCHEMA S 2, which will
> fail during apply.
> Fix is to change %{name}s -> %{name}I in deparse_CreateSchemaStmt.
>
> On Mon, Dec 19, 2022 at 5:02 AM li jie <ggysxcq@gmail.com> wrote:
> >
> > I have presented some comments below:
>
> Thanks for the feedback. I'll look into these.
>
> > 1. AT_AddColumn
> >
> > > + tmpobj = new_objtree_VA("ADD %{objtype}s %{definition}s", 3,
> > [ IF NOT EXISTS ] is missing here.
> >
> ......
> >
> > 9. regress test
> >
> > The test patch is very useful.
> > I see that the sql case in test_ddl_deparse_regress is similar to the
> > one in test_ddl_deparse.
> > Why don't we merge the test cases in test_ddl_deparse_regress into
> > test_ddl_deparse,
> > as the sql case can be completely reused with the sql files in test_ddl_deparse?
> > I believe this will make the tests more comprehensive and reduce redundancy.
>
> We have set up test_ddl_deparse_regress as a new module initially to
> not interfere with what's being tested by test_ddl_deparse. We could
> merge the two test modules if it turns out that we can expand on
> test_ddl_deparse to achieve our testing goals and to add more test
> cases without breaking what's currently being tested by
> test_ddl_deparse.
I have handled most of the comments for [1] in the v55 version patch
attached. I will handle the pending comments in the upcoming version
and reply to it.
[1] - https://www.postgresql.org/message-id/20221207122041.hbfj4hen3ibhdzgn%40alvherre.pgsql
Regards,
Vignesh
