Review comments for v31-0001.
(I tried to give only new comments, but there might be some overlap
with comments I previously made for v30-0001)
======
src/backend/catalog/pg_publication.c
1.
+
+ if (publish_generated_columns_given)
+ {
+ values[Anum_pg_publication_pubgencolumns - 1] =
BoolGetDatum(publish_generated_columns);
+ replaces[Anum_pg_publication_pubgencolumns - 1] = true;
+ }
nit - unnecessary whitespace above here.
======
src/backend/replication/pgoutput/pgoutput.c
2. prepare_all_columns_bms
+ /* Iterate the cols until generated columns are found. */
+ cols = bms_add_member(cols, i + 1);
How does the comment relate to the statement that follows it?
~~~
3.
+ * Skip generated column if pubgencolumns option was not
+ * specified.
nit - /pubgencolumns option/publish_generated_columns parameter/
======
src/bin/pg_dump/pg_dump.c
4.
getPublications:
nit - /i_pub_gencolumns/i_pubgencols/ (it's the same information but simpler)
======
src/bin/pg_dump/pg_dump.h
5.
+ bool pubgencolumns;
} PublicationInfo;
nit - /pubgencolumns/pubgencols/ (it's the same information but simpler)
======
vsrc/bin/psql/describe.c
6.
bool has_pubviaroot;
+ bool has_pubgencol;
nit - /has_pubgencol/has_pubgencols/ (plural consistency)
======
src/include/catalog/pg_publication.h
7.
+ /* true if generated columns data should be published */
+ bool pubgencolumns;
} FormData_pg_publication;
nit - /pubgencolumns/pubgencols/ (it's the same information but simpler)
~~~
8.
+ bool pubgencolumns;
PublicationActions pubactions;
} Publication;
nit - /pubgencolumns/pubgencols/ (it's the same information but simpler)
======
src/test/regress/sql/publication.sql
9.
+-- Test the publication with or without 'PUBLISH_GENERATED_COLUMNS' parameter
+SET client_min_messages = 'ERROR';
+CREATE PUBLICATION pub1 FOR ALL TABLES WITH (PUBLISH_GENERATED_COLUMNS=1);
+\dRp+ pub1
+
+CREATE PUBLICATION pub2 FOR ALL TABLES WITH (PUBLISH_GENERATED_COLUMNS=0);
+\dRp+ pub2
9a.
nit - Use lowercase for the parameters.
~
9b.
nit - Fix the comment to say what the test is actually doing:
"Test the publication 'publish_generated_columns' parameter enabled or disabled"
======
src/test/subscription/t/031_column_list.pl
10.
Later I think you should add another test here to cover the scenario
that I was discussing with Sawada-San -- e.g. when there are 2
publications for the same table subscribed by just 1 subscription but
having different values of the 'publish_generated_columns' for the
publications.
======
Kind Regards,
Peter Smith.
Fujitsu Australia
diff --git a/doc/src/sgml/ddl.sgml b/doc/src/sgml/ddl.sgml
index 2e7804e..cca54bc 100644
--- a/doc/src/sgml/ddl.sgml
+++ b/doc/src/sgml/ddl.sgml
@@ -515,8 +515,8 @@ CREATE TABLE people (
<listitem>
<para>
Generated columns may be skipped during logical replication according to
the
- <command>CREATE PUBLICATION</command> option
- <link
linkend="sql-createpublication-params-with-include-generated-columns">
+ <command>CREATE PUBLICATION</command> parameter
+ <link
linkend="sql-createpublication-params-with-publish-generated-columns">
<literal>publish_generated_columns</literal></link>.
</para>
</listitem>
diff --git a/doc/src/sgml/ref/create_publication.sgml
b/doc/src/sgml/ref/create_publication.sgml
index e133dc3..cd20bd4 100644
--- a/doc/src/sgml/ref/create_publication.sgml
+++ b/doc/src/sgml/ref/create_publication.sgml
@@ -223,7 +223,7 @@ CREATE PUBLICATION <replaceable
class="parameter">name</replaceable>
</listitem>
</varlistentry>
- <varlistentry
id="sql-createpublication-params-with-include-generated-columns">
+ <varlistentry
id="sql-createpublication-params-with-publish-generated-columns">
<term><literal>publish_generated_columns</literal>
(<type>boolean</type>)</term>
<listitem>
<para>
@@ -231,14 +231,6 @@ CREATE PUBLICATION <replaceable
class="parameter">name</replaceable>
associated with the publication should be replicated.
The default is <literal>false</literal>.
</para>
- <para>
- This option is only available for replicating generated column data
from the publisher
- to a regular, non-generated column in the subscriber.
- </para>
- <para>
- This parameter can only be set <literal>true</literal> if
<literal>copy_data</literal> is
- set to <literal>false</literal>.
- </para>
</listitem>
</varlistentry>
diff --git a/src/backend/catalog/pg_publication.c
b/src/backend/catalog/pg_publication.c
index 272b6a1..7ebb851 100644
--- a/src/backend/catalog/pg_publication.c
+++ b/src/backend/catalog/pg_publication.c
@@ -999,7 +999,7 @@ GetPublication(Oid pubid)
pub->pubactions.pubdelete = pubform->pubdelete;
pub->pubactions.pubtruncate = pubform->pubtruncate;
pub->pubviaroot = pubform->pubviaroot;
- pub->pubgencolumns = pubform->pubgencolumns;
+ pub->pubgencols = pubform->pubgencols;
ReleaseSysCache(tup);
@@ -1211,7 +1211,7 @@ pg_get_publication_tables(PG_FUNCTION_ARGS)
if (att->attisdropped)
continue;
- if (att->attgenerated && !pub->pubgencolumns)
+ if (att->attgenerated && !pub->pubgencols)
continue;
attnums[nattnums++] = att->attnum;
diff --git a/src/backend/commands/publicationcmds.c
b/src/backend/commands/publicationcmds.c
index 6242a09..0129db1 100644
--- a/src/backend/commands/publicationcmds.c
+++ b/src/backend/commands/publicationcmds.c
@@ -808,7 +808,7 @@ CreatePublication(ParseState *pstate, CreatePublicationStmt
*stmt)
BoolGetDatum(pubactions.pubtruncate);
values[Anum_pg_publication_pubviaroot - 1] =
BoolGetDatum(publish_via_partition_root);
- values[Anum_pg_publication_pubgencolumns - 1] =
+ values[Anum_pg_publication_pubgencols - 1] =
BoolGetDatum(publish_generated_columns);
tup = heap_form_tuple(RelationGetDescr(rel), values, nulls);
@@ -1018,11 +1018,10 @@ AlterPublicationOptions(ParseState *pstate,
AlterPublicationStmt *stmt,
replaces[Anum_pg_publication_pubviaroot - 1] = true;
}
-
if (publish_generated_columns_given)
{
- values[Anum_pg_publication_pubgencolumns - 1] =
BoolGetDatum(publish_generated_columns);
- replaces[Anum_pg_publication_pubgencolumns - 1] = true;
+ values[Anum_pg_publication_pubgencols - 1] =
BoolGetDatum(publish_generated_columns);
+ replaces[Anum_pg_publication_pubgencols - 1] = true;
}
tup = heap_modify_tuple(tup, RelationGetDescr(rel), values, nulls,
diff --git a/src/backend/replication/pgoutput/pgoutput.c
b/src/backend/replication/pgoutput/pgoutput.c
index 5a39d4f..c91ae3c 100644
--- a/src/backend/replication/pgoutput/pgoutput.c
+++ b/src/backend/replication/pgoutput/pgoutput.c
@@ -1076,7 +1076,7 @@ pgoutput_column_list_init(PGOutputData *data, List
*publications,
* then it is treated the same as if there are no column lists
(even
* if other publications have a list).
*/
- if (!pub->alltables || !pub->pubgencolumns)
+ if (!pub->alltables || !pub->pubgencols)
{
bool pub_no_list = true;
@@ -1100,7 +1100,7 @@ pgoutput_column_list_init(PGOutputData *data, List
*publications,
}
/* Build the column list bitmap in the per-entry
context. */
- if (!pub_no_list || !pub->pubgencolumns) /* when
not null */
+ if (!pub_no_list || !pub->pubgencols) /* when not
null */
{
int i;
int nliveatts = 0;
@@ -1123,10 +1123,10 @@ pgoutput_column_list_init(PGOutputData *data, List
*publications,
continue;
/*
- * Skip generated column if
pubgencolumns option was not
- * specified.
+ * Skip generated column if
publish_generated_columns parameter
+ * was not specified.
*/
- if (pub_no_list && att->attgenerated &&
!pub->pubgencolumns)
+ if (pub_no_list && att->attgenerated &&
!pub->pubgencols)
cols = bms_del_member(cols, i +
1);
nliveatts++;
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index 06fda22..64fb898 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -4282,7 +4282,7 @@ getPublications(Archive *fout)
int i_pubdelete;
int i_pubtruncate;
int i_pubviaroot;
- int i_pubgencolumns;
+ int i_pubgencols;
int i,
ntups;
@@ -4298,7 +4298,7 @@ getPublications(Archive *fout)
appendPQExpBufferStr(query,
"SELECT p.tableoid,
p.oid, p.pubname, "
"p.pubowner, "
- "p.puballtables,
p.pubinsert, p.pubupdate, p.pubdelete, p.pubtruncate, p.pubviaroot,
p.pubgencolumns "
+ "p.puballtables,
p.pubinsert, p.pubupdate, p.pubdelete, p.pubtruncate, p.pubviaroot,
p.pubgencols "
"FROM pg_publication
p");
else if (fout->remoteVersion >= 130000)
appendPQExpBufferStr(query,
@@ -4333,7 +4333,7 @@ getPublications(Archive *fout)
i_pubdelete = PQfnumber(res, "pubdelete");
i_pubtruncate = PQfnumber(res, "pubtruncate");
i_pubviaroot = PQfnumber(res, "pubviaroot");
- i_pubgencolumns = PQfnumber(res, "pubgencolumns");
+ i_pubgencols = PQfnumber(res, "pubgencols");
pubinfo = pg_malloc(ntups * sizeof(PublicationInfo));
@@ -4358,8 +4358,8 @@ getPublications(Archive *fout)
(strcmp(PQgetvalue(res, i, i_pubtruncate), "t") == 0);
pubinfo[i].pubviaroot =
(strcmp(PQgetvalue(res, i, i_pubviaroot), "t") == 0);
- pubinfo[i].pubgencolumns =
- (strcmp(PQgetvalue(res, i, i_pubgencolumns), "t") == 0);
+ pubinfo[i].pubgencols =
+ (strcmp(PQgetvalue(res, i, i_pubgencols), "t") == 0);
/* Decide whether we want to dump it */
selectDumpableObject(&(pubinfo[i].dobj), fout);
@@ -4439,7 +4439,7 @@ dumpPublication(Archive *fout, const PublicationInfo
*pubinfo)
if (pubinfo->pubviaroot)
appendPQExpBufferStr(query, ", publish_via_partition_root =
true");
- if (pubinfo->pubgencolumns)
+ if (pubinfo->pubgencols)
appendPQExpBufferStr(query, ", publish_generated_columns =
true");
appendPQExpBufferStr(query, ");\n");
diff --git a/src/bin/pg_dump/pg_dump.h b/src/bin/pg_dump/pg_dump.h
index de9783c..4002f94 100644
--- a/src/bin/pg_dump/pg_dump.h
+++ b/src/bin/pg_dump/pg_dump.h
@@ -625,7 +625,7 @@ typedef struct _PublicationInfo
bool pubdelete;
bool pubtruncate;
bool pubviaroot;
- bool pubgencolumns;
+ bool pubgencols;
} PublicationInfo;
/*
diff --git a/src/bin/psql/describe.c b/src/bin/psql/describe.c
index 983962b..b8d8b4d 100644
--- a/src/bin/psql/describe.c
+++ b/src/bin/psql/describe.c
@@ -6266,7 +6266,7 @@ listPublications(const char *pattern)
gettext_noop("Via root"));
if (pset.sversion >= 180000)
appendPQExpBuffer(&buf,
- ",\n pubgencolumns AS
\"%s\"",
+ ",\n pubgencols AS \"%s\"",
gettext_noop("Generated
columns"));
appendPQExpBufferStr(&buf,
"\nFROM
pg_catalog.pg_publication\n");
@@ -6356,7 +6356,7 @@ describePublications(const char *pattern)
PGresult *res;
bool has_pubtruncate;
bool has_pubviaroot;
- bool has_pubgencol;
+ bool has_pubgencols;
PQExpBufferData title;
printTableContent cont;
@@ -6373,7 +6373,7 @@ describePublications(const char *pattern)
has_pubtruncate = (pset.sversion >= 110000);
has_pubviaroot = (pset.sversion >= 130000);
- has_pubgencol = (pset.sversion >= 180000);
+ has_pubgencols = (pset.sversion >= 180000);
initPQExpBuffer(&buf);
@@ -6387,9 +6387,9 @@ describePublications(const char *pattern)
if (has_pubviaroot)
appendPQExpBufferStr(&buf,
", pubviaroot");
- if (has_pubgencol)
+ if (has_pubgencols)
appendPQExpBufferStr(&buf,
- ", pubgencolumns");
+ ", pubgencols");
appendPQExpBufferStr(&buf,
"\nFROM
pg_catalog.pg_publication\n");
@@ -6441,7 +6441,7 @@ describePublications(const char *pattern)
ncols++;
if (has_pubviaroot)
ncols++;
- if (has_pubgencol)
+ if (has_pubgencols)
ncols++;
initPQExpBuffer(&title);
@@ -6457,7 +6457,7 @@ describePublications(const char *pattern)
printTableAddHeader(&cont, gettext_noop("Truncates"),
true, align);
if (has_pubviaroot)
printTableAddHeader(&cont, gettext_noop("Via root"),
true, align);
- if (has_pubgencol)
+ if (has_pubgencols)
printTableAddHeader(&cont, gettext_noop("Generated
columns"), true, align);
printTableAddCell(&cont, PQgetvalue(res, i, 2), false, false);
@@ -6469,7 +6469,7 @@ describePublications(const char *pattern)
printTableAddCell(&cont, PQgetvalue(res, i, 7), false,
false);
if (has_pubviaroot)
printTableAddCell(&cont, PQgetvalue(res, i, 8), false,
false);
- if (has_pubgencol)
+ if (has_pubgencols)
printTableAddCell(&cont, PQgetvalue(res, i, 9), false,
false);
if (!puballtables)
diff --git a/src/include/catalog/pg_publication.h
b/src/include/catalog/pg_publication.h
index fc85a64..849b3a0 100644
--- a/src/include/catalog/pg_publication.h
+++ b/src/include/catalog/pg_publication.h
@@ -56,7 +56,7 @@ CATALOG(pg_publication,6104,PublicationRelationId)
bool pubviaroot;
/* true if generated columns data should be published */
- bool pubgencolumns;
+ bool pubgencols;
} FormData_pg_publication;
/* ----------------
@@ -106,7 +106,7 @@ typedef struct Publication
char *name;
bool alltables;
bool pubviaroot;
- bool pubgencolumns;
+ bool pubgencols;
PublicationActions pubactions;
} Publication;
diff --git a/src/test/regress/expected/publication.out
b/src/test/regress/expected/publication.out
index ab703e2..f083d4f 100644
--- a/src/test/regress/expected/publication.out
+++ b/src/test/regress/expected/publication.out
@@ -1753,9 +1753,9 @@ DROP PUBLICATION pub;
DROP TABLE sch1.tbl1;
DROP SCHEMA sch1 cascade;
DROP SCHEMA sch2 cascade;
--- Test the publication with or without 'PUBLISH_GENERATED_COLUMNS' parameter
+-- Test the publication 'publish_generated_columns' parameter enabled or
disabled
SET client_min_messages = 'ERROR';
-CREATE PUBLICATION pub1 FOR ALL TABLES WITH (PUBLISH_GENERATED_COLUMNS=1);
+CREATE PUBLICATION pub1 FOR ALL TABLES WITH (publish_generated_columns=1);
\dRp+ pub1
Publication pub1
Owner | All tables | Inserts | Updates | Deletes |
Truncates | Via root | Generated columns
@@ -1763,7 +1763,7 @@ CREATE PUBLICATION pub1 FOR ALL TABLES WITH
(PUBLISH_GENERATED_COLUMNS=1);
regress_publication_user | t | t | t | t | t
| f | t
(1 row)
-CREATE PUBLICATION pub2 FOR ALL TABLES WITH (PUBLISH_GENERATED_COLUMNS=0);
+CREATE PUBLICATION pub2 FOR ALL TABLES WITH (publish_generated_columns=0);
\dRp+ pub2
Publication pub2
Owner | All tables | Inserts | Updates | Deletes |
Truncates | Via root | Generated columns
diff --git a/src/test/regress/sql/publication.sql
b/src/test/regress/sql/publication.sql
index 2673397..78101b9 100644
--- a/src/test/regress/sql/publication.sql
+++ b/src/test/regress/sql/publication.sql
@@ -1112,12 +1112,12 @@ DROP TABLE sch1.tbl1;
DROP SCHEMA sch1 cascade;
DROP SCHEMA sch2 cascade;
--- Test the publication with or without 'PUBLISH_GENERATED_COLUMNS' parameter
+-- Test the publication 'publish_generated_columns' parameter enabled or
disabled
SET client_min_messages = 'ERROR';
-CREATE PUBLICATION pub1 FOR ALL TABLES WITH (PUBLISH_GENERATED_COLUMNS=1);
+CREATE PUBLICATION pub1 FOR ALL TABLES WITH (publish_generated_columns=1);
\dRp+ pub1
-CREATE PUBLICATION pub2 FOR ALL TABLES WITH (PUBLISH_GENERATED_COLUMNS=0);
+CREATE PUBLICATION pub2 FOR ALL TABLES WITH (publish_generated_columns=0);
\dRp+ pub2
RESET client_min_messages;
