Improve some publication-related error messages

alvherre · Hou zj · alvherre · commit 4148c8b3daf9 · 2022-09-27T14:11:31.000+02:00
While at it, remove an unused queryString parameter from CheckPubRelationColumnList() and make other minor stylistic changes. Backpatch to 15. Reported by Kyotaro Horiguchi <horikyota.ntt@gmail.com> Co-authored-by: Hou zj <houzj.fnst@fujitsu.com> Discussion: https://postgr.es/m/20220926.160426.454497059203258582.horikyota.ntt@gmail.com
diff --git a/src/backend/commands/publicationcmds.c b/src/backend/commands/publicationcmds.c
@@ -53,6 +53,7 @@
 #include "utils/syscache.h"
 #include "utils/varlena.h"
 
+
 /*
  * Information used to validate the columns in the row filter expression. See
  * contain_invalid_rfcolumn_walker for details.
@@ -76,6 +77,7 @@ static void PublicationAddSchemas(Oid pubid, List *schemas, bool if_not_exists,
 								  AlterPublicationStmt *stmt);
 static void PublicationDropSchemas(Oid pubid, List *schemas, bool missing_ok);
 
+
 static void
 parse_publication_options(ParseState *pstate,
 						  List *options,
@@ -125,7 +127,8 @@ parse_publication_options(ParseState *pstate,
 			if (!SplitIdentifierString(publish, ',', &publish_list))
 				ereport(ERROR,
 						(errcode(ERRCODE_SYNTAX_ERROR),
-						 errmsg("invalid list syntax for \"publish\" option")));
+						 errmsg("invalid list syntax in parameter \"%s\"",
+								"publish")));
 
 			/* Process the option list. */
 			foreach(lc, publish_list)
@@ -143,7 +146,8 @@ parse_publication_options(ParseState *pstate,
 				else
 					ereport(ERROR,
 							(errcode(ERRCODE_SYNTAX_ERROR),
-							 errmsg("unrecognized \"publish\" value: \"%s\"", publish_opt)));
+							 errmsg("unrecognized value for publication option \"%s\": \"%s\"",
+									"publish", publish_opt)));
 			}
 		}
 		else if (strcmp(defel->defname, "publish_via_partition_root") == 0)
@@ -444,10 +448,12 @@ contain_mutable_or_user_functions_checker(Oid func_id, void *context)
 }
 
 /*
- * Check if the node contains any unallowed object. See
+ * Check if the node contains any disallowed object. Subroutine for
  * check_simple_rowfilter_expr_walker.
  *
- * Returns the error detail message in errdetail_msg for unallowed expressions.
+ * If a disallowed object is found, *errdetail_msg is set to a (possibly
+ * translated) message to use as errdetail.  If none, *errdetail_msg is not
+ * modified.
  */
 static void
 expr_allowed_in_node(Node *node, ParseState *pstate, char **errdetail_msg)
@@ -678,10 +684,17 @@ TransformPubWhereClauses(List *tables, const char *queryString,
 
 
 /*
- * Check the publication column lists expression for all relations in the list.
+ * Given a list of tables that are going to be added to a publication,
+ * verify that they fulfill the necessary preconditions, namely: no tables
+ * have a column list if any schema is published; and partitioned tables do
+ * not have column lists if publish_via_partition_root is not set.
+ *
+ * 'publish_schema' indicates that the publication contains any TABLES IN
+ * SCHEMA elements (newly added in this command, or preexisting).
+ * 'pubviaroot' is the value of publish_via_partition_root.
  */
 static void
-CheckPubRelationColumnList(List *tables, const char *queryString,
+CheckPubRelationColumnList(char *pubname, List *tables,
 						   bool publish_schema, bool pubviaroot)
 {
 	ListCell   *lc;
@@ -706,23 +719,24 @@ CheckPubRelationColumnList(List *tables, const char *queryString,
 		if (publish_schema)
 			ereport(ERROR,
 					errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-					errmsg("cannot use publication column list for relation \"%s.%s\"",
+					errmsg("cannot use column list for relation \"%s.%s\" in publication \"%s\"",
 						   get_namespace_name(RelationGetNamespace(pri->relation)),
-						   RelationGetRelationName(pri->relation)),
-					errdetail("Column list cannot be specified if any schema is part of the publication or specified in the list."));
+						   RelationGetRelationName(pri->relation), pubname),
+					errdetail("Column lists cannot be specified in publications containing FOR TABLES IN SCHEMA elements."));
 
 		/*
 		 * If the publication doesn't publish changes via the root partitioned
 		 * table, the partition's column list will be used. So disallow using
-		 * the column list on partitioned table in this case.
+		 * a column list on the partitioned table in this case.
 		 */
 		if (!pubviaroot &&
 			pri->relation->rd_rel->relkind == RELKIND_PARTITIONED_TABLE)
 			ereport(ERROR,
 					(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-					 errmsg("cannot use publication column list for relation \"%s\"",
-							RelationGetRelationName(pri->relation)),
-					 errdetail("Column list cannot be used for a partitioned table when %s is false.",
+					 errmsg("cannot use column list for relation \"%s.%s\" in publication \"%s\"",
+							get_namespace_name(RelationGetNamespace(pri->relation)),
+							RelationGetRelationName(pri->relation), pubname),
+					 errdetail("Column lists cannot be specified for partitioned tables when %s is false.",
 							   "publish_via_partition_root")));
 	}
 }
@@ -765,12 +779,10 @@ CreatePublication(ParseState *pstate, CreatePublicationStmt *stmt)
 	puboid = GetSysCacheOid1(PUBLICATIONNAME, Anum_pg_publication_oid,
 							 CStringGetDatum(stmt->pubname));
 	if (OidIsValid(puboid))
-	{
 		ereport(ERROR,
 				(errcode(ERRCODE_DUPLICATE_OBJECT),
 				 errmsg("publication \"%s\" already exists",
 						stmt->pubname)));
-	}
 
 	/* Form a tuple. */
 	memset(values, 0, sizeof(values));
@@ -840,7 +852,7 @@ CreatePublication(ParseState *pstate, CreatePublicationStmt *stmt)
 			TransformPubWhereClauses(rels, pstate->p_sourcetext,
 									 publish_via_partition_root);
 
-			CheckPubRelationColumnList(rels, pstate->p_sourcetext,
+			CheckPubRelationColumnList(stmt->pubname, rels,
 									   schemaidlist != NIL,
 									   publish_via_partition_root);
 
@@ -864,12 +876,10 @@ CreatePublication(ParseState *pstate, CreatePublicationStmt *stmt)
 	InvokeObjectPostCreateHook(PublicationRelationId, puboid, 0);
 
 	if (wal_level != WAL_LEVEL_LOGICAL)
-	{
 		ereport(WARNING,
 				(errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE),
 				 errmsg("wal_level is insufficient to publish logical changes"),
-				 errhint("Set wal_level to logical before creating subscriptions.")));
-	}
+				 errhint("Set wal_level to \"logical\" before creating subscriptions.")));
 
 	return myself;
 }
@@ -1110,7 +1120,7 @@ AlterPublicationTables(AlterPublicationStmt *stmt, HeapTuple tup,
 
 		publish_schema |= is_schema_publication(pubid);
 
-		CheckPubRelationColumnList(rels, queryString, publish_schema,
+		CheckPubRelationColumnList(stmt->pubname, rels, publish_schema,
 								   pubform->pubviaroot);
 
 		PublicationAddTables(pubid, rels, false, stmt);
@@ -1126,7 +1136,7 @@ AlterPublicationTables(AlterPublicationStmt *stmt, HeapTuple tup,
 
 		TransformPubWhereClauses(rels, queryString, pubform->pubviaroot);
 
-		CheckPubRelationColumnList(rels, queryString, publish_schema,
+		CheckPubRelationColumnList(stmt->pubname, rels, publish_schema,
 								   pubform->pubviaroot);
 
 		/*
@@ -1299,8 +1309,9 @@ AlterPublicationSchemas(AlterPublicationStmt *stmt,
 			if (!heap_attisnull(coltuple, Anum_pg_publication_rel_prattrs, NULL))
 				ereport(ERROR,
 						errcode(ERRCODE_INVALID_PARAMETER_VALUE),
-						errmsg("cannot add schema to the publication"),
-						errdetail("Schema cannot be added if any table that specifies column list is already part of the publication."));
+						errmsg("cannot add schema to publication \"%s\"",
+							   stmt->pubname),
+						errdetail("Schemas cannot be added if any tables that specify a column list are already part of the publication."));
 
 			ReleaseSysCache(coltuple);
 		}
diff --git a/src/test/regress/expected/publication.out b/src/test/regress/expected/publication.out
@@ -24,7 +24,7 @@ ALTER PUBLICATION testpub_default SET (publish = update);
 CREATE PUBLICATION testpub_xxx WITH (foo);
 ERROR:  unrecognized publication parameter: "foo"
 CREATE PUBLICATION testpub_xxx WITH (publish = 'cluster, vacuum');
-ERROR:  unrecognized "publish" value: "cluster"
+ERROR:  unrecognized value for publication option "publish": "cluster"
 CREATE PUBLICATION testpub_xxx WITH (publish_via_partition_root = 'true', publish_via_partition_root = '0');
 ERROR:  conflicting or redundant options
 LINE 1: ...ub_xxx WITH (publish_via_partition_root = 'true', publish_vi...
@@ -853,32 +853,32 @@ DETAIL:  Column list used by the publication does not cover the replica identity
 SET client_min_messages = 'ERROR';
 -- failure - cannot use column list and schema together
 CREATE PUBLICATION testpub_tbl9 FOR TABLES IN SCHEMA public, TABLE public.testpub_tbl7(a);
-ERROR:  cannot use publication column list for relation "public.testpub_tbl7"
-DETAIL:  Column list cannot be specified if any schema is part of the publication or specified in the list.
+ERROR:  cannot use column list for relation "public.testpub_tbl7" in publication "testpub_tbl9"
+DETAIL:  Column lists cannot be specified in publications containing FOR TABLES IN SCHEMA elements.
 -- ok - only publish schema
 CREATE PUBLICATION testpub_tbl9 FOR TABLES IN SCHEMA public;
 -- failure - add a table with column list when there is already a schema in the
 -- publication
 ALTER PUBLICATION testpub_tbl9 ADD TABLE public.testpub_tbl7(a);
-ERROR:  cannot use publication column list for relation "public.testpub_tbl7"
-DETAIL:  Column list cannot be specified if any schema is part of the publication or specified in the list.
+ERROR:  cannot use column list for relation "public.testpub_tbl7" in publication "testpub_tbl9"
+DETAIL:  Column lists cannot be specified in publications containing FOR TABLES IN SCHEMA elements.
 -- ok - only publish table with column list
 ALTER PUBLICATION testpub_tbl9 SET TABLE public.testpub_tbl7(a);
 -- failure - specify a schema when there is already a column list in the
 -- publication
 ALTER PUBLICATION testpub_tbl9 ADD TABLES IN SCHEMA public;
-ERROR:  cannot add schema to the publication
-DETAIL:  Schema cannot be added if any table that specifies column list is already part of the publication.
+ERROR:  cannot add schema to publication "testpub_tbl9"
+DETAIL:  Schemas cannot be added if any tables that specify a column list are already part of the publication.
 -- failure - cannot SET column list and schema together
 ALTER PUBLICATION testpub_tbl9 SET TABLES IN SCHEMA public, TABLE public.testpub_tbl7(a);
-ERROR:  cannot use publication column list for relation "public.testpub_tbl7"
-DETAIL:  Column list cannot be specified if any schema is part of the publication or specified in the list.
+ERROR:  cannot use column list for relation "public.testpub_tbl7" in publication "testpub_tbl9"
+DETAIL:  Column lists cannot be specified in publications containing FOR TABLES IN SCHEMA elements.
 -- ok - drop table
 ALTER PUBLICATION testpub_tbl9 DROP TABLE public.testpub_tbl7;
 -- failure - cannot ADD column list and schema together
 ALTER PUBLICATION testpub_tbl9 ADD TABLES IN SCHEMA public, TABLE public.testpub_tbl7(a);
-ERROR:  cannot use publication column list for relation "public.testpub_tbl7"
-DETAIL:  Column list cannot be specified if any schema is part of the publication or specified in the list.
+ERROR:  cannot use column list for relation "public.testpub_tbl7" in publication "testpub_tbl9"
+DETAIL:  Column lists cannot be specified in publications containing FOR TABLES IN SCHEMA elements.
 RESET client_min_messages;
 DROP TABLE testpub_tbl5, testpub_tbl6, testpub_tbl7, testpub_tbl8, testpub_tbl8_1;
 DROP PUBLICATION testpub_table_ins, testpub_fortable, testpub_fortable_insert, testpub_col_list, testpub_tbl9;
@@ -1009,8 +1009,8 @@ UPDATE rf_tbl_abcd_nopk SET a = 1;
 ALTER PUBLICATION testpub6 SET (PUBLISH_VIA_PARTITION_ROOT=0);
 -- fail - cannot use column list for partitioned table
 ALTER PUBLICATION testpub6 SET TABLE rf_tbl_abcd_part_pk (a);
-ERROR:  cannot use publication column list for relation "rf_tbl_abcd_part_pk"
-DETAIL:  Column list cannot be used for a partitioned table when publish_via_partition_root is false.
+ERROR:  cannot use column list for relation "public.rf_tbl_abcd_part_pk" in publication "testpub6"
+DETAIL:  Column lists cannot be specified for partitioned tables when publish_via_partition_root is false.
 -- ok - can use column list for partition
 ALTER PUBLICATION testpub6 SET TABLE rf_tbl_abcd_part_pk_1 (a);
 -- ok - "a" is a PK col
