From 4148c8b3daf95ca308f055e103f6ee82e25b8f99 Mon Sep 17 00:00:00 2001 From: Alvaro Herrera Date: Tue, 27 Sep 2022 14:11:31 +0200 Subject: [PATCH] Improve some publication-related error messages While at it, remove an unused queryString parameter from CheckPubRelationColumnList() and make other minor stylistic changes. Backpatch to 15. Reported by Kyotaro Horiguchi Co-authored-by: Hou zj Discussion: https://postgr.es/m/20220926.160426.454497059203258582.horikyota.ntt@gmail.com --- src/backend/commands/publicationcmds.c | 57 ++++++++++++++--------- src/test/regress/expected/publication.out | 26 +++++------ 2 files changed, 47 insertions(+), 36 deletions(-) diff --git a/src/backend/commands/publicationcmds.c b/src/backend/commands/publicationcmds.c index 99011eec9f..8895e1be4e 100644 --- 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 index bfce1e1bc0..427f87ea07 100644 --- 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