Throw ERROR when publish_generated_columns is specified without a value.

Previously, specifying the publication option 'publish_generated_columns'
without an explicit value would incorrectly default to 'stored', which is
not the intended behavior.

This patch fixes the issue by raising an ERROR when no value is provided
for 'publish_generated_columns', ensuring that users must explicitly
specify a valid option.

Author: Peter Smith <[email protected]>
Reviewed-by: vignesh C <[email protected]>
Backpatch-through: 18, where it was introduced
Discussion: https://postgr.es/m/CAHut+PsCUCWiEKmB10DxhoPfXbF6jw5RD9ib2LuaQeA_XraW7w@mail.gmail.com

diff --git a/src/backend/commands/publicationcmds.c b/src/backend/commands/publicationcmds.c
index 1bf7eaae5b362bd85715b4ded64a17dec129f0bb..803c26ab216dd51433b8e936a55a9bb2972d3abf 100644 (file)
--- a/src/backend/commands/publicationcmds.c
+++ b/src/backend/commands/publicationcmds.c
@@ -2113,20 +2113,20 @@ AlterPublicationOwner_oid(Oid pubid, Oid newOwnerId)
 static char
 defGetGeneratedColsOption(DefElem *def)
 {
-   char       *sval;
+   char       *sval = "";
 
    /*
-    * If no parameter value given, assume "stored" is meant.
+    * A parameter value is required.
     */
-   if (!def->arg)
-       return PUBLISH_GENCOLS_STORED;
-
-   sval = defGetString(def);
+   if (def->arg)
+   {
+       sval = defGetString(def);
 
-   if (pg_strcasecmp(sval, "none") == 0)
-       return PUBLISH_GENCOLS_NONE;
-   if (pg_strcasecmp(sval, "stored") == 0)
-       return PUBLISH_GENCOLS_STORED;
+       if (pg_strcasecmp(sval, "none") == 0)
+           return PUBLISH_GENCOLS_NONE;
+       if (pg_strcasecmp(sval, "stored") == 0)
+           return PUBLISH_GENCOLS_STORED;
+   }
 
    ereport(ERROR,
            errcode(ERRCODE_SYNTAX_ERROR),

diff --git a/src/test/regress/expected/publication.out b/src/test/regress/expected/publication.out
index 1ec3fa34a2d5a6d5f21cd451fd50819888a23dc6..53268059142eedc5bfe7475d067c254a2cc1fc37 100644 (file)
--- a/src/test/regress/expected/publication.out
+++ b/src/test/regress/expected/publication.out
@@ -36,6 +36,9 @@ LINE 1: ...pub_xxx WITH (publish_generated_columns = stored, publish_ge...
 CREATE PUBLICATION testpub_xxx WITH (publish_generated_columns = foo);
 ERROR:  invalid value for publication parameter "publish_generated_columns": "foo"
 DETAIL:  Valid values are "none" and "stored".
+CREATE PUBLICATION testpub_xxx WITH (publish_generated_columns);
+ERROR:  invalid value for publication parameter "publish_generated_columns": ""
+DETAIL:  Valid values are "none" and "stored".
 \dRp
                                                         List of publications
         Name        |          Owner           | All tables | Inserts | Updates | Deletes | Truncates | Generated columns | Via root 
@@ -1844,8 +1847,7 @@ DROP SCHEMA sch1 cascade;
 DROP SCHEMA sch2 cascade;
 -- ======================================================
 -- Test the 'publish_generated_columns' parameter with the following values:
--- 'stored', 'none', and the default (no value specified), which defaults to
--- 'stored'.
+-- 'stored', 'none'.
 SET client_min_messages = 'ERROR';
 CREATE PUBLICATION pub1 FOR ALL TABLES WITH (publish_generated_columns = stored);
 \dRp+ pub1
@@ -1863,17 +1865,8 @@ CREATE PUBLICATION pub2 FOR ALL TABLES WITH (publish_generated_columns = none);
  regress_publication_user | t          | t       | t       | t       | t         | none              | f
 (1 row)
 
-CREATE PUBLICATION pub3 FOR ALL TABLES WITH (publish_generated_columns);
-\dRp+ pub3
-                                                Publication pub3
-          Owner           | All tables | Inserts | Updates | Deletes | Truncates | Generated columns | Via root 
---------------------------+------------+---------+---------+---------+-----------+-------------------+----------
- regress_publication_user | t          | t       | t       | t       | t         | stored            | f
-(1 row)
-
 DROP PUBLICATION pub1;
 DROP PUBLICATION pub2;
-DROP PUBLICATION pub3;
 -- Test the 'publish_generated_columns' parameter as 'none' and 'stored' for
 -- different scenarios with/without generated columns in column lists.
 CREATE TABLE gencols (a int, gen1 int GENERATED ALWAYS AS (a * 2) STORED);

diff --git a/src/test/regress/sql/publication.sql b/src/test/regress/sql/publication.sql
index 2585f08318150c5b5c34876645f6a5be4bea6774..deddf0da8445f32172b48d1a128ae628c89f6c70 100644 (file)
--- a/src/test/regress/sql/publication.sql
+++ b/src/test/regress/sql/publication.sql
@@ -26,6 +26,7 @@ CREATE PUBLICATION testpub_xxx WITH (publish = 'cluster, vacuum');
 CREATE PUBLICATION testpub_xxx WITH (publish_via_partition_root = 'true', publish_via_partition_root = '0');
 CREATE PUBLICATION testpub_xxx WITH (publish_generated_columns = stored, publish_generated_columns = none);
 CREATE PUBLICATION testpub_xxx WITH (publish_generated_columns = foo);
+CREATE PUBLICATION testpub_xxx WITH (publish_generated_columns);
 
 \dRp
 
@@ -1183,19 +1184,15 @@ DROP SCHEMA sch2 cascade;
 -- ======================================================
 
 -- Test the 'publish_generated_columns' parameter with the following values:
--- 'stored', 'none', and the default (no value specified), which defaults to
--- 'stored'.
+-- 'stored', 'none'.
 SET client_min_messages = 'ERROR';
 CREATE PUBLICATION pub1 FOR ALL TABLES WITH (publish_generated_columns = stored);
 \dRp+ pub1
 CREATE PUBLICATION pub2 FOR ALL TABLES WITH (publish_generated_columns = none);
 \dRp+ pub2
-CREATE PUBLICATION pub3 FOR ALL TABLES WITH (publish_generated_columns);
-\dRp+ pub3
 
 DROP PUBLICATION pub1;
 DROP PUBLICATION pub2;
-DROP PUBLICATION pub3;
 
 -- Test the 'publish_generated_columns' parameter as 'none' and 'stored' for
 -- different scenarios with/without generated columns in column lists.