On Mon, 12 Jul 2021 at 17:39, vignesh C <vignes...@gmail.com> wrote:
>
> Thanks for your comments, I have made the changes for the same in the
> V10 patch attached.
> Thoughts?
>
I'm still not happy about changing so many error messages.
Some of the changes might be OK, but aren't strictly necessary. For example:
COPY x from stdin (force_not_null (a), force_not_null (b));
-ERROR: conflicting or redundant options
+ERROR: option "force_not_null" specified more than once
LINE 1: COPY x from stdin (force_not_null (a), force_not_null (b));
^
I actually prefer the original primary error message, for consistency
with other similar cases, and I think the error position indicator is
sufficient to identify the problem. If it were to include the
"specified more than once" text, I would put that in DETAIL.
Other changes are wrong though. For example:
COPY x from stdin (format CSV, FORMAT CSV);
-ERROR: conflicting or redundant options
+ERROR: redundant options specified
LINE 1: COPY x from stdin (format CSV, FORMAT CSV);
^
The problem here is that the code that throws this error throws the
same error if the second format is different, which would make it a
conflicting option, not a redundant one. And I don't think we should
add more code to test whether it's conflicting or redundant, so again,
I think we should just keep the original error message.
Similarly, this error is wrong:
CREATE OR REPLACE FUNCTION foo() RETURNS int AS $$ SELECT 1 $$ STABLE IMMUTABLE;
ERROR: redundant options specified
LINE 1: ...NCTION foo() RETURNS int AS $$ SELECT 1 $$ STABLE IMMUTABLE;
^
And even this error:
CREATE OR REPLACE FUNCTION foo() RETURNS int AS $$ SELECT 1 $$ STRICT STRICT;
ERROR: redundant options specified
LINE 1: ... FUNCTION foo() RETURNS int AS $$ SELECT 1 $$ STRICT STRICT;
^
which looks OK, is actually problematic because the same code also
handles the alternate syntax, which leads to this (which is now wrong
because it's conflicting not redundant):
CREATE OR REPLACE FUNCTION foo() RETURNS int AS $$ SELECT 1 $$ STRICT
CALLED ON NULL INPUT;
ERROR: redundant options specified
LINE 1: ...NCTION foo() RETURNS int AS $$ SELECT 1 $$ STRICT CALLED ON ...
^
The problem is it's actually quite hard to decide in each case whether
the option is redundant or conflicting. Sometimes, it might look
obvious in the code, but actually be much more subtle, due to an
earlier transformation of the grammar. Likewise redundant doesn't
necessarily mean literally specified more than once.
Also, most of these don't have regression test cases, and I'm very
reluctant to change them without proper testing, and that would make
the patch much bigger. To me, this patch is already attempting to
change too much in one go, which is causing problems.
So I suggest a more incremental approach, starting by keeping the
original error message, but improving it where possible with the error
position. Then maybe move on to look at specific cases that can be
further improved with additional detail (keeping the same primary
error message, for consistency).
Here is an updated version, following that approach. It does the following:
1). Keeps the same primary error message ("conflicting or redundant
options") in all cases.
2). Uses errorConflictingDefElem() to throw it, to ensure consistency
and reduce the executable size.
3). Includes your enhancements to make the ParseState available in
more places, so that the error position indicator is shown to indicate
the cause of the error.
IMO, this makes for a much safer incremental change, that is more committable.
As it turns out, there are 110 cases of this error that now use
errorConflictingDefElem(), and of those, just 10 (in 3 functions)
don't have a ParseState readily available to them:
- ATExecSetIdentity()
- parse_output_parameters() x5
- parseCreateReplSlotOptions() x4
It might be possible to improve those (and possibly some of the others
too) by adding some appropriate DETAIL to the error, but as I said, I
suggest doing that in a separate follow-on patch, and only with
careful analysis and testing of each case.
As it stands, the improvements from (3) seem quite worthwhile. Also,
the patch saves a couple of hundred lines of code, and for me an
optimised executable is around 30 kB smaller, which is more than I
expected.
Thoughts?
Regards,
Dean
diff --git a/src/backend/catalog/aclchk.c b/src/backend/catalog/aclchk.c
new file mode 100644
index 5339241..89792b1
--- 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(ParseStat
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
new file mode 100644
index 8265b98..6b33951
--- 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
new file mode 100644
index 2b159b6..029fab4
--- a/src/backend/commands/dbcommands.c
+++ b/src/backend/commands/dbcommands.c
@@ -152,91 +152,61 @@ createdb(ParseState *pstate, const Creat
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, AlterD
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
new file mode 100644
index 84487b7..e900a17
--- 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"),
+ pstate ? parser_errposition(pstate, defel->location) : 0);
+}
diff --git a/src/backend/commands/extension.c b/src/backend/commands/extension.c
new file mode 100644
index b37009b..eaa76af
--- a/src/backend/commands/extension.c
+++ b/src/backend/commands/extension.c
@@ -1731,30 +1731,21 @@ CreateExtension(ParseState *pstate, Crea
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 *pstat
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
new file mode 100644
index bc36311..9b71beb
--- a/src/backend/commands/foreigncmds.c
+++ b/src/backend/commands/foreigncmds.c
@@ -515,7 +515,7 @@ lookup_fdw_validator_func(DefElem *valid
* 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 *
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 *
* Alter foreign-data wrapper
*/
ObjectAddress
-AlterForeignDataWrapper(AlterFdwStmt *stmt)
+AlterForeignDataWrapper(ParseState *pstate, AlterFdwStmt *stmt)
{
Relation rel;
HeapTuple tp;
@@ -717,7 +713,7 @@ AlterForeignDataWrapper(AlterFdwStmt *st
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
new file mode 100644
index 736d047..79d875a
--- a/src/backend/commands/functioncmds.c
+++ b/src/backend/commands/functioncmds.c
@@ -523,7 +523,7 @@ compute_common_attribute(ParseState *pst
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 *pst
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 *pst
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 *pst
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 *pst
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 *pst
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 *pst
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 *pst
/* 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 *
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 *p
* 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
new file mode 100644
index 95c253c..12f9f8b
--- a/src/backend/commands/publicationcmds.c
+++ b/src/backend/commands/publicationcmds.c
@@ -55,7 +55,8 @@ static void PublicationAddTables(Oid pub
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
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
* 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(AlterPublication
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(AlterPublicationS
* AlterPublicationTables.
*/
void
-AlterPublication(AlterPublicationStmt *stmt)
+AlterPublication(ParseState *pstate, AlterPublicationStmt *stmt)
{
Relation rel;
HeapTuple tup;
@@ -459,7 +458,7 @@ AlterPublication(AlterPublicationStmt *s
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
new file mode 100644
index d22d767..72bfdc0
--- a/src/backend/commands/sequence.c
+++ b/src/backend/commands/sequence.c
@@ -1261,90 +1261,63 @@ init_params(ParseState *pstate, List *op
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
new file mode 100644
index eb88d87..2983599
--- a/src/backend/commands/subscriptioncmds.c
+++ b/src/backend/commands/subscriptioncmds.c
@@ -96,7 +96,8 @@ static void ReportSlotConnectionError(Li
* 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;
@@ -133,9 +134,7 @@ parse_subscription_options(List *stmt_op
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);
@@ -144,9 +143,7 @@ parse_subscription_options(List *stmt_op
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);
@@ -155,9 +152,7 @@ parse_subscription_options(List *stmt_op
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);
@@ -166,9 +161,7 @@ parse_subscription_options(List *stmt_op
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);
@@ -181,9 +174,7 @@ parse_subscription_options(List *stmt_op
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);
@@ -192,9 +183,7 @@ parse_subscription_options(List *stmt_op
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);
@@ -208,9 +197,7 @@ parse_subscription_options(List *stmt_op
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);
@@ -219,9 +206,7 @@ parse_subscription_options(List *stmt_op
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);
@@ -230,9 +215,7 @@ parse_subscription_options(List *stmt_op
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);
@@ -362,7 +345,8 @@ publicationListToArray(List *publist)
* Create new subscription.
*/
ObjectAddress
-CreateSubscription(CreateSubscriptionStmt *stmt, bool isTopLevel)
+CreateSubscription(ParseState *pstate, CreateSubscriptionStmt *stmt,
+ bool isTopLevel)
{
Relation rel;
ObjectAddress myself;
@@ -386,7 +370,7 @@ CreateSubscription(CreateSubscriptionStm
SUBOPT_SLOT_NAME | SUBOPT_COPY_DATA |
SUBOPT_SYNCHRONOUS_COMMIT | SUBOPT_BINARY |
SUBOPT_STREAMING);
- 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
@@ -778,7 +762,8 @@ AlterSubscription_refresh(Subscription *
* Alter the existing subscription.
*/
ObjectAddress
-AlterSubscription(AlterSubscriptionStmt *stmt, bool isTopLevel)
+AlterSubscription(ParseState *pstate, AlterSubscriptionStmt *stmt,
+ bool isTopLevel)
{
Relation rel;
ObjectAddress myself;
@@ -831,7 +816,8 @@ AlterSubscription(AlterSubscriptionStmt
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))
{
@@ -876,7 +862,8 @@ AlterSubscription(AlterSubscriptionStmt
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)
@@ -910,7 +897,8 @@ AlterSubscription(AlterSubscriptionStmt
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);
@@ -948,7 +936,8 @@ AlterSubscription(AlterSubscriptionStmt
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] =
@@ -984,7 +973,8 @@ AlterSubscription(AlterSubscriptionStmt
(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);
PreventInTransactionBlock(isTopLevel, "ALTER SUBSCRIPTION ... REFRESH");
diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
new file mode 100644
index dcf14b9..406e36d
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -7587,9 +7587,7 @@ ATExecSetIdentity(Relation rel, const ch
if (strcmp(defel->defname, "generated") == 0)
{
if (generatedEl)
- ereport(ERROR,
- (errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("conflicting or redundant options")));
+ errorConflictingDefElem(defel, NULL);
generatedEl = defel;
}
else
diff --git a/src/backend/commands/typecmds.c b/src/backend/commands/typecmds.c
new file mode 100644
index 58ec65c..93eeff9
--- a/src/backend/commands/typecmds.c
+++ b/src/backend/commands/typecmds.c
@@ -330,10 +330,7 @@ DefineType(ParseState *pstate, List *nam
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
new file mode 100644
index 65bb733..aa69821
--- 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, CreateRol
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, CreateRol
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, CreateRol
* "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
new file mode 100644
index 3afcd6b..9edd1f8
--- a/src/backend/parser/parse_utilcmd.c
+++ b/src/backend/parser/parse_utilcmd.c
@@ -392,9 +392,7 @@ generateSerialExtraStmts(CreateStmtConte
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/replication/pgoutput/pgoutput.c b/src/backend/replication/pgoutput/pgoutput.c
new file mode 100644
index abd5217..9a79412
--- a/src/backend/replication/pgoutput/pgoutput.c
+++ b/src/backend/replication/pgoutput/pgoutput.c
@@ -184,9 +184,7 @@ parse_output_parameters(List *options, P
int64 parsed;
if (protocol_version_given)
- ereport(ERROR,
- (errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("conflicting or redundant options")));
+ errorConflictingDefElem(defel, NULL);
protocol_version_given = true;
if (!scanint8(strVal(defel->arg), true, &parsed))
@@ -205,9 +203,7 @@ parse_output_parameters(List *options, P
else if (strcmp(defel->defname, "publication_names") == 0)
{
if (publication_names_given)
- ereport(ERROR,
- (errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("conflicting or redundant options")));
+ errorConflictingDefElem(defel, NULL);
publication_names_given = true;
if (!SplitIdentifierString(strVal(defel->arg), ',',
@@ -219,9 +215,7 @@ parse_output_parameters(List *options, P
else if (strcmp(defel->defname, "binary") == 0)
{
if (binary_option_given)
- ereport(ERROR,
- (errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("conflicting or redundant options")));
+ errorConflictingDefElem(defel, NULL);
binary_option_given = true;
data->binary = defGetBoolean(defel);
@@ -229,9 +223,7 @@ parse_output_parameters(List *options, P
else if (strcmp(defel->defname, "messages") == 0)
{
if (messages_option_given)
- ereport(ERROR,
- (errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("conflicting or redundant options")));
+ errorConflictingDefElem(defel, NULL);
messages_option_given = true;
data->messages = defGetBoolean(defel);
@@ -239,9 +231,7 @@ parse_output_parameters(List *options, P
else if (strcmp(defel->defname, "streaming") == 0)
{
if (streaming_given)
- ereport(ERROR,
- (errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("conflicting or redundant options")));
+ errorConflictingDefElem(defel, NULL);
streaming_given = true;
data->streaming = defGetBoolean(defel);
diff --git a/src/backend/replication/walsender.c b/src/backend/replication/walsender.c
new file mode 100644
index 3ca2a11..7202f91
--- a/src/backend/replication/walsender.c
+++ b/src/backend/replication/walsender.c
@@ -875,9 +875,7 @@ parseCreateReplSlotOptions(CreateReplica
if (strcmp(defel->defname, "export_snapshot") == 0)
{
if (snapshot_action_given || cmd->kind != REPLICATION_KIND_LOGICAL)
- ereport(ERROR,
- (errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("conflicting or redundant options")));
+ errorConflictingDefElem(defel, NULL);
snapshot_action_given = true;
*snapshot_action = defGetBoolean(defel) ? CRS_EXPORT_SNAPSHOT :
@@ -886,9 +884,7 @@ parseCreateReplSlotOptions(CreateReplica
else if (strcmp(defel->defname, "use_snapshot") == 0)
{
if (snapshot_action_given || cmd->kind != REPLICATION_KIND_LOGICAL)
- ereport(ERROR,
- (errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("conflicting or redundant options")));
+ errorConflictingDefElem(defel, NULL);
snapshot_action_given = true;
*snapshot_action = CRS_USE_SNAPSHOT;
@@ -896,9 +892,7 @@ parseCreateReplSlotOptions(CreateReplica
else if (strcmp(defel->defname, "reserve_wal") == 0)
{
if (reserve_wal_given || cmd->kind != REPLICATION_KIND_PHYSICAL)
- ereport(ERROR,
- (errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("conflicting or redundant options")));
+ errorConflictingDefElem(defel, NULL);
reserve_wal_given = true;
*reserve_wal = true;
@@ -906,9 +900,8 @@ parseCreateReplSlotOptions(CreateReplica
else if (strcmp(defel->defname, "two_phase") == 0)
{
if (two_phase_given || cmd->kind != REPLICATION_KIND_LOGICAL)
- ereport(ERROR,
- (errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("conflicting or redundant options")));
+ errorConflictingDefElem(defel, NULL);
+
two_phase_given = true;
*two_phase = true;
}
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
new file mode 100644
index 7a2da9d..27fbf1f
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -708,7 +708,7 @@ standard_ProcessUtility(PlannedStmt *pst
break;
case T_DoStmt:
- ExecuteDoStmt((DoStmt *) parsetree, isAtomicContext);
+ ExecuteDoStmt(pstate, (DoStmt *) parsetree, isAtomicContext);
break;
case T_CreateTableSpaceStmt:
@@ -888,7 +888,7 @@ standard_ProcessUtility(PlannedStmt *pst
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
new file mode 100644
index 42bf1c7..f84d099
--- a/src/include/commands/defrem.h
+++ b/src/include/commands/defrem.h
@@ -56,7 +56,7 @@ extern ObjectAddress CreateCast(CreateCa
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 AlterForeignServerO
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
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
new file mode 100644
index 00e2e62..efea01f
--- 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
new file mode 100644
index 3b926f3..8bf25ee
--- 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
new file mode 100644
index 8806791..d5e2281
--- a/src/include/commands/typecmds.h
+++ b/src/include/commands/typecmds.h
@@ -25,7 +25,7 @@ extern ObjectAddress DefineType(ParseSta
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
new file mode 100644
index 028e0dd..0b7a3cd
--- a/src/include/commands/user.h
+++ b/src/include/commands/user.h
@@ -25,7 +25,7 @@ typedef void (*check_password_hook_type)
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
new file mode 100644
index c64f071..5f3685e
--- 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_nul
^
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
new file mode 100644
index 809d40a..426080a
--- 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 HAN
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 i
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
new file mode 100644
index 63d6ab7..b5b065a
--- a/src/test/regress/expected/publication.out
+++ b/src/test/regress/expected/publication.out
@@ -27,6 +27,8 @@ CREATE PUBLICATION testpub_xxx WITH (pub
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
