On 2020-04-07 08:44, Amit Langote wrote:
I updated the patch to make the following changes:
* Rewrote the tests to match in style with those committed yesterday
* Renamed all variables that had pubasroot in it to have pubviaroot
instead to match the publication parameter
* Updated pg_publication catalog documentation
Thanks. I have some further questions:
The change in nodeModifyTable.c to add CheckValidResultRel() is unclear.
It doesn't seem to do anything, and it's not clear how it's related to
this patch.
The changes in GetRelationPublications() are confusing to me:
+ if (published_rels)
+ {
+ num = list_length(result);
+ for (i = 0; i < num; i++)
+ *published_rels = lappend_oid(*published_rels, relid);
+ }
This adds relid to the output list "num" times, where num is the number
of publications found. Shouldn't "i" be used in the loop somehow?
Similarly later in the function.
The descriptions of the new fields in RelationSyncEntry don't seem to
match the code accurately, or at least it's confusing.
replicate_as_relid is always filled in with an ancestor, even if
pubviaroot is not set.
I think the pubviaroot field is actually not necessary. We only need
replicate_as_relid.
There is a markup typo in logical-replication.sgml:
<xref linkend=="sql-createpublication"/>
In pg_dump, you missed updating a branch for an older version. See
attached patch.
Also attached a patch to rephrase the psql output a bit to make it not
so long.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From 572549aa23d4e3fa2d1abc0733d33f28cb692c40 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Tue, 7 Apr 2020 10:56:11 +0200
Subject: [PATCH 1/2] fixup! Allow publishing partition changes via ancestors
---
src/bin/pg_dump/pg_dump.c | 4 ++--
1 file changed, 2 insertions(+), 2 deletions(-)
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 4d03608596..c579227b19 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -3891,14 +3891,14 @@ getPublications(Archive *fout)
appendPQExpBuffer(query,
"SELECT p.tableoid, p.oid,
p.pubname, "
"(%s p.pubowner) AS rolname, "
- "p.puballtables, p.pubinsert,
p.pubupdate, p.pubdelete, p.pubtruncate, false as pubviaroot "
+ "p.puballtables, p.pubinsert,
p.pubupdate, p.pubdelete, p.pubtruncate, false AS pubviaroot "
"FROM pg_publication p",
username_subquery);
else
appendPQExpBuffer(query,
"SELECT p.tableoid, p.oid,
p.pubname, "
"(%s p.pubowner) AS rolname, "
- "p.puballtables, p.pubinsert,
p.pubupdate, p.pubdelete, false AS pubtruncate "
+ "p.puballtables, p.pubinsert,
p.pubupdate, p.pubdelete, false AS pubtruncate, false AS pubviaroot "
"FROM pg_publication p",
username_subquery);
--
2.26.0
From 3e12e0ff53e4d1aa5b78b6b7fa181e79ca280ef0 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pe...@eisentraut.org>
Date: Tue, 7 Apr 2020 10:58:55 +0200
Subject: [PATCH 2/2] fixup! Allow publishing partition changes via ancestors
---
src/bin/psql/describe.c | 4 +-
src/test/regress/expected/publication.out | 72 +++++++++++------------
2 files changed, 38 insertions(+), 38 deletions(-)
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index c731ed6322..f05e914b4d 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -5741,7 +5741,7 @@ listPublications(const char *pattern)
if (pset.sversion >= 130000)
appendPQExpBuffer(&buf,
",\n pubviaroot AS \"%s\"",
- gettext_noop("Publishes Using
Root Schema"));
+ gettext_noop("Via root"));
appendPQExpBufferStr(&buf,
"\nFROM
pg_catalog.pg_publication\n");
@@ -5874,7 +5874,7 @@ describePublications(const char *pattern)
if (has_pubtruncate)
printTableAddHeader(&cont, gettext_noop("Truncates"),
true, align);
if (has_pubviaroot)
- printTableAddHeader(&cont, gettext_noop("Publishes
Using Root Schema"), true, align);
+ printTableAddHeader(&cont, gettext_noop("Via root"),
true, align);
printTableAddCell(&cont, PQgetvalue(res, i, 2), false, false);
printTableAddCell(&cont, PQgetvalue(res, i, 3), false, false);
diff --git a/src/test/regress/expected/publication.out
b/src/test/regress/expected/publication.out
index 5b4e73d91b..63d6ab7a4e 100644
--- a/src/test/regress/expected/publication.out
+++ b/src/test/regress/expected/publication.out
@@ -28,18 +28,18 @@ ERROR: unrecognized "publish" value: "cluster"
CREATE PUBLICATION testpub_xxx WITH (publish_via_partition_root = 'true',
publish_via_partition_root = '0');
ERROR: conflicting or redundant options
\dRp
- List of publications
- Name | Owner | All tables | Inserts |
Updates | Deletes | Truncates | Publishes Using Root Schema
---------------------+--------------------------+------------+---------+---------+---------+-----------+-----------------------------
+ List of publications
+ Name | Owner | All tables | Inserts |
Updates | Deletes | Truncates | Via root
+--------------------+--------------------------+------------+---------+---------+---------+-----------+----------
testpib_ins_trunct | regress_publication_user | f | t | f
| f | f | f
testpub_default | regress_publication_user | f | f | t
| f | f | f
(2 rows)
ALTER PUBLICATION testpub_default SET (publish = 'insert, update, delete');
\dRp
- List of publications
- Name | Owner | All tables | Inserts |
Updates | Deletes | Truncates | Publishes Using Root Schema
---------------------+--------------------------+------------+---------+---------+---------+-----------+-----------------------------
+ List of publications
+ Name | Owner | All tables | Inserts |
Updates | Deletes | Truncates | Via root
+--------------------+--------------------------+------------+---------+---------+---------+-----------+----------
testpib_ins_trunct | regress_publication_user | f | t | f
| f | f | f
testpub_default | regress_publication_user | f | t | t
| t | f | f
(2 rows)
@@ -85,9 +85,9 @@ Publications:
"testpub_foralltables"
\dRp+ testpub_foralltables
- Publication testpub_foralltables
- Owner | All tables | Inserts | Updates | Deletes |
Truncates | Publishes Using Root Schema
---------------------------+------------+---------+---------+---------+-----------+-----------------------------
+ Publication testpub_foralltables
+ Owner | All tables | Inserts | Updates | Deletes |
Truncates | Via root
+--------------------------+------------+---------+---------+---------+-----------+----------
regress_publication_user | t | t | t | f | f
| f
(1 row)
@@ -100,18 +100,18 @@ CREATE PUBLICATION testpub3 FOR TABLE testpub_tbl3;
CREATE PUBLICATION testpub4 FOR TABLE ONLY testpub_tbl3;
RESET client_min_messages;
\dRp+ testpub3
- Publication testpub3
- Owner | All tables | Inserts | Updates | Deletes |
Truncates | Publishes Using Root Schema
---------------------------+------------+---------+---------+---------+-----------+-----------------------------
+ Publication testpub3
+ Owner | All tables | Inserts | Updates | Deletes |
Truncates | Via root
+--------------------------+------------+---------+---------+---------+-----------+----------
regress_publication_user | f | t | t | t | t
| f
Tables:
"public.testpub_tbl3"
"public.testpub_tbl3a"
\dRp+ testpub4
- Publication testpub4
- Owner | All tables | Inserts | Updates | Deletes |
Truncates | Publishes Using Root Schema
---------------------------+------------+---------+---------+---------+-----------+-----------------------------
+ Publication testpub4
+ Owner | All tables | Inserts | Updates | Deletes |
Truncates | Via root
+--------------------------+------------+---------+---------+---------+-----------+----------
regress_publication_user | f | t | t | t | t
| f
Tables:
"public.testpub_tbl3"
@@ -131,9 +131,9 @@ ALTER TABLE testpub_parted ATTACH PARTITION testpub_parted1
FOR VALUES IN (1);
-- only parent is listed as being in publication, not the partition
ALTER PUBLICATION testpub_forparted ADD TABLE testpub_parted;
\dRp+ testpub_forparted
- Publication testpub_forparted
- Owner | All tables | Inserts | Updates | Deletes |
Truncates | Publishes Using Root Schema
---------------------------+------------+---------+---------+---------+-----------+-----------------------------
+ Publication testpub_forparted
+ Owner | All tables | Inserts | Updates | Deletes |
Truncates | Via root
+--------------------------+------------+---------+---------+---------+-----------+----------
regress_publication_user | f | t | t | t | t
| f
Tables:
"public.testpub_parted"
@@ -147,9 +147,9 @@ ALTER TABLE testpub_parted DETACH PARTITION testpub_parted1;
UPDATE testpub_parted1 SET a = 1;
ALTER PUBLICATION testpub_forparted SET (publish_via_partition_root = true);
\dRp+ testpub_forparted
- Publication testpub_forparted
- Owner | All tables | Inserts | Updates | Deletes |
Truncates | Publishes Using Root Schema
---------------------------+------------+---------+---------+---------+-----------+-----------------------------
+ Publication testpub_forparted
+ Owner | All tables | Inserts | Updates | Deletes |
Truncates | Via root
+--------------------------+------------+---------+---------+---------+-----------+----------
regress_publication_user | f | t | t | t | t
| t
Tables:
"public.testpub_parted"
@@ -170,9 +170,9 @@ ERROR: relation "testpub_tbl1" is already member of
publication "testpub_fortbl
CREATE PUBLICATION testpub_fortbl FOR TABLE testpub_tbl1;
ERROR: publication "testpub_fortbl" already exists
\dRp+ testpub_fortbl
- Publication testpub_fortbl
- Owner | All tables | Inserts | Updates | Deletes |
Truncates | Publishes Using Root Schema
---------------------------+------------+---------+---------+---------+-----------+-----------------------------
+ Publication testpub_fortbl
+ Owner | All tables | Inserts | Updates | Deletes |
Truncates | Via root
+--------------------------+------------+---------+---------+---------+-----------+----------
regress_publication_user | f | t | t | t | t
| f
Tables:
"pub_test.testpub_nopk"
@@ -211,9 +211,9 @@ Publications:
"testpub_fortbl"
\dRp+ testpub_default
- Publication testpub_default
- Owner | All tables | Inserts | Updates | Deletes |
Truncates | Publishes Using Root Schema
---------------------------+------------+---------+---------+---------+-----------+-----------------------------
+ Publication testpub_default
+ Owner | All tables | Inserts | Updates | Deletes |
Truncates | Via root
+--------------------------+------------+---------+---------+---------+-----------+----------
regress_publication_user | f | t | t | t | f
| f
Tables:
"pub_test.testpub_nopk"
@@ -258,9 +258,9 @@ DROP TABLE testpub_parted;
DROP VIEW testpub_view;
DROP TABLE testpub_tbl1;
\dRp+ testpub_default
- Publication testpub_default
- Owner | All tables | Inserts | Updates | Deletes |
Truncates | Publishes Using Root Schema
---------------------------+------------+---------+---------+---------+-----------+-----------------------------
+ Publication testpub_default
+ Owner | All tables | Inserts | Updates | Deletes |
Truncates | Via root
+--------------------------+------------+---------+---------+---------+-----------+----------
regress_publication_user | f | t | t | t | f
| f
(1 row)
@@ -271,9 +271,9 @@ ERROR: must be owner of publication testpub_default
RESET ROLE;
ALTER PUBLICATION testpub_default RENAME TO testpub_foo;
\dRp testpub_foo
- List of publications
- Name | Owner | All tables | Inserts | Updates |
Deletes | Truncates | Publishes Using Root Schema
--------------+--------------------------+------------+---------+---------+---------+-----------+-----------------------------
+ List of publications
+ Name | Owner | All tables | Inserts | Updates |
Deletes | Truncates | Via root
+-------------+--------------------------+------------+---------+---------+---------+-----------+----------
testpub_foo | regress_publication_user | f | t | t | t
| f | f
(1 row)
@@ -281,9 +281,9 @@ ALTER PUBLICATION testpub_default RENAME TO testpub_foo;
ALTER PUBLICATION testpub_foo RENAME TO testpub_default;
ALTER PUBLICATION testpub_default OWNER TO regress_publication_user2;
\dRp testpub_default
- List of publications
- Name | Owner | All tables | Inserts | Updates
| Deletes | Truncates | Publishes Using Root Schema
------------------+---------------------------+------------+---------+---------+---------+-----------+-----------------------------
+ List of publications
+ Name | Owner | All tables | Inserts | Updates
| Deletes | Truncates | Via root
+-----------------+---------------------------+------------+---------+---------+---------+-----------+----------
testpub_default | regress_publication_user2 | f | t | t
| t | f | f
(1 row)
--
2.26.0
