On Thu, Nov 4, 2021 at 5:41 AM Peter Smith <smithpb2...@gmail.com> wrote:
>
> FYI - I found a small problem with one of the new PublicationObjSpec
> parser error messages that was introduced by the recent schema
> publication commit [1].
>
> The error message text is assuming that the error originates from
> CREATE PUBLICATION, but actually that same error can also come from
> ALTER PUBLICATION.
>
> For example,
>
> e.g.1) Here the error came from CREATE PUBLICATION, so the message
> text looks OK.
>
> test_pub=# CREATE PUBLICATION p1 FOR t1;
> 2021-11-04 10:50:17.208 AEDT [738] ERROR: FOR TABLE/FOR ALL TABLES IN
> SCHEMA should be specified before the table/schema name(s) at
> character 27
> 2021-11-04 10:50:17.208 AEDT [738] STATEMENT: CREATE PUBLICATION p1 FOR t1;
> ERROR: FOR TABLE/FOR ALL TABLES IN SCHEMA should be specified before
> the table/schema name(s)
> LINE 1: CREATE PUBLICATION p1 FOR t1;
> ^
> ~~
>
> e.g.2) Here the error came from ALTER PUBLICATION, so the message text
> is not OK because the ALTER syntax [2] does not even have a FOR
> keyword.
>
> test_pub=# ALTER PUBLICATION p1 SET t1;
> 2021-11-04 10:51:53.912 AEDT [738] ERROR: FOR TABLE/FOR ALL TABLES IN
> SCHEMA should be specified before the table/schema name(s) at
> character 26
> 2021-11-04 10:51:53.912 AEDT [738] STATEMENT: ALTER PUBLICATION p1 SET t1;
> ERROR: FOR TABLE/FOR ALL TABLES IN SCHEMA should be specified before
> the table/schema name(s)
> LINE 1: ALTER PUBLICATION p1 SET t1;
> ^
Thanks for the comment, I changed the error message to remove the FOR
keyword. The attached patch has the changes for the same.
Regards,
Vignesh
From 8a36508d18144150d63812778a8c675a75d0f56d Mon Sep 17 00:00:00 2001
From: Vigneshwaran C <vignes...@gmail.com>
Date: Wed, 3 Nov 2021 09:21:22 +0530
Subject: [PATCH v3] Renamed few enums and changed error message in publish
schema feature.
Renamed PUBLICATIONOBJ_REL_IN_SCHEMA, PUBLICATIONOBJ_CURRSCHEMA,
DO_PUBLICATION_REL_IN_SCHEMA and PRIO_PUBLICATION_REL_IN_SCHEMA
to PUBLICATIONOBJ_TABLE_IN_SCHEMA, PUBLICATIONOBJ_TABLE_IN_CUR_SCHEMA,
DO_PUBLICATION_TABLE_IN_SCHEMA and PRIO_PUBLICATION_TABLE_IN_SCHEMA in publish
schema feature, this change is required to avoid confusion with other objects
like sequences which will be available in the future and also changed
the error message to keep it common for both create and alter publication.
---
src/backend/commands/publicationcmds.c | 8 ++++----
src/backend/parser/gram.y | 14 +++++++-------
src/bin/pg_dump/pg_dump.c | 6 +++---
src/bin/pg_dump/pg_dump.h | 2 +-
src/bin/pg_dump/pg_dump_sort.c | 6 +++---
src/include/nodes/parsenodes.h | 5 +++--
src/test/regress/expected/publication.out | 4 ++--
7 files changed, 23 insertions(+), 22 deletions(-)
diff --git a/src/backend/commands/publicationcmds.c b/src/backend/commands/publicationcmds.c
index d1fff13d2e..7d4a0e95f6 100644
--- a/src/backend/commands/publicationcmds.c
+++ b/src/backend/commands/publicationcmds.c
@@ -169,13 +169,13 @@ ObjectsInPublicationToOids(List *pubobjspec_list, ParseState *pstate,
case PUBLICATIONOBJ_TABLE:
*rels = lappend(*rels, pubobj->pubtable);
break;
- case PUBLICATIONOBJ_REL_IN_SCHEMA:
+ case PUBLICATIONOBJ_TABLE_IN_SCHEMA:
schemaid = get_namespace_oid(pubobj->name, false);
/* Filter out duplicates if user specifies "sch1, sch1" */
*schemas = list_append_unique_oid(*schemas, schemaid);
break;
- case PUBLICATIONOBJ_CURRSCHEMA:
+ case PUBLICATIONOBJ_TABLE_IN_CUR_SCHEMA:
search_path = fetch_search_path(false);
if (search_path == NIL) /* nothing valid in search_path? */
ereport(ERROR,
@@ -214,7 +214,7 @@ CheckObjSchemaNotAlreadyInPublication(List *rels, List *schemaidlist,
if (list_member_oid(schemaidlist, relSchemaId))
{
- if (checkobjtype == PUBLICATIONOBJ_REL_IN_SCHEMA)
+ if (checkobjtype == PUBLICATIONOBJ_TABLE_IN_SCHEMA)
ereport(ERROR,
errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("cannot add schema \"%s\" to publication",
@@ -613,7 +613,7 @@ AlterPublicationSchemas(AlterPublicationStmt *stmt,
rels = OpenReliIdList(reloids);
CheckObjSchemaNotAlreadyInPublication(rels, schemaidlist,
- PUBLICATIONOBJ_REL_IN_SCHEMA);
+ PUBLICATIONOBJ_TABLE_IN_SCHEMA);
CloseTableList(rels);
PublicationAddSchemas(pubform->oid, schemaidlist, false, stmt);
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index d0eb80e69c..a6d0cefa6b 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -9662,14 +9662,14 @@ PublicationObjSpec:
| ALL TABLES IN_P SCHEMA ColId
{
$$ = makeNode(PublicationObjSpec);
- $$->pubobjtype = PUBLICATIONOBJ_REL_IN_SCHEMA;
+ $$->pubobjtype = PUBLICATIONOBJ_TABLE_IN_SCHEMA;
$$->name = $5;
$$->location = @5;
}
| ALL TABLES IN_P SCHEMA CURRENT_SCHEMA
{
$$ = makeNode(PublicationObjSpec);
- $$->pubobjtype = PUBLICATIONOBJ_CURRSCHEMA;
+ $$->pubobjtype = PUBLICATIONOBJ_TABLE_IN_CUR_SCHEMA;
$$->location = @5;
}
| ColId
@@ -17323,7 +17323,7 @@ preprocess_pubobj_list(List *pubobjspec_list, core_yyscan_t yyscanner)
if (pubobj->pubobjtype == PUBLICATIONOBJ_CONTINUATION)
ereport(ERROR,
errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("FOR TABLE/FOR ALL TABLES IN SCHEMA should be specified before the table/schema name(s)"),
+ errmsg("TABLE/ALL TABLES IN SCHEMA should be specified before the table/schema name(s)"),
parser_errposition(pubobj->location));
foreach(cell, pubobjspec_list)
@@ -17351,17 +17351,17 @@ preprocess_pubobj_list(List *pubobjspec_list, core_yyscan_t yyscanner)
pubobj->name = NULL;
}
}
- else if (pubobj->pubobjtype == PUBLICATIONOBJ_REL_IN_SCHEMA ||
- pubobj->pubobjtype == PUBLICATIONOBJ_CURRSCHEMA)
+ else if (pubobj->pubobjtype == PUBLICATIONOBJ_TABLE_IN_SCHEMA ||
+ pubobj->pubobjtype == PUBLICATIONOBJ_TABLE_IN_CUR_SCHEMA)
{
/*
* We can distinguish between the different type of schema
* objects based on whether name and pubtable is set.
*/
if (pubobj->name)
- pubobj->pubobjtype = PUBLICATIONOBJ_REL_IN_SCHEMA;
+ pubobj->pubobjtype = PUBLICATIONOBJ_TABLE_IN_SCHEMA;
else if (!pubobj->name && !pubobj->pubtable)
- pubobj->pubobjtype = PUBLICATIONOBJ_CURRSCHEMA;
+ pubobj->pubobjtype = PUBLICATIONOBJ_TABLE_IN_CUR_SCHEMA;
else
ereport(ERROR,
errcode(ERRCODE_SYNTAX_ERROR),
diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c
index b9635a95b6..7e98371d25 100644
--- a/src/bin/pg_dump/pg_dump.c
+++ b/src/bin/pg_dump/pg_dump.c
@@ -4194,7 +4194,7 @@ getPublicationNamespaces(Archive *fout)
continue;
/* OK, make a DumpableObject for this relationship */
- pubsinfo[j].dobj.objType = DO_PUBLICATION_REL_IN_SCHEMA;
+ pubsinfo[j].dobj.objType = DO_PUBLICATION_TABLE_IN_SCHEMA;
pubsinfo[j].dobj.catId.tableoid =
atooid(PQgetvalue(res, i, i_tableoid));
pubsinfo[j].dobj.catId.oid = atooid(PQgetvalue(res, i, i_oid));
@@ -10331,7 +10331,7 @@ dumpDumpableObject(Archive *fout, const DumpableObject *dobj)
case DO_PUBLICATION_REL:
dumpPublicationTable(fout, (const PublicationRelInfo *) dobj);
break;
- case DO_PUBLICATION_REL_IN_SCHEMA:
+ case DO_PUBLICATION_TABLE_IN_SCHEMA:
dumpPublicationNamespace(fout,
(const PublicationSchemaInfo *) dobj);
break;
@@ -18559,7 +18559,7 @@ addBoundaryDependencies(DumpableObject **dobjs, int numObjs,
case DO_POLICY:
case DO_PUBLICATION:
case DO_PUBLICATION_REL:
- case DO_PUBLICATION_REL_IN_SCHEMA:
+ case DO_PUBLICATION_TABLE_IN_SCHEMA:
case DO_SUBSCRIPTION:
/* Post-data objects: must come after the post-data boundary */
addObjectDependency(dobj, postDataBound->dumpId);
diff --git a/src/bin/pg_dump/pg_dump.h b/src/bin/pg_dump/pg_dump.h
index f9af14b793..d1d8608e9c 100644
--- a/src/bin/pg_dump/pg_dump.h
+++ b/src/bin/pg_dump/pg_dump.h
@@ -81,7 +81,7 @@ typedef enum
DO_POLICY,
DO_PUBLICATION,
DO_PUBLICATION_REL,
- DO_PUBLICATION_REL_IN_SCHEMA,
+ DO_PUBLICATION_TABLE_IN_SCHEMA,
DO_SUBSCRIPTION
} DumpableObjectType;
diff --git a/src/bin/pg_dump/pg_dump_sort.c b/src/bin/pg_dump/pg_dump_sort.c
index 9901d9e0ba..410d1790ee 100644
--- a/src/bin/pg_dump/pg_dump_sort.c
+++ b/src/bin/pg_dump/pg_dump_sort.c
@@ -82,7 +82,7 @@ enum dbObjectTypePriorities
PRIO_POLICY,
PRIO_PUBLICATION,
PRIO_PUBLICATION_REL,
- PRIO_PUBLICATION_REL_IN_SCHEMA,
+ PRIO_PUBLICATION_TABLE_IN_SCHEMA,
PRIO_SUBSCRIPTION,
PRIO_DEFAULT_ACL, /* done in ACL pass */
PRIO_EVENT_TRIGGER, /* must be next to last! */
@@ -136,7 +136,7 @@ static const int dbObjectTypePriority[] =
PRIO_POLICY, /* DO_POLICY */
PRIO_PUBLICATION, /* DO_PUBLICATION */
PRIO_PUBLICATION_REL, /* DO_PUBLICATION_REL */
- PRIO_PUBLICATION_REL_IN_SCHEMA, /* DO_PUBLICATION_REL_IN_SCHEMA */
+ PRIO_PUBLICATION_TABLE_IN_SCHEMA, /* DO_PUBLICATION_TABLE_IN_SCHEMA */
PRIO_SUBSCRIPTION /* DO_SUBSCRIPTION */
};
@@ -1479,7 +1479,7 @@ describeDumpableObject(DumpableObject *obj, char *buf, int bufsize)
"PUBLICATION TABLE (ID %d OID %u)",
obj->dumpId, obj->catId.oid);
return;
- case DO_PUBLICATION_REL_IN_SCHEMA:
+ case DO_PUBLICATION_TABLE_IN_SCHEMA:
snprintf(buf, bufsize,
"PUBLICATION TABLES IN SCHEMA (ID %d OID %u)",
obj->dumpId, obj->catId.oid);
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 49123e28a4..067138e6b5 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -3649,8 +3649,9 @@ typedef struct PublicationTable
typedef enum PublicationObjSpecType
{
PUBLICATIONOBJ_TABLE, /* Table type */
- PUBLICATIONOBJ_REL_IN_SCHEMA, /* Relations in schema type */
- PUBLICATIONOBJ_CURRSCHEMA, /* Get the first element from search_path */
+ PUBLICATIONOBJ_TABLE_IN_SCHEMA, /* Tables in schema type */
+ PUBLICATIONOBJ_TABLE_IN_CUR_SCHEMA, /* Get the first element from
+ * search_path */
PUBLICATIONOBJ_CONTINUATION /* Continuation of previous type */
} PublicationObjSpecType;
diff --git a/src/test/regress/expected/publication.out b/src/test/regress/expected/publication.out
index 0f4fe4db8f..2ff21a7543 100644
--- a/src/test/regress/expected/publication.out
+++ b/src/test/regress/expected/publication.out
@@ -478,7 +478,7 @@ RESET SEARCH_PATH;
-- check create publication on CURRENT_SCHEMA where TABLE/ALL TABLES in SCHEMA
-- is not specified
CREATE PUBLICATION testpub_forschema1 FOR CURRENT_SCHEMA;
-ERROR: FOR TABLE/FOR ALL TABLES IN SCHEMA should be specified before the table/schema name(s)
+ERROR: TABLE/ALL TABLES IN SCHEMA should be specified before the table/schema name(s)
LINE 1: CREATE PUBLICATION testpub_forschema1 FOR CURRENT_SCHEMA;
^
-- check create publication on CURRENT_SCHEMA along with FOR TABLE
@@ -747,7 +747,7 @@ Tables from schemas:
-- fail specifying table without any of 'FOR ALL TABLES IN SCHEMA' or
--'FOR TABLE' or 'FOR ALL TABLES'
CREATE PUBLICATION testpub_error FOR pub_test2.tbl1;
-ERROR: FOR TABLE/FOR ALL TABLES IN SCHEMA should be specified before the table/schema name(s)
+ERROR: TABLE/ALL TABLES IN SCHEMA should be specified before the table/schema name(s)
LINE 1: CREATE PUBLICATION testpub_error FOR pub_test2.tbl1;
^
DROP VIEW testpub_view;
--
2.30.2
