From 2bfb50b3df11399ed80347dd03bfaf8cd5acf962 Mon Sep 17 00:00:00 2001 From: Dean Rasheed Date: Thu, 15 Jul 2021 08:49:45 +0100 Subject: [PATCH] Improve reporting of "conflicting or redundant options" errors. When reporting "conflicting or redundant options" errors, try to ensure that errposition() is used, to help the user identify the offending option. Formerly, errposition() was invoked in less than 60% of cases. This patch raises that to over 90%, but there remain a few places where the ParseState is not readily available. Using errdetail() might improve the error in such cases, but that is left as a task for the future. Additionally, since this error is thrown from over 100 places in the codebase, introduce a dedicated function to throw it, reducing code duplication. Extracted from a slightly larger patch by Vignesh C. Reviewed by Bharath Rupireddy, Alvaro Herrera, Dilip Kumar, Hou Zhijie, Peter Smith, Daniel Gustafsson, Julien Rouhaud and me. Discussion: https://postgr.es/m/CALDaNm33FFSS5tVyvmkoK2cCMuDVxcui=gFrjti9ROfynqSAGA@mail.gmail.com --- src/backend/catalog/aclchk.c | 11 +- src/backend/commands/copy.c | 59 +++-------- src/backend/commands/dbcommands.c | 70 +++---------- src/backend/commands/define.c | 12 +++ src/backend/commands/extension.c | 20 +--- src/backend/commands/foreigncmds.c | 18 ++-- src/backend/commands/functioncmds.c | 53 +++------- src/backend/commands/publicationcmds.c | 27 +++-- src/backend/commands/sequence.c | 45 ++------- src/backend/commands/subscriptioncmds.c | 66 +++++------- src/backend/commands/typecmds.c | 31 ++---- src/backend/commands/user.c | 112 +++++---------------- src/backend/parser/parse_utilcmd.c | 4 +- src/backend/tcop/utility.c | 20 ++-- src/include/commands/defrem.h | 7 +- src/include/commands/publicationcmds.h | 4 +- src/include/commands/subscriptioncmds.h | 4 +- src/include/commands/typecmds.h | 2 +- src/include/commands/user.h | 2 +- src/test/regress/expected/copy2.out | 2 + src/test/regress/expected/foreign_data.out | 4 + src/test/regress/expected/publication.out | 2 + 22 files changed, 180 insertions(+), 395 deletions(-) diff --git a/src/backend/catalog/aclchk.c b/src/backend/catalog/aclchk.c index 53392414f10..89792b154ee 100644 --- a/src/backend/catalog/aclchk.c +++ b/src/backend/catalog/aclchk.c @@ -59,6 +59,7 @@ #include "catalog/pg_ts_template.h" #include "catalog/pg_type.h" #include "commands/dbcommands.h" +#include "commands/defrem.h" #include "commands/event_trigger.h" #include "commands/extension.h" #include "commands/proclang.h" @@ -921,19 +922,13 @@ ExecAlterDefaultPrivilegesStmt(ParseState *pstate, AlterDefaultPrivilegesStmt *s if (strcmp(defel->defname, "schemas") == 0) { if (dnspnames) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("conflicting or redundant options"), - parser_errposition(pstate, defel->location))); + errorConflictingDefElem(defel, pstate); dnspnames = defel; } else if (strcmp(defel->defname, "roles") == 0) { if (drolespecs) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("conflicting or redundant options"), - parser_errposition(pstate, defel->location))); + errorConflictingDefElem(defel, pstate); drolespecs = defel; } else diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c index 8265b981ebd..6b33951e0c9 100644 --- a/src/backend/commands/copy.c +++ b/src/backend/commands/copy.c @@ -357,10 +357,7 @@ ProcessCopyOptions(ParseState *pstate, char *fmt = defGetString(defel); if (format_specified) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("conflicting or redundant options"), - parser_errposition(pstate, defel->location))); + errorConflictingDefElem(defel, pstate); format_specified = true; if (strcmp(fmt, "text") == 0) /* default format */ ; @@ -377,66 +374,45 @@ ProcessCopyOptions(ParseState *pstate, else if (strcmp(defel->defname, "freeze") == 0) { if (freeze_specified) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("conflicting or redundant options"), - parser_errposition(pstate, defel->location))); + errorConflictingDefElem(defel, pstate); freeze_specified = true; opts_out->freeze = defGetBoolean(defel); } else if (strcmp(defel->defname, "delimiter") == 0) { if (opts_out->delim) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("conflicting or redundant options"), - parser_errposition(pstate, defel->location))); + errorConflictingDefElem(defel, pstate); opts_out->delim = defGetString(defel); } else if (strcmp(defel->defname, "null") == 0) { if (opts_out->null_print) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("conflicting or redundant options"), - parser_errposition(pstate, defel->location))); + errorConflictingDefElem(defel, pstate); opts_out->null_print = defGetString(defel); } else if (strcmp(defel->defname, "header") == 0) { if (header_specified) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("conflicting or redundant options"), - parser_errposition(pstate, defel->location))); + errorConflictingDefElem(defel, pstate); header_specified = true; opts_out->header_line = defGetBoolean(defel); } else if (strcmp(defel->defname, "quote") == 0) { if (opts_out->quote) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("conflicting or redundant options"), - parser_errposition(pstate, defel->location))); + errorConflictingDefElem(defel, pstate); opts_out->quote = defGetString(defel); } else if (strcmp(defel->defname, "escape") == 0) { if (opts_out->escape) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("conflicting or redundant options"), - parser_errposition(pstate, defel->location))); + errorConflictingDefElem(defel, pstate); opts_out->escape = defGetString(defel); } else if (strcmp(defel->defname, "force_quote") == 0) { if (opts_out->force_quote || opts_out->force_quote_all) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("conflicting or redundant options"), - parser_errposition(pstate, defel->location))); + errorConflictingDefElem(defel, pstate); if (defel->arg && IsA(defel->arg, A_Star)) opts_out->force_quote_all = true; else if (defel->arg && IsA(defel->arg, List)) @@ -451,10 +427,7 @@ ProcessCopyOptions(ParseState *pstate, else if (strcmp(defel->defname, "force_not_null") == 0) { if (opts_out->force_notnull) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("conflicting or redundant options"), - parser_errposition(pstate, defel->location))); + errorConflictingDefElem(defel, pstate); if (defel->arg && IsA(defel->arg, List)) opts_out->force_notnull = castNode(List, defel->arg); else @@ -467,9 +440,7 @@ ProcessCopyOptions(ParseState *pstate, else if (strcmp(defel->defname, "force_null") == 0) { if (opts_out->force_null) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("conflicting or redundant options"))); + errorConflictingDefElem(defel, pstate); if (defel->arg && IsA(defel->arg, List)) opts_out->force_null = castNode(List, defel->arg); else @@ -487,10 +458,7 @@ ProcessCopyOptions(ParseState *pstate, * allowed for the column list to be NIL. */ if (opts_out->convert_selectively) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("conflicting or redundant options"), - parser_errposition(pstate, defel->location))); + errorConflictingDefElem(defel, pstate); opts_out->convert_selectively = true; if (defel->arg == NULL || IsA(defel->arg, List)) opts_out->convert_select = castNode(List, defel->arg); @@ -504,10 +472,7 @@ ProcessCopyOptions(ParseState *pstate, else if (strcmp(defel->defname, "encoding") == 0) { if (opts_out->file_encoding >= 0) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("conflicting or redundant options"), - parser_errposition(pstate, defel->location))); + errorConflictingDefElem(defel, pstate); opts_out->file_encoding = pg_char_to_encoding(defGetString(defel)); if (opts_out->file_encoding < 0) ereport(ERROR, diff --git a/src/backend/commands/dbcommands.c b/src/backend/commands/dbcommands.c index 2b159b60ebb..029fab48df3 100644 --- a/src/backend/commands/dbcommands.c +++ b/src/backend/commands/dbcommands.c @@ -152,91 +152,61 @@ createdb(ParseState *pstate, const CreatedbStmt *stmt) if (strcmp(defel->defname, "tablespace") == 0) { if (dtablespacename) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("conflicting or redundant options"), - parser_errposition(pstate, defel->location))); + errorConflictingDefElem(defel, pstate); dtablespacename = defel; } else if (strcmp(defel->defname, "owner") == 0) { if (downer) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("conflicting or redundant options"), - parser_errposition(pstate, defel->location))); + errorConflictingDefElem(defel, pstate); downer = defel; } else if (strcmp(defel->defname, "template") == 0) { if (dtemplate) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("conflicting or redundant options"), - parser_errposition(pstate, defel->location))); + errorConflictingDefElem(defel, pstate); dtemplate = defel; } else if (strcmp(defel->defname, "encoding") == 0) { if (dencoding) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("conflicting or redundant options"), - parser_errposition(pstate, defel->location))); + errorConflictingDefElem(defel, pstate); dencoding = defel; } else if (strcmp(defel->defname, "locale") == 0) { if (dlocale) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("conflicting or redundant options"), - parser_errposition(pstate, defel->location))); + errorConflictingDefElem(defel, pstate); dlocale = defel; } else if (strcmp(defel->defname, "lc_collate") == 0) { if (dcollate) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("conflicting or redundant options"), - parser_errposition(pstate, defel->location))); + errorConflictingDefElem(defel, pstate); dcollate = defel; } else if (strcmp(defel->defname, "lc_ctype") == 0) { if (dctype) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("conflicting or redundant options"), - parser_errposition(pstate, defel->location))); + errorConflictingDefElem(defel, pstate); dctype = defel; } else if (strcmp(defel->defname, "is_template") == 0) { if (distemplate) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("conflicting or redundant options"), - parser_errposition(pstate, defel->location))); + errorConflictingDefElem(defel, pstate); distemplate = defel; } else if (strcmp(defel->defname, "allow_connections") == 0) { if (dallowconnections) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("conflicting or redundant options"), - parser_errposition(pstate, defel->location))); + errorConflictingDefElem(defel, pstate); dallowconnections = defel; } else if (strcmp(defel->defname, "connection_limit") == 0) { if (dconnlimit) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("conflicting or redundant options"), - parser_errposition(pstate, defel->location))); + errorConflictingDefElem(defel, pstate); dconnlimit = defel; } else if (strcmp(defel->defname, "location") == 0) @@ -1497,37 +1467,25 @@ AlterDatabase(ParseState *pstate, AlterDatabaseStmt *stmt, bool isTopLevel) if (strcmp(defel->defname, "is_template") == 0) { if (distemplate) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("conflicting or redundant options"), - parser_errposition(pstate, defel->location))); + errorConflictingDefElem(defel, pstate); distemplate = defel; } else if (strcmp(defel->defname, "allow_connections") == 0) { if (dallowconnections) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("conflicting or redundant options"), - parser_errposition(pstate, defel->location))); + errorConflictingDefElem(defel, pstate); dallowconnections = defel; } else if (strcmp(defel->defname, "connection_limit") == 0) { if (dconnlimit) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("conflicting or redundant options"), - parser_errposition(pstate, defel->location))); + errorConflictingDefElem(defel, pstate); dconnlimit = defel; } else if (strcmp(defel->defname, "tablespace") == 0) { if (dtablespace) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("conflicting or redundant options"), - parser_errposition(pstate, defel->location))); + errorConflictingDefElem(defel, pstate); dtablespace = defel; } else diff --git a/src/backend/commands/define.c b/src/backend/commands/define.c index 84487b7d4b4..aafd7554e4e 100644 --- a/src/backend/commands/define.c +++ b/src/backend/commands/define.c @@ -347,3 +347,15 @@ defGetStringList(DefElem *def) return (List *) def->arg; } + +/* + * Raise an error about a conflicting DefElem. + */ +void +errorConflictingDefElem(DefElem *defel, ParseState *pstate) +{ + ereport(ERROR, + errcode(ERRCODE_SYNTAX_ERROR), + errmsg("conflicting or redundant options"), + parser_errposition(pstate, defel->location)); +} diff --git a/src/backend/commands/extension.c b/src/backend/commands/extension.c index b37009bfec4..eaa76af47b0 100644 --- a/src/backend/commands/extension.c +++ b/src/backend/commands/extension.c @@ -1731,30 +1731,21 @@ CreateExtension(ParseState *pstate, CreateExtensionStmt *stmt) if (strcmp(defel->defname, "schema") == 0) { if (d_schema) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("conflicting or redundant options"), - parser_errposition(pstate, defel->location))); + errorConflictingDefElem(defel, pstate); d_schema = defel; schemaName = defGetString(d_schema); } else if (strcmp(defel->defname, "new_version") == 0) { if (d_new_version) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("conflicting or redundant options"), - parser_errposition(pstate, defel->location))); + errorConflictingDefElem(defel, pstate); d_new_version = defel; versionName = defGetString(d_new_version); } else if (strcmp(defel->defname, "cascade") == 0) { if (d_cascade) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("conflicting or redundant options"), - parser_errposition(pstate, defel->location))); + errorConflictingDefElem(defel, pstate); d_cascade = defel; cascade = defGetBoolean(d_cascade); } @@ -3051,10 +3042,7 @@ ExecAlterExtensionStmt(ParseState *pstate, AlterExtensionStmt *stmt) if (strcmp(defel->defname, "new_version") == 0) { if (d_new_version) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("conflicting or redundant options"), - parser_errposition(pstate, defel->location))); + errorConflictingDefElem(defel, pstate); d_new_version = defel; } else diff --git a/src/backend/commands/foreigncmds.c b/src/backend/commands/foreigncmds.c index bc36311d383..9b71beb1d38 100644 --- a/src/backend/commands/foreigncmds.c +++ b/src/backend/commands/foreigncmds.c @@ -515,7 +515,7 @@ lookup_fdw_validator_func(DefElem *validator) * Process function options of CREATE/ALTER FDW */ static void -parse_func_options(List *func_options, +parse_func_options(ParseState *pstate, List *func_options, bool *handler_given, Oid *fdwhandler, bool *validator_given, Oid *fdwvalidator) { @@ -534,18 +534,14 @@ parse_func_options(List *func_options, if (strcmp(def->defname, "handler") == 0) { if (*handler_given) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("conflicting or redundant options"))); + errorConflictingDefElem(def, pstate); *handler_given = true; *fdwhandler = lookup_fdw_handler_func(def); } else if (strcmp(def->defname, "validator") == 0) { if (*validator_given) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("conflicting or redundant options"))); + errorConflictingDefElem(def, pstate); *validator_given = true; *fdwvalidator = lookup_fdw_validator_func(def); } @@ -559,7 +555,7 @@ parse_func_options(List *func_options, * Create a foreign-data wrapper */ ObjectAddress -CreateForeignDataWrapper(CreateFdwStmt *stmt) +CreateForeignDataWrapper(ParseState *pstate, CreateFdwStmt *stmt) { Relation rel; Datum values[Natts_pg_foreign_data_wrapper]; @@ -611,7 +607,7 @@ CreateForeignDataWrapper(CreateFdwStmt *stmt) values[Anum_pg_foreign_data_wrapper_fdwowner - 1] = ObjectIdGetDatum(ownerId); /* Lookup handler and validator functions, if given */ - parse_func_options(stmt->func_options, + parse_func_options(pstate, stmt->func_options, &handler_given, &fdwhandler, &validator_given, &fdwvalidator); @@ -675,7 +671,7 @@ CreateForeignDataWrapper(CreateFdwStmt *stmt) * Alter foreign-data wrapper */ ObjectAddress -AlterForeignDataWrapper(AlterFdwStmt *stmt) +AlterForeignDataWrapper(ParseState *pstate, AlterFdwStmt *stmt) { Relation rel; HeapTuple tp; @@ -717,7 +713,7 @@ AlterForeignDataWrapper(AlterFdwStmt *stmt) memset(repl_null, false, sizeof(repl_null)); memset(repl_repl, false, sizeof(repl_repl)); - parse_func_options(stmt->func_options, + parse_func_options(pstate, stmt->func_options, &handler_given, &fdwhandler, &validator_given, &fdwvalidator); diff --git a/src/backend/commands/functioncmds.c b/src/backend/commands/functioncmds.c index 736d04780a7..79d875ab10b 100644 --- a/src/backend/commands/functioncmds.c +++ b/src/backend/commands/functioncmds.c @@ -523,7 +523,7 @@ compute_common_attribute(ParseState *pstate, if (is_procedure) goto procedure_error; if (*volatility_item) - goto duplicate_error; + errorConflictingDefElem(defel, pstate); *volatility_item = defel; } @@ -532,14 +532,14 @@ compute_common_attribute(ParseState *pstate, if (is_procedure) goto procedure_error; if (*strict_item) - goto duplicate_error; + errorConflictingDefElem(defel, pstate); *strict_item = defel; } else if (strcmp(defel->defname, "security") == 0) { if (*security_item) - goto duplicate_error; + errorConflictingDefElem(defel, pstate); *security_item = defel; } @@ -548,7 +548,7 @@ compute_common_attribute(ParseState *pstate, if (is_procedure) goto procedure_error; if (*leakproof_item) - goto duplicate_error; + errorConflictingDefElem(defel, pstate); *leakproof_item = defel; } @@ -561,7 +561,7 @@ compute_common_attribute(ParseState *pstate, if (is_procedure) goto procedure_error; if (*cost_item) - goto duplicate_error; + errorConflictingDefElem(defel, pstate); *cost_item = defel; } @@ -570,7 +570,7 @@ compute_common_attribute(ParseState *pstate, if (is_procedure) goto procedure_error; if (*rows_item) - goto duplicate_error; + errorConflictingDefElem(defel, pstate); *rows_item = defel; } @@ -579,7 +579,7 @@ compute_common_attribute(ParseState *pstate, if (is_procedure) goto procedure_error; if (*support_item) - goto duplicate_error; + errorConflictingDefElem(defel, pstate); *support_item = defel; } @@ -588,7 +588,7 @@ compute_common_attribute(ParseState *pstate, if (is_procedure) goto procedure_error; if (*parallel_item) - goto duplicate_error; + errorConflictingDefElem(defel, pstate); *parallel_item = defel; } @@ -598,13 +598,6 @@ compute_common_attribute(ParseState *pstate, /* Recognized an option */ return true; -duplicate_error: - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("conflicting or redundant options"), - parser_errposition(pstate, defel->location))); - return false; /* keep compiler quiet */ - procedure_error: ereport(ERROR, (errcode(ERRCODE_INVALID_FUNCTION_DEFINITION), @@ -765,37 +758,25 @@ compute_function_attributes(ParseState *pstate, if (strcmp(defel->defname, "as") == 0) { if (as_item) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("conflicting or redundant options"), - parser_errposition(pstate, defel->location))); + errorConflictingDefElem(defel, pstate); as_item = defel; } else if (strcmp(defel->defname, "language") == 0) { if (language_item) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("conflicting or redundant options"), - parser_errposition(pstate, defel->location))); + errorConflictingDefElem(defel, pstate); language_item = defel; } else if (strcmp(defel->defname, "transform") == 0) { if (transform_item) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("conflicting or redundant options"), - parser_errposition(pstate, defel->location))); + errorConflictingDefElem(defel, pstate); transform_item = defel; } else if (strcmp(defel->defname, "window") == 0) { if (windowfunc_item) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("conflicting or redundant options"), - parser_errposition(pstate, defel->location))); + errorConflictingDefElem(defel, pstate); if (is_procedure) ereport(ERROR, (errcode(ERRCODE_INVALID_FUNCTION_DEFINITION), @@ -2070,7 +2051,7 @@ IsThereFunctionInNamespace(const char *proname, int pronargs, * See at ExecuteCallStmt() about the atomic argument. */ void -ExecuteDoStmt(DoStmt *stmt, bool atomic) +ExecuteDoStmt(ParseState *pstate, DoStmt *stmt, bool atomic) { InlineCodeBlock *codeblock = makeNode(InlineCodeBlock); ListCell *arg; @@ -2089,17 +2070,13 @@ ExecuteDoStmt(DoStmt *stmt, bool atomic) if (strcmp(defel->defname, "as") == 0) { if (as_item) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("conflicting or redundant options"))); + errorConflictingDefElem(defel, pstate); as_item = defel; } else if (strcmp(defel->defname, "language") == 0) { if (language_item) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("conflicting or redundant options"))); + errorConflictingDefElem(defel, pstate); language_item = defel; } else diff --git a/src/backend/commands/publicationcmds.c b/src/backend/commands/publicationcmds.c index 95c253c8e08..12f9f8b6974 100644 --- a/src/backend/commands/publicationcmds.c +++ b/src/backend/commands/publicationcmds.c @@ -55,7 +55,8 @@ static void PublicationAddTables(Oid pubid, List *rels, bool if_not_exists, static void PublicationDropTables(Oid pubid, List *rels, bool missing_ok); static void -parse_publication_options(List *options, +parse_publication_options(ParseState *pstate, + List *options, bool *publish_given, PublicationActions *pubactions, bool *publish_via_partition_root_given, @@ -85,9 +86,7 @@ parse_publication_options(List *options, ListCell *lc; if (*publish_given) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("conflicting or redundant options"))); + errorConflictingDefElem(defel, pstate); /* * If publish option was given only the explicitly listed actions @@ -128,9 +127,7 @@ parse_publication_options(List *options, else if (strcmp(defel->defname, "publish_via_partition_root") == 0) { if (*publish_via_partition_root_given) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("conflicting or redundant options"))); + errorConflictingDefElem(defel, pstate); *publish_via_partition_root_given = true; *publish_via_partition_root = defGetBoolean(defel); } @@ -145,7 +142,7 @@ parse_publication_options(List *options, * Create new publication. */ ObjectAddress -CreatePublication(CreatePublicationStmt *stmt) +CreatePublication(ParseState *pstate, CreatePublicationStmt *stmt) { Relation rel; ObjectAddress myself; @@ -192,7 +189,8 @@ CreatePublication(CreatePublicationStmt *stmt) DirectFunctionCall1(namein, CStringGetDatum(stmt->pubname)); values[Anum_pg_publication_pubowner - 1] = ObjectIdGetDatum(GetUserId()); - parse_publication_options(stmt->options, + parse_publication_options(pstate, + stmt->options, &publish_given, &pubactions, &publish_via_partition_root_given, &publish_via_partition_root); @@ -256,8 +254,8 @@ CreatePublication(CreatePublicationStmt *stmt) * Change options of a publication. */ static void -AlterPublicationOptions(AlterPublicationStmt *stmt, Relation rel, - HeapTuple tup) +AlterPublicationOptions(ParseState *pstate, AlterPublicationStmt *stmt, + Relation rel, HeapTuple tup) { bool nulls[Natts_pg_publication]; bool replaces[Natts_pg_publication]; @@ -269,7 +267,8 @@ AlterPublicationOptions(AlterPublicationStmt *stmt, Relation rel, ObjectAddress obj; Form_pg_publication pubform; - parse_publication_options(stmt->options, + parse_publication_options(pstate, + stmt->options, &publish_given, &pubactions, &publish_via_partition_root_given, &publish_via_partition_root); @@ -434,7 +433,7 @@ AlterPublicationTables(AlterPublicationStmt *stmt, Relation rel, * AlterPublicationTables. */ void -AlterPublication(AlterPublicationStmt *stmt) +AlterPublication(ParseState *pstate, AlterPublicationStmt *stmt) { Relation rel; HeapTuple tup; @@ -459,7 +458,7 @@ AlterPublication(AlterPublicationStmt *stmt) stmt->pubname); if (stmt->options) - AlterPublicationOptions(stmt, rel, tup); + AlterPublicationOptions(pstate, stmt, rel, tup); else AlterPublicationTables(stmt, rel, tup); diff --git a/src/backend/commands/sequence.c b/src/backend/commands/sequence.c index d22d767d2fa..72bfdc07a49 100644 --- a/src/backend/commands/sequence.c +++ b/src/backend/commands/sequence.c @@ -1261,90 +1261,63 @@ init_params(ParseState *pstate, List *options, bool for_identity, if (strcmp(defel->defname, "as") == 0) { if (as_type) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("conflicting or redundant options"), - parser_errposition(pstate, defel->location))); + errorConflictingDefElem(defel, pstate); as_type = defel; *need_seq_rewrite = true; } else if (strcmp(defel->defname, "increment") == 0) { if (increment_by) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("conflicting or redundant options"), - parser_errposition(pstate, defel->location))); + errorConflictingDefElem(defel, pstate); increment_by = defel; *need_seq_rewrite = true; } else if (strcmp(defel->defname, "start") == 0) { if (start_value) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("conflicting or redundant options"), - parser_errposition(pstate, defel->location))); + errorConflictingDefElem(defel, pstate); start_value = defel; *need_seq_rewrite = true; } else if (strcmp(defel->defname, "restart") == 0) { if (restart_value) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("conflicting or redundant options"), - parser_errposition(pstate, defel->location))); + errorConflictingDefElem(defel, pstate); restart_value = defel; *need_seq_rewrite = true; } else if (strcmp(defel->defname, "maxvalue") == 0) { if (max_value) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("conflicting or redundant options"), - parser_errposition(pstate, defel->location))); + errorConflictingDefElem(defel, pstate); max_value = defel; *need_seq_rewrite = true; } else if (strcmp(defel->defname, "minvalue") == 0) { if (min_value) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("conflicting or redundant options"), - parser_errposition(pstate, defel->location))); + errorConflictingDefElem(defel, pstate); min_value = defel; *need_seq_rewrite = true; } else if (strcmp(defel->defname, "cache") == 0) { if (cache_value) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("conflicting or redundant options"), - parser_errposition(pstate, defel->location))); + errorConflictingDefElem(defel, pstate); cache_value = defel; *need_seq_rewrite = true; } else if (strcmp(defel->defname, "cycle") == 0) { if (is_cycled) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("conflicting or redundant options"), - parser_errposition(pstate, defel->location))); + errorConflictingDefElem(defel, pstate); is_cycled = defel; *need_seq_rewrite = true; } else if (strcmp(defel->defname, "owned_by") == 0) { if (*owned_by) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("conflicting or redundant options"), - parser_errposition(pstate, defel->location))); + errorConflictingDefElem(defel, pstate); *owned_by = defGetQualifiedName(defel); } else if (strcmp(defel->defname, "sequence_name") == 0) diff --git a/src/backend/commands/subscriptioncmds.c b/src/backend/commands/subscriptioncmds.c index 5f834a9c300..239d263f835 100644 --- a/src/backend/commands/subscriptioncmds.c +++ b/src/backend/commands/subscriptioncmds.c @@ -98,7 +98,8 @@ static void ReportSlotConnectionError(List *rstates, Oid subid, char *slotname, * Caller is expected to have cleared 'opts'. */ static void -parse_subscription_options(List *stmt_options, bits32 supported_opts, SubOpts *opts) +parse_subscription_options(ParseState *pstate, List *stmt_options, + bits32 supported_opts, SubOpts *opts) { ListCell *lc; @@ -137,9 +138,7 @@ parse_subscription_options(List *stmt_options, bits32 supported_opts, SubOpts *o strcmp(defel->defname, "connect") == 0) { if (IsSet(opts->specified_opts, SUBOPT_CONNECT)) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("conflicting or redundant options"))); + errorConflictingDefElem(defel, pstate); opts->specified_opts |= SUBOPT_CONNECT; opts->connect = defGetBoolean(defel); @@ -148,9 +147,7 @@ parse_subscription_options(List *stmt_options, bits32 supported_opts, SubOpts *o strcmp(defel->defname, "enabled") == 0) { if (IsSet(opts->specified_opts, SUBOPT_ENABLED)) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("conflicting or redundant options"))); + errorConflictingDefElem(defel, pstate); opts->specified_opts |= SUBOPT_ENABLED; opts->enabled = defGetBoolean(defel); @@ -159,9 +156,7 @@ parse_subscription_options(List *stmt_options, bits32 supported_opts, SubOpts *o strcmp(defel->defname, "create_slot") == 0) { if (IsSet(opts->specified_opts, SUBOPT_CREATE_SLOT)) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("conflicting or redundant options"))); + errorConflictingDefElem(defel, pstate); opts->specified_opts |= SUBOPT_CREATE_SLOT; opts->create_slot = defGetBoolean(defel); @@ -170,9 +165,7 @@ parse_subscription_options(List *stmt_options, bits32 supported_opts, SubOpts *o strcmp(defel->defname, "slot_name") == 0) { if (IsSet(opts->specified_opts, SUBOPT_SLOT_NAME)) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("conflicting or redundant options"))); + errorConflictingDefElem(defel, pstate); opts->specified_opts |= SUBOPT_SLOT_NAME; opts->slot_name = defGetString(defel); @@ -185,9 +178,7 @@ parse_subscription_options(List *stmt_options, bits32 supported_opts, SubOpts *o strcmp(defel->defname, "copy_data") == 0) { if (IsSet(opts->specified_opts, SUBOPT_COPY_DATA)) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("conflicting or redundant options"))); + errorConflictingDefElem(defel, pstate); opts->specified_opts |= SUBOPT_COPY_DATA; opts->copy_data = defGetBoolean(defel); @@ -196,9 +187,7 @@ parse_subscription_options(List *stmt_options, bits32 supported_opts, SubOpts *o strcmp(defel->defname, "synchronous_commit") == 0) { if (IsSet(opts->specified_opts, SUBOPT_SYNCHRONOUS_COMMIT)) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("conflicting or redundant options"))); + errorConflictingDefElem(defel, pstate); opts->specified_opts |= SUBOPT_SYNCHRONOUS_COMMIT; opts->synchronous_commit = defGetString(defel); @@ -212,9 +201,7 @@ parse_subscription_options(List *stmt_options, bits32 supported_opts, SubOpts *o strcmp(defel->defname, "refresh") == 0) { if (IsSet(opts->specified_opts, SUBOPT_REFRESH)) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("conflicting or redundant options"))); + errorConflictingDefElem(defel, pstate); opts->specified_opts |= SUBOPT_REFRESH; opts->refresh = defGetBoolean(defel); @@ -223,9 +210,7 @@ parse_subscription_options(List *stmt_options, bits32 supported_opts, SubOpts *o strcmp(defel->defname, "binary") == 0) { if (IsSet(opts->specified_opts, SUBOPT_BINARY)) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("conflicting or redundant options"))); + errorConflictingDefElem(defel, pstate); opts->specified_opts |= SUBOPT_BINARY; opts->binary = defGetBoolean(defel); @@ -234,9 +219,7 @@ parse_subscription_options(List *stmt_options, bits32 supported_opts, SubOpts *o strcmp(defel->defname, "streaming") == 0) { if (IsSet(opts->specified_opts, SUBOPT_STREAMING)) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("conflicting or redundant options"))); + errorConflictingDefElem(defel, pstate); opts->specified_opts |= SUBOPT_STREAMING; opts->streaming = defGetBoolean(defel); @@ -257,9 +240,7 @@ parse_subscription_options(List *stmt_options, bits32 supported_opts, SubOpts *o errmsg("unrecognized subscription parameter: \"%s\"", defel->defname))); if (IsSet(opts->specified_opts, SUBOPT_TWOPHASE_COMMIT)) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("conflicting or redundant options"))); + errorConflictingDefElem(defel, pstate); opts->specified_opts |= SUBOPT_TWOPHASE_COMMIT; opts->twophase = defGetBoolean(defel); @@ -408,7 +389,8 @@ publicationListToArray(List *publist) * Create new subscription. */ ObjectAddress -CreateSubscription(CreateSubscriptionStmt *stmt, bool isTopLevel) +CreateSubscription(ParseState *pstate, CreateSubscriptionStmt *stmt, + bool isTopLevel) { Relation rel; ObjectAddress myself; @@ -432,7 +414,7 @@ CreateSubscription(CreateSubscriptionStmt *stmt, bool isTopLevel) SUBOPT_SLOT_NAME | SUBOPT_COPY_DATA | SUBOPT_SYNCHRONOUS_COMMIT | SUBOPT_BINARY | SUBOPT_STREAMING | SUBOPT_TWOPHASE_COMMIT); - parse_subscription_options(stmt->options, supported_opts, &opts); + parse_subscription_options(pstate, stmt->options, supported_opts, &opts); /* * Since creating a replication slot is not transactional, rolling back @@ -853,7 +835,8 @@ AlterSubscription_refresh(Subscription *sub, bool copy_data) * Alter the existing subscription. */ ObjectAddress -AlterSubscription(AlterSubscriptionStmt *stmt, bool isTopLevel) +AlterSubscription(ParseState *pstate, AlterSubscriptionStmt *stmt, + bool isTopLevel) { Relation rel; ObjectAddress myself; @@ -906,7 +889,8 @@ AlterSubscription(AlterSubscriptionStmt *stmt, bool isTopLevel) SUBOPT_SYNCHRONOUS_COMMIT | SUBOPT_BINARY | SUBOPT_STREAMING); - parse_subscription_options(stmt->options, supported_opts, &opts); + parse_subscription_options(pstate, stmt->options, + supported_opts, &opts); if (IsSet(opts.specified_opts, SUBOPT_SLOT_NAME)) { @@ -957,7 +941,8 @@ AlterSubscription(AlterSubscriptionStmt *stmt, bool isTopLevel) case ALTER_SUBSCRIPTION_ENABLED: { - parse_subscription_options(stmt->options, SUBOPT_ENABLED, &opts); + parse_subscription_options(pstate, stmt->options, + SUBOPT_ENABLED, &opts); Assert(IsSet(opts.specified_opts, SUBOPT_ENABLED)); if (!sub->slotname && opts.enabled) @@ -991,7 +976,8 @@ AlterSubscription(AlterSubscriptionStmt *stmt, bool isTopLevel) case ALTER_SUBSCRIPTION_SET_PUBLICATION: { supported_opts = SUBOPT_COPY_DATA | SUBOPT_REFRESH; - parse_subscription_options(stmt->options, supported_opts, &opts); + parse_subscription_options(pstate, stmt->options, + supported_opts, &opts); values[Anum_pg_subscription_subpublications - 1] = publicationListToArray(stmt->publication); @@ -1040,7 +1026,8 @@ AlterSubscription(AlterSubscriptionStmt *stmt, bool isTopLevel) if (isadd) supported_opts |= SUBOPT_COPY_DATA; - parse_subscription_options(stmt->options, supported_opts, &opts); + parse_subscription_options(pstate, stmt->options, + supported_opts, &opts); publist = merge_publications(sub->publications, stmt->publication, isadd, stmt->subname); values[Anum_pg_subscription_subpublications - 1] = @@ -1087,7 +1074,8 @@ AlterSubscription(AlterSubscriptionStmt *stmt, bool isTopLevel) (errcode(ERRCODE_OBJECT_NOT_IN_PREREQUISITE_STATE), errmsg("ALTER SUBSCRIPTION ... REFRESH is not allowed for disabled subscriptions"))); - parse_subscription_options(stmt->options, SUBOPT_COPY_DATA, &opts); + parse_subscription_options(pstate, stmt->options, + SUBOPT_COPY_DATA, &opts); /* * The subscription option "two_phase" requires that diff --git a/src/backend/commands/typecmds.c b/src/backend/commands/typecmds.c index 58ec65c6afc..93eeff950b1 100644 --- a/src/backend/commands/typecmds.c +++ b/src/backend/commands/typecmds.c @@ -330,10 +330,7 @@ DefineType(ParseState *pstate, List *names, List *parameters) continue; } if (*defelp != NULL) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("conflicting or redundant options"), - parser_errposition(pstate, defel->location))); + errorConflictingDefElem(defel, pstate); *defelp = defel; } @@ -1336,7 +1333,7 @@ checkEnumOwner(HeapTuple tup) * and users might have queries with that same assumption. */ ObjectAddress -DefineRange(CreateRangeStmt *stmt) +DefineRange(ParseState *pstate, CreateRangeStmt *stmt) { char *typeName; Oid typeNamespace; @@ -1411,50 +1408,38 @@ DefineRange(CreateRangeStmt *stmt) if (strcmp(defel->defname, "subtype") == 0) { if (OidIsValid(rangeSubtype)) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("conflicting or redundant options"))); + errorConflictingDefElem(defel, pstate); /* we can look up the subtype name immediately */ rangeSubtype = typenameTypeId(NULL, defGetTypeName(defel)); } else if (strcmp(defel->defname, "subtype_opclass") == 0) { if (rangeSubOpclassName != NIL) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("conflicting or redundant options"))); + errorConflictingDefElem(defel, pstate); rangeSubOpclassName = defGetQualifiedName(defel); } else if (strcmp(defel->defname, "collation") == 0) { if (rangeCollationName != NIL) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("conflicting or redundant options"))); + errorConflictingDefElem(defel, pstate); rangeCollationName = defGetQualifiedName(defel); } else if (strcmp(defel->defname, "canonical") == 0) { if (rangeCanonicalName != NIL) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("conflicting or redundant options"))); + errorConflictingDefElem(defel, pstate); rangeCanonicalName = defGetQualifiedName(defel); } else if (strcmp(defel->defname, "subtype_diff") == 0) { if (rangeSubtypeDiffName != NIL) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("conflicting or redundant options"))); + errorConflictingDefElem(defel, pstate); rangeSubtypeDiffName = defGetQualifiedName(defel); } else if (strcmp(defel->defname, "multirange_type_name") == 0) { if (multirangeTypeName != NULL) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("conflicting or redundant options"))); + errorConflictingDefElem(defel, pstate); /* we can look up the subtype name immediately */ multirangeNamespace = QualifiedNameGetCreationNamespace(defGetQualifiedName(defel), &multirangeTypeName); diff --git a/src/backend/commands/user.c b/src/backend/commands/user.c index 65bb7339589..aa69821be49 100644 --- a/src/backend/commands/user.c +++ b/src/backend/commands/user.c @@ -27,6 +27,7 @@ #include "catalog/pg_db_role_setting.h" #include "commands/comment.h" #include "commands/dbcommands.h" +#include "commands/defrem.h" #include "commands/seclabel.h" #include "commands/user.h" #include "libpq/crypt.h" @@ -128,10 +129,7 @@ CreateRole(ParseState *pstate, CreateRoleStmt *stmt) if (strcmp(defel->defname, "password") == 0) { if (dpassword) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("conflicting or redundant options"), - parser_errposition(pstate, defel->location))); + errorConflictingDefElem(defel, pstate); dpassword = defel; } else if (strcmp(defel->defname, "sysid") == 0) @@ -142,109 +140,73 @@ CreateRole(ParseState *pstate, CreateRoleStmt *stmt) else if (strcmp(defel->defname, "superuser") == 0) { if (dissuper) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("conflicting or redundant options"), - parser_errposition(pstate, defel->location))); + errorConflictingDefElem(defel, pstate); dissuper = defel; } else if (strcmp(defel->defname, "inherit") == 0) { if (dinherit) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("conflicting or redundant options"), - parser_errposition(pstate, defel->location))); + errorConflictingDefElem(defel, pstate); dinherit = defel; } else if (strcmp(defel->defname, "createrole") == 0) { if (dcreaterole) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("conflicting or redundant options"), - parser_errposition(pstate, defel->location))); + errorConflictingDefElem(defel, pstate); dcreaterole = defel; } else if (strcmp(defel->defname, "createdb") == 0) { if (dcreatedb) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("conflicting or redundant options"), - parser_errposition(pstate, defel->location))); + errorConflictingDefElem(defel, pstate); dcreatedb = defel; } else if (strcmp(defel->defname, "canlogin") == 0) { if (dcanlogin) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("conflicting or redundant options"), - parser_errposition(pstate, defel->location))); + errorConflictingDefElem(defel, pstate); dcanlogin = defel; } else if (strcmp(defel->defname, "isreplication") == 0) { if (disreplication) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("conflicting or redundant options"), - parser_errposition(pstate, defel->location))); + errorConflictingDefElem(defel, pstate); disreplication = defel; } else if (strcmp(defel->defname, "connectionlimit") == 0) { if (dconnlimit) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("conflicting or redundant options"), - parser_errposition(pstate, defel->location))); + errorConflictingDefElem(defel, pstate); dconnlimit = defel; } else if (strcmp(defel->defname, "addroleto") == 0) { if (daddroleto) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("conflicting or redundant options"), - parser_errposition(pstate, defel->location))); + errorConflictingDefElem(defel, pstate); daddroleto = defel; } else if (strcmp(defel->defname, "rolemembers") == 0) { if (drolemembers) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("conflicting or redundant options"), - parser_errposition(pstate, defel->location))); + errorConflictingDefElem(defel, pstate); drolemembers = defel; } else if (strcmp(defel->defname, "adminmembers") == 0) { if (dadminmembers) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("conflicting or redundant options"), - parser_errposition(pstate, defel->location))); + errorConflictingDefElem(defel, pstate); dadminmembers = defel; } else if (strcmp(defel->defname, "validUntil") == 0) { if (dvalidUntil) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("conflicting or redundant options"), - parser_errposition(pstate, defel->location))); + errorConflictingDefElem(defel, pstate); dvalidUntil = defel; } else if (strcmp(defel->defname, "bypassrls") == 0) { if (dbypassRLS) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("conflicting or redundant options"), - parser_errposition(pstate, defel->location))); + errorConflictingDefElem(defel, pstate); dbypassRLS = defel; } else @@ -528,7 +490,7 @@ CreateRole(ParseState *pstate, CreateRoleStmt *stmt) * "ALTER ROLE role ROLE rolenames", we don't document it. */ Oid -AlterRole(AlterRoleStmt *stmt) +AlterRole(ParseState *pstate, AlterRoleStmt *stmt) { Datum new_record[Natts_pg_authid]; bool new_record_nulls[Natts_pg_authid]; @@ -577,90 +539,68 @@ AlterRole(AlterRoleStmt *stmt) if (strcmp(defel->defname, "password") == 0) { if (dpassword) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("conflicting or redundant options"))); + errorConflictingDefElem(defel, pstate); dpassword = defel; } else if (strcmp(defel->defname, "superuser") == 0) { if (dissuper) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("conflicting or redundant options"))); + errorConflictingDefElem(defel, pstate); dissuper = defel; } else if (strcmp(defel->defname, "inherit") == 0) { if (dinherit) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("conflicting or redundant options"))); + errorConflictingDefElem(defel, pstate); dinherit = defel; } else if (strcmp(defel->defname, "createrole") == 0) { if (dcreaterole) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("conflicting or redundant options"))); + errorConflictingDefElem(defel, pstate); dcreaterole = defel; } else if (strcmp(defel->defname, "createdb") == 0) { if (dcreatedb) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("conflicting or redundant options"))); + errorConflictingDefElem(defel, pstate); dcreatedb = defel; } else if (strcmp(defel->defname, "canlogin") == 0) { if (dcanlogin) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("conflicting or redundant options"))); + errorConflictingDefElem(defel, pstate); dcanlogin = defel; } else if (strcmp(defel->defname, "isreplication") == 0) { if (disreplication) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("conflicting or redundant options"))); + errorConflictingDefElem(defel, pstate); disreplication = defel; } else if (strcmp(defel->defname, "connectionlimit") == 0) { if (dconnlimit) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("conflicting or redundant options"))); + errorConflictingDefElem(defel, pstate); dconnlimit = defel; } else if (strcmp(defel->defname, "rolemembers") == 0 && stmt->action != 0) { if (drolemembers) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("conflicting or redundant options"))); + errorConflictingDefElem(defel, pstate); drolemembers = defel; } else if (strcmp(defel->defname, "validUntil") == 0) { if (dvalidUntil) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("conflicting or redundant options"))); + errorConflictingDefElem(defel, pstate); dvalidUntil = defel; } else if (strcmp(defel->defname, "bypassrls") == 0) { if (dbypassRLS) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("conflicting or redundant options"))); + errorConflictingDefElem(defel, pstate); dbypassRLS = defel; } else diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c index 3afcd6b5118..9edd1f8d51b 100644 --- a/src/backend/parser/parse_utilcmd.c +++ b/src/backend/parser/parse_utilcmd.c @@ -392,9 +392,7 @@ generateSerialExtraStmts(CreateStmtContext *cxt, ColumnDef *column, if (strcmp(defel->defname, "sequence_name") == 0) { if (nameEl) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("conflicting or redundant options"))); + errorConflictingDefElem(defel, cxt->pstate); nameEl = defel; nameEl_idx = foreach_current_index(option); } diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c index 7a2da9dab43..27fbf1f3aae 100644 --- a/src/backend/tcop/utility.c +++ b/src/backend/tcop/utility.c @@ -708,7 +708,7 @@ standard_ProcessUtility(PlannedStmt *pstmt, break; case T_DoStmt: - ExecuteDoStmt((DoStmt *) parsetree, isAtomicContext); + ExecuteDoStmt(pstate, (DoStmt *) parsetree, isAtomicContext); break; case T_CreateTableSpaceStmt: @@ -888,7 +888,7 @@ standard_ProcessUtility(PlannedStmt *pstmt, case T_AlterRoleStmt: /* no event triggers for global objects */ - AlterRole((AlterRoleStmt *) parsetree); + AlterRole(pstate, (AlterRoleStmt *) parsetree); break; case T_AlterRoleSetStmt: @@ -1552,11 +1552,11 @@ ProcessUtilitySlow(ParseState *pstate, break; case T_CreateFdwStmt: - address = CreateForeignDataWrapper((CreateFdwStmt *) parsetree); + address = CreateForeignDataWrapper(pstate, (CreateFdwStmt *) parsetree); break; case T_AlterFdwStmt: - address = AlterForeignDataWrapper((AlterFdwStmt *) parsetree); + address = AlterForeignDataWrapper(pstate, (AlterFdwStmt *) parsetree); break; case T_CreateForeignServerStmt: @@ -1601,7 +1601,7 @@ ProcessUtilitySlow(ParseState *pstate, break; case T_CreateRangeStmt: /* CREATE TYPE AS RANGE */ - address = DefineRange((CreateRangeStmt *) parsetree); + address = DefineRange(pstate, (CreateRangeStmt *) parsetree); break; case T_AlterEnumStmt: /* ALTER TYPE (enum) */ @@ -1802,11 +1802,11 @@ ProcessUtilitySlow(ParseState *pstate, break; case T_CreatePublicationStmt: - address = CreatePublication((CreatePublicationStmt *) parsetree); + address = CreatePublication(pstate, (CreatePublicationStmt *) parsetree); break; case T_AlterPublicationStmt: - AlterPublication((AlterPublicationStmt *) parsetree); + AlterPublication(pstate, (AlterPublicationStmt *) parsetree); /* * AlterPublication calls EventTriggerCollectSimpleCommand @@ -1816,12 +1816,14 @@ ProcessUtilitySlow(ParseState *pstate, break; case T_CreateSubscriptionStmt: - address = CreateSubscription((CreateSubscriptionStmt *) parsetree, + address = CreateSubscription(pstate, + (CreateSubscriptionStmt *) parsetree, isTopLevel); break; case T_AlterSubscriptionStmt: - address = AlterSubscription((AlterSubscriptionStmt *) parsetree, + address = AlterSubscription(pstate, + (AlterSubscriptionStmt *) parsetree, isTopLevel); break; diff --git a/src/include/commands/defrem.h b/src/include/commands/defrem.h index 42bf1c75198..f84d09959cc 100644 --- a/src/include/commands/defrem.h +++ b/src/include/commands/defrem.h @@ -56,7 +56,7 @@ extern ObjectAddress CreateCast(CreateCastStmt *stmt); extern ObjectAddress CreateTransform(CreateTransformStmt *stmt); extern void IsThereFunctionInNamespace(const char *proname, int pronargs, oidvector *proargtypes, Oid nspOid); -extern void ExecuteDoStmt(DoStmt *stmt, bool atomic); +extern void ExecuteDoStmt(ParseState *pstate, DoStmt *stmt, bool atomic); extern void ExecuteCallStmt(CallStmt *stmt, ParamListInfo params, bool atomic, DestReceiver *dest); extern TupleDesc CallStmtResultDesc(CallStmt *stmt); extern Oid get_transform_oid(Oid type_id, Oid lang_id, bool missing_ok); @@ -121,8 +121,8 @@ extern ObjectAddress AlterForeignServerOwner(const char *name, Oid newOwnerId); extern void AlterForeignServerOwner_oid(Oid, Oid newOwnerId); extern ObjectAddress AlterForeignDataWrapperOwner(const char *name, Oid newOwnerId); extern void AlterForeignDataWrapperOwner_oid(Oid fwdId, Oid newOwnerId); -extern ObjectAddress CreateForeignDataWrapper(CreateFdwStmt *stmt); -extern ObjectAddress AlterForeignDataWrapper(AlterFdwStmt *stmt); +extern ObjectAddress CreateForeignDataWrapper(ParseState *pstate, CreateFdwStmt *stmt); +extern ObjectAddress AlterForeignDataWrapper(ParseState *pstate, AlterFdwStmt *stmt); extern ObjectAddress CreateForeignServer(CreateForeignServerStmt *stmt); extern ObjectAddress AlterForeignServer(AlterForeignServerStmt *stmt); extern ObjectAddress CreateUserMapping(CreateUserMappingStmt *stmt); @@ -153,5 +153,6 @@ extern List *defGetQualifiedName(DefElem *def); extern TypeName *defGetTypeName(DefElem *def); extern int defGetTypeLength(DefElem *def); extern List *defGetStringList(DefElem *def); +extern void errorConflictingDefElem(DefElem *defel, ParseState *pstate) pg_attribute_noreturn(); #endif /* DEFREM_H */ diff --git a/src/include/commands/publicationcmds.h b/src/include/commands/publicationcmds.h index 00e2e626e6c..efea01f2a93 100644 --- a/src/include/commands/publicationcmds.h +++ b/src/include/commands/publicationcmds.h @@ -18,8 +18,8 @@ #include "catalog/objectaddress.h" #include "nodes/parsenodes.h" -extern ObjectAddress CreatePublication(CreatePublicationStmt *stmt); -extern void AlterPublication(AlterPublicationStmt *stmt); +extern ObjectAddress CreatePublication(ParseState *pstate, CreatePublicationStmt *stmt); +extern void AlterPublication(ParseState *pstate, AlterPublicationStmt *stmt); extern void RemovePublicationRelById(Oid proid); extern ObjectAddress AlterPublicationOwner(const char *name, Oid newOwnerId); diff --git a/src/include/commands/subscriptioncmds.h b/src/include/commands/subscriptioncmds.h index 3b926f35d76..8bf25ee66c2 100644 --- a/src/include/commands/subscriptioncmds.h +++ b/src/include/commands/subscriptioncmds.h @@ -18,9 +18,9 @@ #include "catalog/objectaddress.h" #include "nodes/parsenodes.h" -extern ObjectAddress CreateSubscription(CreateSubscriptionStmt *stmt, +extern ObjectAddress CreateSubscription(ParseState *pstate, CreateSubscriptionStmt *stmt, bool isTopLevel); -extern ObjectAddress AlterSubscription(AlterSubscriptionStmt *stmt, bool isTopLevel); +extern ObjectAddress AlterSubscription(ParseState *pstate, AlterSubscriptionStmt *stmt, bool isTopLevel); extern void DropSubscription(DropSubscriptionStmt *stmt, bool isTopLevel); extern ObjectAddress AlterSubscriptionOwner(const char *name, Oid newOwnerId); diff --git a/src/include/commands/typecmds.h b/src/include/commands/typecmds.h index 880679127b5..d5e22811d0d 100644 --- a/src/include/commands/typecmds.h +++ b/src/include/commands/typecmds.h @@ -25,7 +25,7 @@ extern ObjectAddress DefineType(ParseState *pstate, List *names, List *parameter extern void RemoveTypeById(Oid typeOid); extern ObjectAddress DefineDomain(CreateDomainStmt *stmt); extern ObjectAddress DefineEnum(CreateEnumStmt *stmt); -extern ObjectAddress DefineRange(CreateRangeStmt *stmt); +extern ObjectAddress DefineRange(ParseState *pstate, CreateRangeStmt *stmt); extern ObjectAddress AlterEnum(AlterEnumStmt *stmt); extern ObjectAddress DefineCompositeType(RangeVar *typevar, List *coldeflist); extern Oid AssignTypeArrayOid(void); diff --git a/src/include/commands/user.h b/src/include/commands/user.h index 028e0dde568..0b7a3cd65fd 100644 --- a/src/include/commands/user.h +++ b/src/include/commands/user.h @@ -25,7 +25,7 @@ typedef void (*check_password_hook_type) (const char *username, const char *shad extern PGDLLIMPORT check_password_hook_type check_password_hook; extern Oid CreateRole(ParseState *pstate, CreateRoleStmt *stmt); -extern Oid AlterRole(AlterRoleStmt *stmt); +extern Oid AlterRole(ParseState *pstate, AlterRoleStmt *stmt); extern Oid AlterRoleSet(AlterRoleSetStmt *stmt); extern void DropRole(DropRoleStmt *stmt); extern void GrantRole(GrantRoleStmt *stmt); diff --git a/src/test/regress/expected/copy2.out b/src/test/regress/expected/copy2.out index c64f0719e7b..5f3685e9efe 100644 --- a/src/test/regress/expected/copy2.out +++ b/src/test/regress/expected/copy2.out @@ -67,6 +67,8 @@ LINE 1: COPY x from stdin (force_not_null (a), force_not_null (b)); ^ COPY x from stdin (force_null (a), force_null (b)); ERROR: conflicting or redundant options +LINE 1: COPY x from stdin (force_null (a), force_null (b)); + ^ COPY x from stdin (convert_selectively (a), convert_selectively (b)); ERROR: conflicting or redundant options LINE 1: COPY x from stdin (convert_selectively (a), convert_selectiv... diff --git a/src/test/regress/expected/foreign_data.out b/src/test/regress/expected/foreign_data.out index 809d40a79a9..426080ae39b 100644 --- a/src/test/regress/expected/foreign_data.out +++ b/src/test/regress/expected/foreign_data.out @@ -95,6 +95,8 @@ CREATE FOREIGN DATA WRAPPER test_fdw HANDLER invalid_fdw_handler; -- ERROR ERROR: function invalid_fdw_handler must return type fdw_handler CREATE FOREIGN DATA WRAPPER test_fdw HANDLER test_fdw_handler HANDLER invalid_fdw_handler; -- ERROR ERROR: conflicting or redundant options +LINE 1: ...GN DATA WRAPPER test_fdw HANDLER test_fdw_handler HANDLER in... + ^ CREATE FOREIGN DATA WRAPPER test_fdw HANDLER test_fdw_handler; DROP FOREIGN DATA WRAPPER test_fdw; -- ALTER FOREIGN DATA WRAPPER @@ -201,6 +203,8 @@ ALTER FOREIGN DATA WRAPPER foo HANDLER invalid_fdw_handler; -- ERROR ERROR: function invalid_fdw_handler must return type fdw_handler ALTER FOREIGN DATA WRAPPER foo HANDLER test_fdw_handler HANDLER anything; -- ERROR ERROR: conflicting or redundant options +LINE 1: ...FOREIGN DATA WRAPPER foo HANDLER test_fdw_handler HANDLER an... + ^ ALTER FOREIGN DATA WRAPPER foo HANDLER test_fdw_handler; WARNING: changing the foreign-data wrapper handler can change behavior of existing foreign tables DROP FUNCTION invalid_fdw_handler(); diff --git a/src/test/regress/expected/publication.out b/src/test/regress/expected/publication.out index 63d6ab7a4ef..b5b065a1b6f 100644 --- a/src/test/regress/expected/publication.out +++ b/src/test/regress/expected/publication.out @@ -27,6 +27,8 @@ CREATE PUBLICATION testpub_xxx WITH (publish = 'cluster, vacuum'); ERROR: unrecognized "publish" value: "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... + ^ \dRp List of publications Name | Owner | All tables | Inserts | Updates | Deletes | Truncates | Via root