public inbox for pgsql-general@postgresql.org
help / color / mirror / Atom feedFrom: li jie <ggysxcq@gmail.com>
To: vignesh C <vignesh21@gmail.com>
Cc: Peter Smith <smithpb2250@gmail.com>
Cc: Zheng Li <zhengli10@gmail.com>
Cc: Ajin Cherian <itsajin@gmail.com>
Cc: Dilip Kumar <dilipbalaut@gmail.com>
Cc: Alvaro Herrera <alvherre@alvh.no-ip.org>
Cc: houzj.fnst@fujitsu.com <houzj.fnst@fujitsu.com>
Cc: Amit Kapila <amit.kapila16@gmail.com>
Cc: Masahiko Sawada <sawada.mshk@gmail.com>
Cc: Japin Li <japinli@hotmail.com>
Cc: rajesh singarapu <rajesh.rs0541@gmail.com>
Cc: PostgreSQL Hackers <pgsql-hackers@lists.postgresql.org>
Subject: Re: Support logical replication of DDLs
Date: Mon, 19 Dec 2022 18:02:17 +0800
Message-ID: <CAGfChW5Qbb8WKznzVqp+cyTqkzLt_vEeSe14td14et6HjnH7qw@mail.gmail.com> (raw)
In-Reply-To: <CALDaNm3q_2Qfo4XbFkBT8mPz_ALS2MyuR=isP7_8Pz27JnLHiw@mail.gmail.com>
References: <CAAD30UKg8rXeGM8Oy_MAmxKBL_K5DiHXdeNF=hUefcu1C_6VfQ@mail.gmail.com>
<20221006171601.6um4ey5idm4h62vf@alvherre.pgsql>
<CAAD30UJh+LcUND+zg_A5dbQ_Bi=m_n7qUfKrOwAx2v4jBKWvKQ@mail.gmail.com>
<CAFPTHDbVujj6C3TdMCmoBXovpc4=5Ow3i5M1_HNhmnqdiA5qSA@mail.gmail.com>
<CAAD30UKCCpMTZd4WSSxYu-qRAEFd+0kjXTgje+EXHdo1JTkz0g@mail.gmail.com>
<CALDaNm3F9GvQ9bPrxobhqkUKP3HmrRZGCU3EX0xt3=Ef0-Reaw@mail.gmail.com>
<CAFPTHDY2O42ouQFMeEbPt51CWQ=zyzYhgK6B9basyd5PLaOv0A@mail.gmail.com>
<CAFPTHDZmS2PrOyJx8OAe+Nt-Fx4-GZetJatqCEJhDNXA2orwCg@mail.gmail.com>
<CAFiTN-tWkJW-JEGANkK+O3KXUjv_Yb5Tb+r83ujbognHG_brTA@mail.gmail.com>
<CALDaNm08gZq9a7xnsbaJMmHmi29_kbEuyShHHfxAKLXPh6btWQ@mail.gmail.com>
<CAFPTHDbGRpXOF-j+oemm=xkzjrh-ZnqPV7SMS_+QBPYdUwhWxg@mail.gmail.com>
<CAAD30UL=wV7pvdwJ6w5UQUc3urHs6W2vnomK2MKUyek3rzsKcA@mail.gmail.com>
<CAHut+Psm+9q++y8b70QTeBeZiYcfDNtc_obxSwhxukp0MFhnRA@mail.gmail.com>
<CAHut+PsO0dwWoB4B6L3Ucd6D8ckgdXY2Sd3JK7F_3wLsXU7ZAg@mail.gmail.com>
<CAHut+Puxo_kq2toicNK_BQdeccK3REGW-Xv8tVauFvTNku6V-w@mail.gmail.com>
<CAHut+Pu6H6hY0JJNNRCRmFpM_3817z=0xjm-_ibP+cL85pBOpQ@mail.gmail.com>
<CALDaNm3q_2Qfo4XbFkBT8mPz_ALS2MyuR=isP7_8Pz27JnLHiw@mail.gmail.com>
I have presented some comments below:
1. AT_AddColumn
> + tmpobj = new_objtree_VA("ADD %{objtype}s %{definition}s", 3,
[ IF NOT EXISTS ] is missing here.
2. AT_DropColumn
> + tmpobj = new_objtree_VA("DROP %{objtype}s %{column}I", 3,
[ IF EXISTS ] is missing here.
3. AT_DropConstraint
> + tmpobj = new_objtree_VA("DROP CONSTRAINT %{constraint}I", 2,
[ IF EXISTS ] is missing here.
4. AT_DetachPartition
> + tmpobj = new_objtree_VA("DETACH PARTITION %{partition_identity}D", 2,
[ CONCURRENTLY | FINALIZE ] is missing here.
5. deparse_CreateSeqStmt
> + ret = new_objtree_VA("CREATE %{persistence}s SEQUENCE %{identity}D %{definition: }s", 3,
[ IF NOT EXISTS ] is missing here.
6. deparse_IndexStmt
> + ret = new_objtree_VA("CREATE %{unique}s INDEX %{concurrently}s %{if_not_exists}s %{name}I ON %{table}D USING %{index_am}s (%{definition}s)", 7,
[ INCLUDE ] and [ ONLY ] are missing here.
7. deparse_RuleStmt
> + foreach(cell, actions)
> + list = lappend(list, new_string_object(lfirst(cell)));
if (actions == NIL)
list = lappend(list, new_string_object("NOTHING"));
else
{
foreach(cell, actions)
list = lappend(list, new_string_object(lfirst(cell)));
}
8. AT_AddIndexConstraint
> + tmpobj = new_objtree_VA("ADD CONSTRAINT %{name}I %{constraint_type}s USING INDEX %{index_name}I %{deferrable}s %{init_deferred}s", 6,
> + "type", ObjTypeString, "add constraint using index",
> + "name", ObjTypeString, get_constraint_name(constrOid),
> + "constraint_type", ObjTypeString,
> + istmt->deferrable ? "DEFERRABLE" : "NOT DEFERRABLE",
"constraint_type", ObjTypeString,
istmt->primary ? "PRIMARY KEY" : "UNIQUE",
9. regress test
Zheng Li <zhengli10@gmail.com> 于2022年12月12日周一 12:58写道：
>
> Hi,
>
> Attached please find the DDL deparser testing module in the v45-0007
> patch, this testing module is written by Runqi Tian in [1] with minor
> modification from myself. I think we can
> start adding more tests to the module now that we're getting close to
> finish implementing the DDL deparser.
>
> This testing module ddl_deparse_regress aims to achieve the following
> four testing goals for the DDL deparser:
> 1. Test that the generated JSON blob is expected using SQL tests.
> 2. Test that the re-formed DDL command is expected using SQL tests.
> 3. Test that the re-formed DDL command has the same effect as the
> original command
> by comparing the results of pg_dump, using the SQL tests in 1 and 2.
> 4. Test that any new DDL syntax is handled by the DDL deparser by
> capturing and deparsing
> DDL commands ran by pg_regress.
>
> 1 and 2 is tested with SQL tests, by comparing the deparsed JSON blob
> and the re-formed command.
> 3 is tested with TAP framework in t/001_compare_dumped_results.pl
> 4 is tested with TAP framework and pg_regress in 002_regress_tests.pl,
> the execution is currently commented out because it will fail due
> unimplemented commands in the DDL deparser.
>
> [1] https://www.postgresql.org/message-id/flat/CAH8n8_jMTunxxtP4L-3tc%3DGNamg%3Dmg1X%3DtgHr9CqqjjzFLwQng...
>
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.
Regards,
li jie
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-general@postgresql.org
Cc: ggysxcq@gmail.com, vignesh21@gmail.com, smithpb2250@gmail.com, zhengli10@gmail.com, itsajin@gmail.com, dilipbalaut@gmail.com, alvherre@alvh.no-ip.org, houzj.fnst@fujitsu.com, amit.kapila16@gmail.com, sawada.mshk@gmail.com, japinli@hotmail.com, rajesh.rs0541@gmail.com, pgsql-hackers@lists.postgresql.org
Subject: Re: Support logical replication of DDLs
In-Reply-To: <CAGfChW5Qbb8WKznzVqp+cyTqkzLt_vEeSe14td14et6HjnH7qw@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox