public inbox for [email protected]
help / color / mirror / Atom feedFrom: [email protected] <[email protected]>
To: 'vignesh C' <[email protected]>
Cc: Amit Kapila <[email protected]>
Cc: Euler Taveira <[email protected]>
Cc: Peter Eisentraut <[email protected]>
Cc: PostgreSQL Hackers <[email protected]>
Subject: RE: Skipping schema changes in publication
Date: Thu, 28 Apr 2022 11:20:52 +0000
Message-ID: <TYCPR01MB8373047EB3D48C7FE7109428EDFD9@TYCPR01MB8373.jpnprd01.prod.outlook.com> (raw)
In-Reply-To: <CALDaNm3bpBwQSFMCAXuKm-UFNAD4J27M4Ey+o=_87NRW9YBcJg@mail.gmail.com>
References: <CALDaNm3=JrucjhiiwsYQw5-PGtBHFONa6F7hhWCXMsGvh=tamA@mail.gmail.com>
<CALDaNm1G0=0J2O+6pMStKuWUjxJJT5vbO-5jiUqzYVZM_sJ2+w@mail.gmail.com>
<CALDaNm3Rc2KxGzLy-4qxFS+MWS6GN-ROVoPEb1MFA53W3iAqog@mail.gmail.com>
<[email protected]>
<[email protected]>
<CAA4eK1KikfJMVggZ_aNWPio7zrdObVdidG3SZVWEZ_LOU=vXkg@mail.gmail.com>
<CALDaNm2KJRmQMcpOn0QPoKm6ET93Z-PSmbwwdm8O0Lr3eWqkTw@mail.gmail.com>
<TYCPR01MB8373FE72A4CDBE7A5E8886EBEDFB9@TYCPR01MB8373.jpnprd01.prod.outlook.com>
<CALDaNm3bpBwQSFMCAXuKm-UFNAD4J27M4Ey+o=_87NRW9YBcJg@mail.gmail.com>
On Wednesday, April 27, 2022 9:50 PM vignesh C <[email protected]> wrote:
> Thanks for the comments, the attached v3 patch has the changes for the same.
Hi
Thank you for updating the patch. Several minor comments on v3.
(1) commit message
The new syntax allows specifying schemas. For example:
CREATE PUBLICATION pub1 FOR ALL TABLES EXCEPT TABLE t1,t2;
OR
ALTER PUBLICATION pub1 ADD EXCEPT TABLE t1,t2;
We have above sentence, but it looks better
to make the description a bit more accurate.
Kindly change
From :
"The new syntax allows specifying schemas"
To :
"The new syntax allows specifying excluded relations"
Also, kindly change "OR" to "or",
because this description is not syntax.
(2) publication_add_relation
@@ -396,6 +400,9 @@ publication_add_relation(Oid pubid, PublicationRelInfo *pri,
ObjectIdGetDatum(pubid);
values[Anum_pg_publication_rel_prrelid - 1] =
ObjectIdGetDatum(relid);
+ values[Anum_pg_publication_rel_prexcept - 1] =
+ BoolGetDatum(pri->except);
+
/* Add qualifications, if available */
It would be better to remove the blank line,
because with this change, we'll have two blank
lines in a row.
(3) pg_dump.h & pg_dump_sort.c
@@ -80,6 +80,7 @@ typedef enum
DO_REFRESH_MATVIEW,
DO_POLICY,
DO_PUBLICATION,
+ DO_PUBLICATION_EXCEPT_REL,
DO_PUBLICATION_REL,
DO_PUBLICATION_TABLE_IN_SCHEMA,
DO_SUBSCRIPTION
@@ -90,6 +90,7 @@ enum dbObjectTypePriorities
PRIO_FK_CONSTRAINT,
PRIO_POLICY,
PRIO_PUBLICATION,
+ PRIO_PUBLICATION_EXCEPT_REL,
PRIO_PUBLICATION_REL,
PRIO_PUBLICATION_TABLE_IN_SCHEMA,
PRIO_SUBSCRIPTION,
@@ -144,6 +145,7 @@ static const int dbObjectTypePriority[] =
PRIO_REFRESH_MATVIEW, /* DO_REFRESH_MATVIEW */
PRIO_POLICY, /* DO_POLICY */
PRIO_PUBLICATION, /* DO_PUBLICATION */
+ PRIO_PUBLICATION_EXCEPT_REL, /* DO_PUBLICATION_EXCEPT_REL */
PRIO_PUBLICATION_REL, /* DO_PUBLICATION_REL */
PRIO_PUBLICATION_TABLE_IN_SCHEMA, /* DO_PUBLICATION_TABLE_IN_SCHEMA */
PRIO_SUBSCRIPTION /* DO_SUBSCRIPTION */
How about having similar order between
pg_dump.h and pg_dump_sort.c, like
we'll add DO_PUBLICATION_EXCEPT_REL
after DO_PUBLICATION_REL in pg_dump.h ?
(4) GetAllTablesPublicationRelations
+ /*
+ * pg_publication_rel and pg_publication_namespace will only have except
+ * tables in case of all tables publication, no need to pass except flag
+ * to get the relations.
+ */
+ List *exceptpubtablelist = GetPublicationRelations(pubid, PUBLICATION_PART_ALL);
+
There is one unnecessary space in a comment
"...pg_publication_namespace will only have...". Kindly remove it.
Then, how about diving the variable declaration and
the insertion of the return value of GetPublicationRelations ?
That might be aligned with other places in this file.
(5) GetTopMostAncestorInPublication
@@ -302,8 +303,9 @@ GetTopMostAncestorInPublication(Oid puboid, List *ancestors, int *ancestor_level
foreach(lc, ancestors)
{
Oid ancestor = lfirst_oid(lc);
- List *apubids = GetRelationPublications(ancestor);
+ List *apubids = GetRelationPublications(ancestor, false);
List *aschemaPubids = NIL;
+ List *aexceptpubids;
level++;
@@ -317,7 +319,9 @@ GetTopMostAncestorInPublication(Oid puboid, List *ancestors, int *ancestor_level
else
{
aschemaPubids = GetSchemaPublications(get_rel_namespace(ancestor));
- if (list_member_oid(aschemaPubids, puboid))
+ aexceptpubids = GetRelationPublications(ancestor, true);
+ if (list_member_oid(aschemaPubids, puboid) ||
+ (puballtables && !list_member_oid(aexceptpubids, puboid)))
{
topmost_relid = ancestor;
It seems we forgot to call list_free for "aexceptpubids".
Best Regards,
Takamichi Osumi
view thread (377+ messages) latest in thread
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: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: RE: Skipping schema changes in publication
In-Reply-To: <TYCPR01MB8373047EB3D48C7FE7109428EDFD9@TYCPR01MB8373.jpnprd01.prod.outlook.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