Here are two WIP patches to improve the DefElem list processing that is
used by many utility commands.
One factors out the duplicate checks, which are currently taking up a
lot of space with duplicate code. I haven't applied this everywhere
yet, but the patch shows how much boring code can be saved.
The other adds a location field to the DefElem node. This allows
showing an error pointer if there is a problem in one of the options,
which can be useful for complex commands such as COPY or CREATE USER.
At the moment, these patches are independent and don't work together,
but the second one would become much easier if the first one is accepted.
If these ideas are acceptable, I'll produce a complete patch series.
--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From 89af91f91944ba70c538d249f042eebf14fdd2cd Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pete...@gmx.net>
Date: Thu, 4 Aug 2016 11:29:27 -0400
Subject: [PATCH] Add location field to DefElem
---
contrib/file_fdw/file_fdw.c | 5 ++-
src/backend/commands/copy.c | 93 +++++++++++++++++++++++++----------------
src/backend/commands/sequence.c | 36 +++++++++-------
src/backend/commands/user.c | 41 +++++++++++-------
src/backend/nodes/copyfuncs.c | 1 +
src/backend/nodes/equalfuncs.c | 2 +
src/backend/nodes/makefuncs.c | 1 +
src/backend/nodes/outfuncs.c | 1 +
src/backend/nodes/readfuncs.c | 1 +
src/backend/parser/gram.y | 44 +++++++++++++++++++
src/backend/tcop/utility.c | 36 +++++++++-------
src/include/commands/copy.h | 7 ++--
src/include/commands/sequence.h | 5 ++-
src/include/commands/user.h | 3 +-
src/include/nodes/parsenodes.h | 1 +
15 files changed, 190 insertions(+), 87 deletions(-)
diff --git a/contrib/file_fdw/file_fdw.c b/contrib/file_fdw/file_fdw.c
index c049131..85d9913 100644
--- a/contrib/file_fdw/file_fdw.c
+++ b/contrib/file_fdw/file_fdw.c
@@ -293,7 +293,7 @@ file_fdw_validator(PG_FUNCTION_ARGS)
/*
* Now apply the core COPY code's validation logic for more checks.
*/
- ProcessCopyOptions(NULL, true, other_options);
+ ProcessCopyOptions(NULL, NULL, true, other_options);
/*
* Filename option is required for file_fdw foreign tables.
@@ -632,7 +632,8 @@ fileBeginForeignScan(ForeignScanState *node, int eflags)
* Create CopyState from FDW options. We always acquire all columns, so
* as to match the expected ScanTupleSlot signature.
*/
- cstate = BeginCopyFrom(node->ss.ss_currentRelation,
+ cstate = BeginCopyFrom(NULL,
+ node->ss.ss_currentRelation,
filename,
false,
NIL,
diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c
index f45b330..1f85cda 100644
--- a/src/backend/commands/copy.c
+++ b/src/backend/commands/copy.c
@@ -279,12 +279,12 @@ static const char BinarySignature[11] = "PGCOPY\n\377\r\n\0";
/* non-export function prototypes */
-static CopyState BeginCopy(bool is_from, Relation rel, Node *raw_query,
- const char *queryString, const Oid queryRelId, List *attnamelist,
+static CopyState BeginCopy(ParseState *pstate, bool is_from, Relation rel, Node *raw_query,
+ const Oid queryRelId, List *attnamelist,
List *options);
static void EndCopy(CopyState cstate);
static void ClosePipeToProgram(CopyState cstate);
-static CopyState BeginCopyTo(Relation rel, Node *query, const char *queryString,
+static CopyState BeginCopyTo(ParseState *pstate, Relation rel, Node *query,
const Oid queryRelId, const char *filename, bool is_program,
List *attnamelist, List *options);
static void EndCopyTo(CopyState cstate);
@@ -787,7 +787,7 @@ CopyLoadRawBuf(CopyState cstate)
* the table or the specifically requested columns.
*/
Oid
-DoCopy(const CopyStmt *stmt, const char *queryString, uint64 *processed)
+DoCopy(ParseState *pstate, const CopyStmt *stmt, uint64 *processed)
{
CopyState cstate;
bool is_from = stmt->is_from;
@@ -936,7 +936,7 @@ DoCopy(const CopyStmt *stmt, const char *queryString, uint64 *processed)
PreventCommandIfReadOnly("COPY FROM");
PreventCommandIfParallelMode("COPY FROM");
- cstate = BeginCopyFrom(rel, stmt->filename, stmt->is_program,
+ cstate = BeginCopyFrom(pstate, rel, stmt->filename, stmt->is_program,
stmt->attlist, stmt->options);
cstate->range_table = range_table;
*processed = CopyFrom(cstate); /* copy from file to database */
@@ -944,7 +944,7 @@ DoCopy(const CopyStmt *stmt, const char *queryString, uint64 *processed)
}
else
{
- cstate = BeginCopyTo(rel, query, queryString, relid,
+ cstate = BeginCopyTo(pstate, rel, query, relid,
stmt->filename, stmt->is_program,
stmt->attlist, stmt->options);
*processed = DoCopyTo(cstate); /* copy from database to file */
@@ -980,7 +980,8 @@ DoCopy(const CopyStmt *stmt, const char *queryString, uint64 *processed)
* self-consistency of the options list.
*/
void
-ProcessCopyOptions(CopyState cstate,
+ProcessCopyOptions(ParseState *pstate,
+ CopyState cstate,
bool is_from,
List *options)
{
@@ -1005,7 +1006,8 @@ ProcessCopyOptions(CopyState cstate,
if (format_specified)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("conflicting or redundant options")));
+ errmsg("conflicting or redundant options"),
+ parser_errposition(pstate, defel->location)));
format_specified = true;
if (strcmp(fmt, "text") == 0)
/* default format */ ;
@@ -1016,14 +1018,16 @@ ProcessCopyOptions(CopyState cstate,
else
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("COPY format \"%s\" not recognized", fmt)));
+ errmsg("COPY format \"%s\" not recognized", fmt),
+ parser_errposition(pstate, defel->location)));
}
else if (strcmp(defel->defname, "oids") == 0)
{
if (cstate->oids)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("conflicting or redundant options")));
+ errmsg("conflicting or redundant options"),
+ parser_errposition(pstate, defel->location)));
cstate->oids = defGetBoolean(defel);
}
else if (strcmp(defel->defname, "freeze") == 0)
@@ -1031,7 +1035,8 @@ ProcessCopyOptions(CopyState cstate,
if (cstate->freeze)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("conflicting or redundant options")));
+ errmsg("conflicting or redundant options"),
+ parser_errposition(pstate, defel->location)));
cstate->freeze = defGetBoolean(defel);
}
else if (strcmp(defel->defname, "delimiter") == 0)
@@ -1039,7 +1044,8 @@ ProcessCopyOptions(CopyState cstate,
if (cstate->delim)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("conflicting or redundant options")));
+ errmsg("conflicting or redundant options"),
+ parser_errposition(pstate, defel->location)));
cstate->delim = defGetString(defel);
}
else if (strcmp(defel->defname, "null") == 0)
@@ -1047,7 +1053,8 @@ ProcessCopyOptions(CopyState cstate,
if (cstate->null_print)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("conflicting or redundant options")));
+ errmsg("conflicting or redundant options"),
+ parser_errposition(pstate, defel->location)));
cstate->null_print = defGetString(defel);
}
else if (strcmp(defel->defname, "header") == 0)
@@ -1055,7 +1062,8 @@ ProcessCopyOptions(CopyState cstate,
if (cstate->header_line)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("conflicting or redundant options")));
+ errmsg("conflicting or redundant options"),
+ parser_errposition(pstate, defel->location)));
cstate->header_line = defGetBoolean(defel);
}
else if (strcmp(defel->defname, "quote") == 0)
@@ -1063,7 +1071,8 @@ ProcessCopyOptions(CopyState cstate,
if (cstate->quote)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("conflicting or redundant options")));
+ errmsg("conflicting or redundant options"),
+ parser_errposition(pstate, defel->location)));
cstate->quote = defGetString(defel);
}
else if (strcmp(defel->defname, "escape") == 0)
@@ -1071,7 +1080,8 @@ ProcessCopyOptions(CopyState cstate,
if (cstate->escape)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("conflicting or redundant options")));
+ errmsg("conflicting or redundant options"),
+ parser_errposition(pstate, defel->location)));
cstate->escape = defGetString(defel);
}
else if (strcmp(defel->defname, "force_quote") == 0)
@@ -1079,7 +1089,8 @@ ProcessCopyOptions(CopyState cstate,
if (cstate->force_quote || cstate->force_quote_all)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("conflicting or redundant options")));
+ errmsg("conflicting or redundant options"),
+ parser_errposition(pstate, defel->location)));
if (defel->arg && IsA(defel->arg, A_Star))
cstate->force_quote_all = true;
else if (defel->arg && IsA(defel->arg, List))
@@ -1088,21 +1099,24 @@ ProcessCopyOptions(CopyState cstate,
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("argument to option \"%s\" must be a list of column names",
- defel->defname)));
+ defel->defname),
+ parser_errposition(pstate, defel->location)));
}
else if (strcmp(defel->defname, "force_not_null") == 0)
{
if (cstate->force_notnull)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("conflicting or redundant options")));
+ errmsg("conflicting or redundant options"),
+ parser_errposition(pstate, defel->location)));
if (defel->arg && IsA(defel->arg, List))
cstate->force_notnull = (List *) defel->arg;
else
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("argument to option \"%s\" must be a list of column names",
- defel->defname)));
+ defel->defname),
+ parser_errposition(pstate, defel->location)));
}
else if (strcmp(defel->defname, "force_null") == 0)
{
@@ -1116,7 +1130,8 @@ ProcessCopyOptions(CopyState cstate,
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("argument to option \"%s\" must be a list of column names",
- defel->defname)));
+ defel->defname),
+ parser_errposition(pstate, defel->location)));
}
else if (strcmp(defel->defname, "convert_selectively") == 0)
{
@@ -1128,7 +1143,8 @@ ProcessCopyOptions(CopyState cstate,
if (cstate->convert_selectively)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("conflicting or redundant options")));
+ errmsg("conflicting or redundant options"),
+ parser_errposition(pstate, defel->location)));
cstate->convert_selectively = true;
if (defel->arg == NULL || IsA(defel->arg, List))
cstate->convert_select = (List *) defel->arg;
@@ -1136,26 +1152,30 @@ ProcessCopyOptions(CopyState cstate,
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("argument to option \"%s\" must be a list of column names",
- defel->defname)));
+ defel->defname),
+ parser_errposition(pstate, defel->location)));
}
else if (strcmp(defel->defname, "encoding") == 0)
{
if (cstate->file_encoding >= 0)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("conflicting or redundant options")));
+ errmsg("conflicting or redundant options"),
+ parser_errposition(pstate, defel->location)));
cstate->file_encoding = pg_char_to_encoding(defGetString(defel));
if (cstate->file_encoding < 0)
ereport(ERROR,
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
errmsg("argument to option \"%s\" must be a valid encoding name",
- defel->defname)));
+ defel->defname),
+ parser_errposition(pstate, defel->location)));
}
else
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
errmsg("option \"%s\" not recognized",
- defel->defname)));
+ defel->defname),
+ parser_errposition(pstate, defel->location)));
}
/*
@@ -1318,10 +1338,10 @@ ProcessCopyOptions(CopyState cstate,
* NULL values as <null_print>.
*/
static CopyState
-BeginCopy(bool is_from,
+BeginCopy(ParseState *pstate,
+ bool is_from,
Relation rel,
Node *raw_query,
- const char *queryString,
const Oid queryRelId,
List *attnamelist,
List *options)
@@ -1347,7 +1367,7 @@ BeginCopy(bool is_from,
oldcontext = MemoryContextSwitchTo(cstate->copycontext);
/* Extract options from the statement node tree */
- ProcessCopyOptions(cstate, is_from, options);
+ ProcessCopyOptions(pstate, cstate, is_from, options);
/* Process the source/target relation or query */
if (rel)
@@ -1392,7 +1412,7 @@ BeginCopy(bool is_from,
* DECLARE CURSOR and PREPARE.) XXX FIXME someday.
*/
rewritten = pg_analyze_and_rewrite((Node *) copyObject(raw_query),
- queryString, NULL, 0);
+ pstate->p_sourcetext, NULL, 0);
/* check that we got back something we can work with */
if (rewritten == NIL)
@@ -1492,7 +1512,7 @@ BeginCopy(bool is_from,
((DR_copy *) dest)->cstate = cstate;
/* Create a QueryDesc requesting no output */
- cstate->queryDesc = CreateQueryDesc(plan, queryString,
+ cstate->queryDesc = CreateQueryDesc(plan, pstate->p_sourcetext,
GetActiveSnapshot(),
InvalidSnapshot,
dest, NULL, 0);
@@ -1680,9 +1700,9 @@ EndCopy(CopyState cstate)
* Setup CopyState to read tuples from a table or a query for COPY TO.
*/
static CopyState
-BeginCopyTo(Relation rel,
+BeginCopyTo(ParseState *pstate,
+ Relation rel,
Node *query,
- const char *queryString,
const Oid queryRelId,
const char *filename,
bool is_program,
@@ -1725,7 +1745,7 @@ BeginCopyTo(Relation rel,
RelationGetRelationName(rel))));
}
- cstate = BeginCopy(false, rel, query, queryString, queryRelId, attnamelist,
+ cstate = BeginCopy(pstate, false, rel, query, queryRelId, attnamelist,
options);
oldcontext = MemoryContextSwitchTo(cstate->copycontext);
@@ -2649,7 +2669,8 @@ CopyFromInsertBatch(CopyState cstate, EState *estate, CommandId mycid,
* Returns a CopyState, to be passed to NextCopyFrom and related functions.
*/
CopyState
-BeginCopyFrom(Relation rel,
+BeginCopyFrom(ParseState *pstate,
+ Relation rel,
const char *filename,
bool is_program,
List *attnamelist,
@@ -2670,7 +2691,7 @@ BeginCopyFrom(Relation rel,
MemoryContext oldcontext;
bool volatile_defexprs;
- cstate = BeginCopy(true, rel, NULL, NULL, InvalidOid, attnamelist, options);
+ cstate = BeginCopy(pstate, true, rel, NULL, InvalidOid, attnamelist, options);
oldcontext = MemoryContextSwitchTo(cstate->copycontext);
/* Initialize state variables */
diff --git a/src/backend/commands/sequence.c b/src/backend/commands/sequence.c
index c98f981..fc3a8ee 100644
--- a/src/backend/commands/sequence.c
+++ b/src/backend/commands/sequence.c
@@ -94,7 +94,7 @@ static void create_seq_hashtable(void);
static void init_sequence(Oid relid, SeqTable *p_elm, Relation *p_rel);
static Form_pg_sequence read_seq_tuple(SeqTable elm, Relation rel,
Buffer *buf, HeapTuple seqtuple);
-static void init_params(List *options, bool isInit,
+static void init_params(ParseState *pstate, List *options, bool isInit,
Form_pg_sequence new, List **owned_by);
static void do_setval(Oid relid, int64 next, bool iscalled);
static void process_owned_by(Relation seqrel, List *owned_by);
@@ -105,7 +105,7 @@ static void process_owned_by(Relation seqrel, List *owned_by);
* Creates a new sequence relation
*/
ObjectAddress
-DefineSequence(CreateSeqStmt *seq)
+DefineSequence(ParseState *pstate, CreateSeqStmt *seq)
{
FormData_pg_sequence new;
List *owned_by;
@@ -145,7 +145,7 @@ DefineSequence(CreateSeqStmt *seq)
}
/* Check and set all option values */
- init_params(seq->options, true, &new, &owned_by);
+ init_params(pstate, seq->options, true, &new, &owned_by);
/*
* Create relation (and fill value[] and null[] for the tuple)
@@ -404,7 +404,7 @@ fill_seq_with_data(Relation rel, HeapTuple tuple)
* Modify the definition of a sequence relation
*/
ObjectAddress
-AlterSequence(AlterSeqStmt *stmt)
+AlterSequence(ParseState *pstate, AlterSeqStmt *stmt)
{
Oid relid;
SeqTable elm;
@@ -440,7 +440,7 @@ AlterSequence(AlterSeqStmt *stmt)
memcpy(&new, seq, sizeof(FormData_pg_sequence));
/* Check and set new values */
- init_params(stmt->options, false, &new, &owned_by);
+ init_params(pstate, stmt->options, false, &new, &owned_by);
/* Clear local cache so that we don't think we have cached numbers */
/* Note that we do not change the currval() state */
@@ -1163,7 +1163,7 @@ read_seq_tuple(SeqTable elm, Relation rel, Buffer *buf, HeapTuple seqtuple)
* otherwise, do not change existing options that aren't explicitly overridden.
*/
static void
-init_params(List *options, bool isInit,
+init_params(ParseState *pstate, List *options, bool isInit,
Form_pg_sequence new, List **owned_by)
{
DefElem *start_value = NULL;
@@ -1186,7 +1186,8 @@ init_params(List *options, bool isInit,
if (increment_by)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("conflicting or redundant options")));
+ errmsg("conflicting or redundant options"),
+ parser_errposition(pstate, defel->location)));
increment_by = defel;
}
else if (strcmp(defel->defname, "start") == 0)
@@ -1194,7 +1195,8 @@ init_params(List *options, bool isInit,
if (start_value)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("conflicting or redundant options")));
+ errmsg("conflicting or redundant options"),
+ parser_errposition(pstate, defel->location)));
start_value = defel;
}
else if (strcmp(defel->defname, "restart") == 0)
@@ -1202,7 +1204,8 @@ init_params(List *options, bool isInit,
if (restart_value)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("conflicting or redundant options")));
+ errmsg("conflicting or redundant options"),
+ parser_errposition(pstate, defel->location)));
restart_value = defel;
}
else if (strcmp(defel->defname, "maxvalue") == 0)
@@ -1210,7 +1213,8 @@ init_params(List *options, bool isInit,
if (max_value)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("conflicting or redundant options")));
+ errmsg("conflicting or redundant options"),
+ parser_errposition(pstate, defel->location)));
max_value = defel;
}
else if (strcmp(defel->defname, "minvalue") == 0)
@@ -1218,7 +1222,8 @@ init_params(List *options, bool isInit,
if (min_value)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("conflicting or redundant options")));
+ errmsg("conflicting or redundant options"),
+ parser_errposition(pstate, defel->location)));
min_value = defel;
}
else if (strcmp(defel->defname, "cache") == 0)
@@ -1226,7 +1231,8 @@ init_params(List *options, bool isInit,
if (cache_value)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("conflicting or redundant options")));
+ errmsg("conflicting or redundant options"),
+ parser_errposition(pstate, defel->location)));
cache_value = defel;
}
else if (strcmp(defel->defname, "cycle") == 0)
@@ -1234,7 +1240,8 @@ init_params(List *options, bool isInit,
if (is_cycled)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("conflicting or redundant options")));
+ errmsg("conflicting or redundant options"),
+ parser_errposition(pstate, defel->location)));
is_cycled = defel;
}
else if (strcmp(defel->defname, "owned_by") == 0)
@@ -1242,7 +1249,8 @@ init_params(List *options, bool isInit,
if (*owned_by)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("conflicting or redundant options")));
+ errmsg("conflicting or redundant options"),
+ parser_errposition(pstate, defel->location)));
*owned_by = defGetQualifiedName(defel);
}
else
diff --git a/src/backend/commands/user.c b/src/backend/commands/user.c
index b6ea950..c41bfca 100644
--- a/src/backend/commands/user.c
+++ b/src/backend/commands/user.c
@@ -69,7 +69,7 @@ have_createrole_privilege(void)
* CREATE ROLE
*/
Oid
-CreateRole(CreateRoleStmt *stmt)
+CreateRole(ParseState *pstate, CreateRoleStmt *stmt)
{
Relation pg_authid_rel;
TupleDesc pg_authid_dsc;
@@ -136,7 +136,8 @@ CreateRole(CreateRoleStmt *stmt)
if (dpassword)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("conflicting or redundant options")));
+ errmsg("conflicting or redundant options"),
+ parser_errposition(pstate, defel->location)));
dpassword = defel;
if (strcmp(defel->defname, "encryptedPassword") == 0)
encrypt_password = true;
@@ -153,7 +154,8 @@ CreateRole(CreateRoleStmt *stmt)
if (dissuper)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("conflicting or redundant options")));
+ errmsg("conflicting or redundant options"),
+ parser_errposition(pstate, defel->location)));
dissuper = defel;
}
else if (strcmp(defel->defname, "inherit") == 0)
@@ -161,7 +163,8 @@ CreateRole(CreateRoleStmt *stmt)
if (dinherit)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("conflicting or redundant options")));
+ errmsg("conflicting or redundant options"),
+ parser_errposition(pstate, defel->location)));
dinherit = defel;
}
else if (strcmp(defel->defname, "createrole") == 0)
@@ -169,7 +172,8 @@ CreateRole(CreateRoleStmt *stmt)
if (dcreaterole)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("conflicting or redundant options")));
+ errmsg("conflicting or redundant options"),
+ parser_errposition(pstate, defel->location)));
dcreaterole = defel;
}
else if (strcmp(defel->defname, "createdb") == 0)
@@ -177,7 +181,8 @@ CreateRole(CreateRoleStmt *stmt)
if (dcreatedb)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("conflicting or redundant options")));
+ errmsg("conflicting or redundant options"),
+ parser_errposition(pstate, defel->location)));
dcreatedb = defel;
}
else if (strcmp(defel->defname, "canlogin") == 0)
@@ -185,7 +190,8 @@ CreateRole(CreateRoleStmt *stmt)
if (dcanlogin)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("conflicting or redundant options")));
+ errmsg("conflicting or redundant options"),
+ parser_errposition(pstate, defel->location)));
dcanlogin = defel;
}
else if (strcmp(defel->defname, "isreplication") == 0)
@@ -193,7 +199,8 @@ CreateRole(CreateRoleStmt *stmt)
if (disreplication)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("conflicting or redundant options")));
+ errmsg("conflicting or redundant options"),
+ parser_errposition(pstate, defel->location)));
disreplication = defel;
}
else if (strcmp(defel->defname, "connectionlimit") == 0)
@@ -201,7 +208,8 @@ CreateRole(CreateRoleStmt *stmt)
if (dconnlimit)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("conflicting or redundant options")));
+ errmsg("conflicting or redundant options"),
+ parser_errposition(pstate, defel->location)));
dconnlimit = defel;
}
else if (strcmp(defel->defname, "addroleto") == 0)
@@ -209,7 +217,8 @@ CreateRole(CreateRoleStmt *stmt)
if (daddroleto)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("conflicting or redundant options")));
+ errmsg("conflicting or redundant options"),
+ parser_errposition(pstate, defel->location)));
daddroleto = defel;
}
else if (strcmp(defel->defname, "rolemembers") == 0)
@@ -217,7 +226,8 @@ CreateRole(CreateRoleStmt *stmt)
if (drolemembers)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("conflicting or redundant options")));
+ errmsg("conflicting or redundant options"),
+ parser_errposition(pstate, defel->location)));
drolemembers = defel;
}
else if (strcmp(defel->defname, "adminmembers") == 0)
@@ -225,7 +235,8 @@ CreateRole(CreateRoleStmt *stmt)
if (dadminmembers)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("conflicting or redundant options")));
+ errmsg("conflicting or redundant options"),
+ parser_errposition(pstate, defel->location)));
dadminmembers = defel;
}
else if (strcmp(defel->defname, "validUntil") == 0)
@@ -233,7 +244,8 @@ CreateRole(CreateRoleStmt *stmt)
if (dvalidUntil)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("conflicting or redundant options")));
+ errmsg("conflicting or redundant options"),
+ parser_errposition(pstate, defel->location)));
dvalidUntil = defel;
}
else if (strcmp(defel->defname, "bypassrls") == 0)
@@ -241,7 +253,8 @@ CreateRole(CreateRoleStmt *stmt)
if (dbypassRLS)
ereport(ERROR,
(errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("conflicting or redundant options")));
+ errmsg("conflicting or redundant options"),
+ parser_errposition(pstate, defel->location)));
dbypassRLS = defel;
}
else
diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index 3244c76..aa792ec 100644
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -2659,6 +2659,7 @@ _copyDefElem(const DefElem *from)
COPY_STRING_FIELD(defname);
COPY_NODE_FIELD(arg);
COPY_SCALAR_FIELD(defaction);
+ COPY_LOCATION_FIELD(location);
return newnode;
}
diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c
index 1eb6799..cd81769 100644
--- a/src/backend/nodes/equalfuncs.c
+++ b/src/backend/nodes/equalfuncs.c
@@ -2413,6 +2413,7 @@ _equalDefElem(const DefElem *a, const DefElem *b)
COMPARE_STRING_FIELD(defname);
COMPARE_NODE_FIELD(arg);
COMPARE_SCALAR_FIELD(defaction);
+ COMPARE_LOCATION_FIELD(location);
return true;
}
@@ -2423,6 +2424,7 @@ _equalLockingClause(const LockingClause *a, const LockingClause *b)
COMPARE_NODE_FIELD(lockedRels);
COMPARE_SCALAR_FIELD(strength);
COMPARE_SCALAR_FIELD(waitPolicy);
+ COMPARE_LOCATION_FIELD(location);
return true;
}
diff --git a/src/backend/nodes/makefuncs.c b/src/backend/nodes/makefuncs.c
index d72a85e..1fd4f8b 100644
--- a/src/backend/nodes/makefuncs.c
+++ b/src/backend/nodes/makefuncs.c
@@ -548,6 +548,7 @@ makeDefElem(char *name, Node *arg)
res->defname = name;
res->arg = arg;
res->defaction = DEFELEM_UNSPEC;
+ res->location = -1;
return res;
}
diff --git a/src/backend/nodes/outfuncs.c b/src/backend/nodes/outfuncs.c
index acaf4ea..64d882d 100644
--- a/src/backend/nodes/outfuncs.c
+++ b/src/backend/nodes/outfuncs.c
@@ -2529,6 +2529,7 @@ _outDefElem(StringInfo str, const DefElem *node)
WRITE_STRING_FIELD(defname);
WRITE_NODE_FIELD(arg);
WRITE_ENUM_FIELD(defaction, DefElemAction);
+ WRITE_LOCATION_FIELD(location);
}
static void
diff --git a/src/backend/nodes/readfuncs.c b/src/backend/nodes/readfuncs.c
index 94954dc..af55ebb 100644
--- a/src/backend/nodes/readfuncs.c
+++ b/src/backend/nodes/readfuncs.c
@@ -1372,6 +1372,7 @@ _readDefElem(void)
READ_STRING_FIELD(defname);
READ_NODE_FIELD(arg);
READ_ENUM_FIELD(defaction, DefElemAction);
+ READ_LOCATION_FIELD(location);
READ_DONE();
}
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 0cae446..82d1182 100644
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -927,37 +927,45 @@ AlterOptRoleElem:
{
$$ = makeDefElem("password",
(Node *)makeString($2));
+ $$->location = @1;
}
| PASSWORD NULL_P
{
$$ = makeDefElem("password", NULL);
+ $$->location = @1;
}
| ENCRYPTED PASSWORD Sconst
{
$$ = makeDefElem("encryptedPassword",
(Node *)makeString($3));
+ $$->location = @1;
}
| UNENCRYPTED PASSWORD Sconst
{
$$ = makeDefElem("unencryptedPassword",
(Node *)makeString($3));
+ $$->location = @1;
}
| INHERIT
{
$$ = makeDefElem("inherit", (Node *)makeInteger(TRUE));
+ $$->location = @1;
}
| CONNECTION LIMIT SignedIconst
{
$$ = makeDefElem("connectionlimit", (Node *)makeInteger($3));
+ $$->location = @1;
}
| VALID UNTIL Sconst
{
$$ = makeDefElem("validUntil", (Node *)makeString($3));
+ $$->location = @1;
}
/* Supported but not documented for roles, for use by ALTER GROUP. */
| USER role_list
{
$$ = makeDefElem("rolemembers", (Node *)$2);
+ $$->location = @1;
}
| IDENT
{
@@ -1003,6 +1011,7 @@ AlterOptRoleElem:
(errcode(ERRCODE_SYNTAX_ERROR),
errmsg("unrecognized role option \"%s\"", $1),
parser_errposition(@1)));
+ $$->location = @1;
}
;
@@ -1012,22 +1021,27 @@ CreateOptRoleElem:
| SYSID Iconst
{
$$ = makeDefElem("sysid", (Node *)makeInteger($2));
+ $$->location = @1;
}
| ADMIN role_list
{
$$ = makeDefElem("adminmembers", (Node *)$2);
+ $$->location = @1;
}
| ROLE role_list
{
$$ = makeDefElem("rolemembers", (Node *)$2);
+ $$->location = @1;
}
| IN_P ROLE role_list
{
$$ = makeDefElem("addroleto", (Node *)$3);
+ $$->location = @1;
}
| IN_P GROUP_P role_list
{
$$ = makeDefElem("addroleto", (Node *)$3);
+ $$->location = @1;
}
;
@@ -2670,58 +2684,72 @@ copy_opt_item:
BINARY
{
$$ = makeDefElem("format", (Node *)makeString("binary"));
+ $$->location = @1;
}
| OIDS
{
$$ = makeDefElem("oids", (Node *)makeInteger(TRUE));
+ $$->location = @1;
}
| FREEZE
{
$$ = makeDefElem("freeze", (Node *)makeInteger(TRUE));
+ $$->location = @1;
}
| DELIMITER opt_as Sconst
{
$$ = makeDefElem("delimiter", (Node *)makeString($3));
+ $$->location = @1;
}
| NULL_P opt_as Sconst
{
$$ = makeDefElem("null", (Node *)makeString($3));
+ $$->location = @1;
}
| CSV
{
$$ = makeDefElem("format", (Node *)makeString("csv"));
+ $$->location = @1;
}
| HEADER_P
{
$$ = makeDefElem("header", (Node *)makeInteger(TRUE));
+ $$->location = @1;
}
| QUOTE opt_as Sconst
{
$$ = makeDefElem("quote", (Node *)makeString($3));
+ $$->location = @1;
}
| ESCAPE opt_as Sconst
{
$$ = makeDefElem("escape", (Node *)makeString($3));
+ $$->location = @1;
}
| FORCE QUOTE columnList
{
$$ = makeDefElem("force_quote", (Node *)$3);
+ $$->location = @1;
}
| FORCE QUOTE '*'
{
$$ = makeDefElem("force_quote", (Node *)makeNode(A_Star));
+ $$->location = @1;
}
| FORCE NOT NULL_P columnList
{
$$ = makeDefElem("force_not_null", (Node *)$4);
+ $$->location = @1;
}
| FORCE NULL_P columnList
{
$$ = makeDefElem("force_null", (Node *)$3);
+ $$->location = @1;
}
| ENCODING Sconst
{
$$ = makeDefElem("encoding", (Node *)makeString($2));
+ $$->location = @1;
}
;
@@ -2731,6 +2759,7 @@ opt_binary:
BINARY
{
$$ = makeDefElem("format", (Node *)makeString("binary"));
+ $$->location = @1;
}
| /*EMPTY*/ { $$ = NULL; }
;
@@ -2739,6 +2768,7 @@ opt_oids:
WITH OIDS
{
$$ = makeDefElem("oids", (Node *)makeInteger(TRUE));
+ $$->location = @1;
}
| /*EMPTY*/ { $$ = NULL; }
;
@@ -2747,6 +2777,7 @@ copy_delimiter:
opt_using DELIMITERS Sconst
{
$$ = makeDefElem("delimiter", (Node *)makeString($3));
+ $$->location = @2;
}
| /*EMPTY*/ { $$ = NULL; }
;
@@ -2772,6 +2803,7 @@ copy_generic_opt_elem:
ColLabel copy_generic_opt_arg
{
$$ = makeDefElem($1, $2);
+ $$->location = @1;
}
;
@@ -3637,50 +3669,62 @@ SeqOptList: SeqOptElem { $$ = list_make1($1); }
SeqOptElem: CACHE NumericOnly
{
$$ = makeDefElem("cache", (Node *)$2);
+ $$->location = @1;
}
| CYCLE
{
$$ = makeDefElem("cycle", (Node *)makeInteger(TRUE));
+ $$->location = @1;
}
| NO CYCLE
{
$$ = makeDefElem("cycle", (Node *)makeInteger(FALSE));
+ $$->location = @1;
}
| INCREMENT opt_by NumericOnly
{
$$ = makeDefElem("increment", (Node *)$3);
+ $$->location = @1;
}
| MAXVALUE NumericOnly
{
$$ = makeDefElem("maxvalue", (Node *)$2);
+ $$->location = @1;
}
| MINVALUE NumericOnly
{
$$ = makeDefElem("minvalue", (Node *)$2);
+ $$->location = @1;
}
| NO MAXVALUE
{
$$ = makeDefElem("maxvalue", NULL);
+ $$->location = @1;
}
| NO MINVALUE
{
$$ = makeDefElem("minvalue", NULL);
+ $$->location = @1;
}
| OWNED BY any_name
{
$$ = makeDefElem("owned_by", (Node *)$3);
+ $$->location = @1;
}
| START opt_with NumericOnly
{
$$ = makeDefElem("start", (Node *)$3);
+ $$->location = @1;
}
| RESTART
{
$$ = makeDefElem("restart", NULL);
+ $$->location = @1;
}
| RESTART opt_with NumericOnly
{
$$ = makeDefElem("restart", (Node *)$3);
+ $$->location = @1;
}
;
diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index ac50c2a..2c9b484 100644
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -71,7 +71,8 @@
ProcessUtility_hook_type ProcessUtility_hook = NULL;
/* local function declarations */
-static void ProcessUtilitySlow(Node *parsetree,
+static void ProcessUtilitySlow(ParseState *pstate,
+ Node *parsetree,
const char *queryString,
ProcessUtilityContext context,
ParamListInfo params,
@@ -358,12 +359,16 @@ standard_ProcessUtility(Node *parsetree,
char *completionTag)
{
bool isTopLevel = (context == PROCESS_UTILITY_TOPLEVEL);
+ ParseState *pstate;
check_xact_readonly(parsetree);
if (completionTag)
completionTag[0] = '\0';
+ pstate = make_parsestate(NULL);
+ pstate->p_sourcetext = queryString;
+
switch (nodeTag(parsetree))
{
/*
@@ -540,7 +545,7 @@ standard_ProcessUtility(Node *parsetree,
{
uint64 processed;
- DoCopy((CopyStmt *) parsetree, queryString, &processed);
+ DoCopy(pstate, (CopyStmt *) parsetree, &processed);
if (completionTag)
snprintf(completionTag, COMPLETION_TAG_BUFSIZE,
"COPY " UINT64_FORMAT, processed);
@@ -698,7 +703,7 @@ standard_ProcessUtility(Node *parsetree,
*/
case T_CreateRoleStmt:
/* no event triggers for global objects */
- CreateRole((CreateRoleStmt *) parsetree);
+ CreateRole(pstate, (CreateRoleStmt *) parsetree);
break;
case T_AlterRoleStmt:
@@ -803,7 +808,7 @@ standard_ProcessUtility(Node *parsetree,
GrantStmt *stmt = (GrantStmt *) parsetree;
if (EventTriggerSupportsGrantObjectType(stmt->objtype))
- ProcessUtilitySlow(parsetree, queryString,
+ ProcessUtilitySlow(pstate, parsetree, queryString,
context, params,
dest, completionTag);
else
@@ -816,7 +821,7 @@ standard_ProcessUtility(Node *parsetree,
DropStmt *stmt = (DropStmt *) parsetree;
if (EventTriggerSupportsObjectType(stmt->removeType))
- ProcessUtilitySlow(parsetree, queryString,
+ ProcessUtilitySlow(pstate, parsetree, queryString,
context, params,
dest, completionTag);
else
@@ -829,7 +834,7 @@ standard_ProcessUtility(Node *parsetree,
RenameStmt *stmt = (RenameStmt *) parsetree;
if (EventTriggerSupportsObjectType(stmt->renameType))
- ProcessUtilitySlow(parsetree, queryString,
+ ProcessUtilitySlow(pstate, parsetree, queryString,
context, params,
dest, completionTag);
else
@@ -842,7 +847,7 @@ standard_ProcessUtility(Node *parsetree,
AlterObjectDependsStmt *stmt = (AlterObjectDependsStmt *) parsetree;
if (EventTriggerSupportsObjectType(stmt->objectType))
- ProcessUtilitySlow(parsetree, queryString,
+ ProcessUtilitySlow(pstate, parsetree, queryString,
context, params,
dest, completionTag);
else
@@ -855,7 +860,7 @@ standard_ProcessUtility(Node *parsetree,
AlterObjectSchemaStmt *stmt = (AlterObjectSchemaStmt *) parsetree;
if (EventTriggerSupportsObjectType(stmt->objectType))
- ProcessUtilitySlow(parsetree, queryString,
+ ProcessUtilitySlow(pstate, parsetree, queryString,
context, params,
dest, completionTag);
else
@@ -868,7 +873,7 @@ standard_ProcessUtility(Node *parsetree,
AlterOwnerStmt *stmt = (AlterOwnerStmt *) parsetree;
if (EventTriggerSupportsObjectType(stmt->objectType))
- ProcessUtilitySlow(parsetree, queryString,
+ ProcessUtilitySlow(pstate, parsetree, queryString,
context, params,
dest, completionTag);
else
@@ -881,7 +886,7 @@ standard_ProcessUtility(Node *parsetree,
CommentStmt *stmt = (CommentStmt *) parsetree;
if (EventTriggerSupportsObjectType(stmt->objtype))
- ProcessUtilitySlow(parsetree, queryString,
+ ProcessUtilitySlow(pstate, parsetree, queryString,
context, params,
dest, completionTag);
else
@@ -894,7 +899,7 @@ standard_ProcessUtility(Node *parsetree,
SecLabelStmt *stmt = (SecLabelStmt *) parsetree;
if (EventTriggerSupportsObjectType(stmt->objtype))
- ProcessUtilitySlow(parsetree, queryString,
+ ProcessUtilitySlow(pstate, parsetree, queryString,
context, params,
dest, completionTag);
else
@@ -904,7 +909,7 @@ standard_ProcessUtility(Node *parsetree,
default:
/* All other statement types have event trigger support */
- ProcessUtilitySlow(parsetree, queryString,
+ ProcessUtilitySlow(pstate, parsetree, queryString,
context, params,
dest, completionTag);
break;
@@ -917,7 +922,8 @@ standard_ProcessUtility(Node *parsetree,
* perform the trigger support calls if the context allows it.
*/
static void
-ProcessUtilitySlow(Node *parsetree,
+ProcessUtilitySlow(ParseState *pstate,
+ Node *parsetree,
const char *queryString,
ProcessUtilityContext context,
ParamListInfo params,
@@ -1385,11 +1391,11 @@ ProcessUtilitySlow(Node *parsetree,
break;
case T_CreateSeqStmt:
- address = DefineSequence((CreateSeqStmt *) parsetree);
+ address = DefineSequence(pstate, (CreateSeqStmt *) parsetree);
break;
case T_AlterSeqStmt:
- address = AlterSequence((AlterSeqStmt *) parsetree);
+ address = AlterSequence(pstate, (AlterSeqStmt *) parsetree);
break;
case T_CreateTableAsStmt:
diff --git a/src/include/commands/copy.h b/src/include/commands/copy.h
index 314d1f7..65eb347 100644
--- a/src/include/commands/copy.h
+++ b/src/include/commands/copy.h
@@ -16,16 +16,17 @@
#include "nodes/execnodes.h"
#include "nodes/parsenodes.h"
+#include "parser/parse_node.h"
#include "tcop/dest.h"
/* CopyStateData is private in commands/copy.c */
typedef struct CopyStateData *CopyState;
-extern Oid DoCopy(const CopyStmt *stmt, const char *queryString,
+extern Oid DoCopy(ParseState *state, const CopyStmt *stmt,
uint64 *processed);
-extern void ProcessCopyOptions(CopyState cstate, bool is_from, List *options);
-extern CopyState BeginCopyFrom(Relation rel, const char *filename,
+extern void ProcessCopyOptions(ParseState *pstate, CopyState cstate, bool is_from, List *options);
+extern CopyState BeginCopyFrom(ParseState *pstate, Relation rel, const char *filename,
bool is_program, List *attnamelist, List *options);
extern void EndCopyFrom(CopyState cstate);
extern bool NextCopyFrom(CopyState cstate, ExprContext *econtext,
diff --git a/src/include/commands/sequence.h b/src/include/commands/sequence.h
index 6af60d8..392a626 100644
--- a/src/include/commands/sequence.h
+++ b/src/include/commands/sequence.h
@@ -18,6 +18,7 @@
#include "fmgr.h"
#include "lib/stringinfo.h"
#include "nodes/parsenodes.h"
+#include "parser/parse_node.h"
#include "storage/relfilenode.h"
@@ -73,8 +74,8 @@ extern Datum lastval(PG_FUNCTION_ARGS);
extern Datum pg_sequence_parameters(PG_FUNCTION_ARGS);
-extern ObjectAddress DefineSequence(CreateSeqStmt *stmt);
-extern ObjectAddress AlterSequence(AlterSeqStmt *stmt);
+extern ObjectAddress DefineSequence(ParseState *pstate, CreateSeqStmt *stmt);
+extern ObjectAddress AlterSequence(ParseState *pstate, AlterSeqStmt *stmt);
extern void ResetSequence(Oid seq_relid);
extern void ResetSequenceCaches(void);
diff --git a/src/include/commands/user.h b/src/include/commands/user.h
index d35cb0c..1f0cfcc 100644
--- a/src/include/commands/user.h
+++ b/src/include/commands/user.h
@@ -13,6 +13,7 @@
#include "catalog/objectaddress.h"
#include "nodes/parsenodes.h"
+#include "parser/parse_node.h"
/* Hook to check passwords in CreateRole() and AlterRole() */
@@ -23,7 +24,7 @@ typedef void (*check_password_hook_type) (const char *username, const char *pass
extern PGDLLIMPORT check_password_hook_type check_password_hook;
-extern Oid CreateRole(CreateRoleStmt *stmt);
+extern Oid CreateRole(ParseState *pstate, CreateRoleStmt *stmt);
extern Oid AlterRole(AlterRoleStmt *stmt);
extern Oid AlterRoleSet(AlterRoleSetStmt *stmt);
extern void DropRole(DropRoleStmt *stmt);
diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 1481fff..3716c2e 100644
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -666,6 +666,7 @@ typedef struct DefElem
char *defname;
Node *arg; /* a (Value *) or a (TypeName *) */
DefElemAction defaction; /* unspecified action, or SET/ADD/DROP */
+ int location; /* token location, or -1 if unknown */
} DefElem;
/*
--
2.9.2
From acb47e57a04633b80cbeb027fd9ece84a0b2d750 Mon Sep 17 00:00:00 2001
From: Peter Eisentraut <pete...@gmx.net>
Date: Thu, 4 Aug 2016 11:00:15 -0400
Subject: [PATCH] Factor out duplicate check in List of DefElems
---
src/backend/commands/copy.c | 71 +-------------
src/backend/commands/user.c | 218 +++++-------------------------------------
src/backend/nodes/nodeFuncs.c | 23 +++++
src/include/nodes/nodeFuncs.h | 2 +
4 files changed, 50 insertions(+), 264 deletions(-)
diff --git a/src/backend/commands/copy.c b/src/backend/commands/copy.c
index f45b330..b2842d8 100644
--- a/src/backend/commands/copy.c
+++ b/src/backend/commands/copy.c
@@ -34,6 +34,7 @@
#include "libpq/pqformat.h"
#include "mb/pg_wchar.h"
#include "miscadmin.h"
+#include "nodes/nodeFuncs.h"
#include "optimizer/clauses.h"
#include "optimizer/planner.h"
#include "nodes/makefuncs.h"
@@ -984,7 +985,6 @@ ProcessCopyOptions(CopyState cstate,
bool is_from,
List *options)
{
- bool format_specified = false;
ListCell *option;
/* Support external use for option sanity checking */
@@ -993,6 +993,8 @@ ProcessCopyOptions(CopyState cstate,
cstate->file_encoding = -1;
+ DefElem_List_check_duplicates(options);
+
/* Extract options from the statement node tree */
foreach(option, options)
{
@@ -1002,11 +1004,6 @@ ProcessCopyOptions(CopyState cstate,
{
char *fmt = defGetString(defel);
- if (format_specified)
- ereport(ERROR,
- (errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("conflicting or redundant options")));
- format_specified = true;
if (strcmp(fmt, "text") == 0)
/* default format */ ;
else if (strcmp(fmt, "csv") == 0)
@@ -1019,67 +1016,21 @@ ProcessCopyOptions(CopyState cstate,
errmsg("COPY format \"%s\" not recognized", fmt)));
}
else if (strcmp(defel->defname, "oids") == 0)
- {
- if (cstate->oids)
- ereport(ERROR,
- (errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("conflicting or redundant options")));
cstate->oids = defGetBoolean(defel);
- }
else if (strcmp(defel->defname, "freeze") == 0)
- {
- if (cstate->freeze)
- ereport(ERROR,
- (errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("conflicting or redundant options")));
cstate->freeze = defGetBoolean(defel);
- }
else if (strcmp(defel->defname, "delimiter") == 0)
- {
- if (cstate->delim)
- ereport(ERROR,
- (errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("conflicting or redundant options")));
cstate->delim = defGetString(defel);
- }
else if (strcmp(defel->defname, "null") == 0)
- {
- if (cstate->null_print)
- ereport(ERROR,
- (errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("conflicting or redundant options")));
cstate->null_print = defGetString(defel);
- }
else if (strcmp(defel->defname, "header") == 0)
- {
- if (cstate->header_line)
- ereport(ERROR,
- (errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("conflicting or redundant options")));
cstate->header_line = defGetBoolean(defel);
- }
else if (strcmp(defel->defname, "quote") == 0)
- {
- if (cstate->quote)
- ereport(ERROR,
- (errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("conflicting or redundant options")));
cstate->quote = defGetString(defel);
- }
else if (strcmp(defel->defname, "escape") == 0)
- {
- if (cstate->escape)
- ereport(ERROR,
- (errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("conflicting or redundant options")));
cstate->escape = defGetString(defel);
- }
else if (strcmp(defel->defname, "force_quote") == 0)
{
- if (cstate->force_quote || cstate->force_quote_all)
- ereport(ERROR,
- (errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("conflicting or redundant options")));
if (defel->arg && IsA(defel->arg, A_Star))
cstate->force_quote_all = true;
else if (defel->arg && IsA(defel->arg, List))
@@ -1092,10 +1043,6 @@ ProcessCopyOptions(CopyState cstate,
}
else if (strcmp(defel->defname, "force_not_null") == 0)
{
- if (cstate->force_notnull)
- ereport(ERROR,
- (errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("conflicting or redundant options")));
if (defel->arg && IsA(defel->arg, List))
cstate->force_notnull = (List *) defel->arg;
else
@@ -1106,10 +1053,6 @@ ProcessCopyOptions(CopyState cstate,
}
else if (strcmp(defel->defname, "force_null") == 0)
{
- if (cstate->force_null)
- ereport(ERROR,
- (errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("conflicting or redundant options")));
if (defel->arg && IsA(defel->arg, List))
cstate->force_null = (List *) defel->arg;
else
@@ -1125,10 +1068,6 @@ ProcessCopyOptions(CopyState cstate,
* named columns to binary form, storing the rest as NULLs. It's
* allowed for the column list to be NIL.
*/
- if (cstate->convert_selectively)
- ereport(ERROR,
- (errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("conflicting or redundant options")));
cstate->convert_selectively = true;
if (defel->arg == NULL || IsA(defel->arg, List))
cstate->convert_select = (List *) defel->arg;
@@ -1140,10 +1079,6 @@ ProcessCopyOptions(CopyState cstate,
}
else if (strcmp(defel->defname, "encoding") == 0)
{
- if (cstate->file_encoding >= 0)
- ereport(ERROR,
- (errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("conflicting or redundant options")));
cstate->file_encoding = pg_char_to_encoding(defGetString(defel));
if (cstate->file_encoding < 0)
ereport(ERROR,
diff --git a/src/backend/commands/user.c b/src/backend/commands/user.c
index b6ea950..628ff78 100644
--- a/src/backend/commands/user.c
+++ b/src/backend/commands/user.c
@@ -30,6 +30,7 @@
#include "commands/seclabel.h"
#include "commands/user.h"
#include "libpq/md5.h"
+#include "nodes/nodeFuncs.h"
#include "miscadmin.h"
#include "storage/lmgr.h"
#include "utils/acl.h"
@@ -97,19 +98,6 @@ CreateRole(CreateRoleStmt *stmt)
char *validUntil = NULL; /* time the login is valid until */
Datum validUntil_datum; /* same, as timestamptz Datum */
bool validUntil_null;
- DefElem *dpassword = NULL;
- DefElem *dissuper = NULL;
- DefElem *dinherit = NULL;
- DefElem *dcreaterole = NULL;
- DefElem *dcreatedb = NULL;
- DefElem *dcanlogin = NULL;
- DefElem *disreplication = NULL;
- DefElem *dconnlimit = NULL;
- DefElem *daddroleto = NULL;
- DefElem *drolemembers = NULL;
- DefElem *dadminmembers = NULL;
- DefElem *dvalidUntil = NULL;
- DefElem *dbypassRLS = NULL;
/* The defaults can vary depending on the original statement type */
switch (stmt->stmt_type)
@@ -124,6 +112,8 @@ CreateRole(CreateRoleStmt *stmt)
break;
}
+ DefElem_List_check_duplicates(stmt->options);
+
/* Extract options from the statement node tree */
foreach(option, stmt->options)
{
@@ -133,11 +123,8 @@ CreateRole(CreateRoleStmt *stmt)
strcmp(defel->defname, "encryptedPassword") == 0 ||
strcmp(defel->defname, "unencryptedPassword") == 0)
{
- if (dpassword)
- ereport(ERROR,
- (errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("conflicting or redundant options")));
- dpassword = defel;
+ if (defel->arg)
+ password = strVal(defel->arg);
if (strcmp(defel->defname, "encryptedPassword") == 0)
encrypt_password = true;
else if (strcmp(defel->defname, "unencryptedPassword") == 0)
@@ -149,139 +136,40 @@ CreateRole(CreateRoleStmt *stmt)
(errmsg("SYSID can no longer be specified")));
}
else if (strcmp(defel->defname, "superuser") == 0)
- {
- if (dissuper)
- ereport(ERROR,
- (errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("conflicting or redundant options")));
- dissuper = defel;
- }
+ issuper = intVal(defel->arg) != 0;
else if (strcmp(defel->defname, "inherit") == 0)
- {
- if (dinherit)
- ereport(ERROR,
- (errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("conflicting or redundant options")));
- dinherit = defel;
- }
+ inherit = intVal(defel->arg) != 0;
else if (strcmp(defel->defname, "createrole") == 0)
- {
- if (dcreaterole)
- ereport(ERROR,
- (errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("conflicting or redundant options")));
- dcreaterole = defel;
- }
+ createrole = intVal(defel->arg) != 0;
else if (strcmp(defel->defname, "createdb") == 0)
- {
- if (dcreatedb)
- ereport(ERROR,
- (errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("conflicting or redundant options")));
- dcreatedb = defel;
- }
+ createdb = intVal(defel->arg) != 0;
else if (strcmp(defel->defname, "canlogin") == 0)
- {
- if (dcanlogin)
- ereport(ERROR,
- (errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("conflicting or redundant options")));
- dcanlogin = defel;
- }
+ canlogin = intVal(defel->arg) != 0;
else if (strcmp(defel->defname, "isreplication") == 0)
- {
- if (disreplication)
- ereport(ERROR,
- (errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("conflicting or redundant options")));
- disreplication = defel;
- }
+ isreplication = intVal(defel->arg) != 0;
else if (strcmp(defel->defname, "connectionlimit") == 0)
{
- if (dconnlimit)
+ connlimit = intVal(defel->arg);
+ if (connlimit < -1)
ereport(ERROR,
- (errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("conflicting or redundant options")));
- dconnlimit = defel;
+ (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
+ errmsg("invalid connection limit: %d", connlimit)));
}
else if (strcmp(defel->defname, "addroleto") == 0)
- {
- if (daddroleto)
- ereport(ERROR,
- (errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("conflicting or redundant options")));
- daddroleto = defel;
- }
+ addroleto = (List *) defel->arg;
else if (strcmp(defel->defname, "rolemembers") == 0)
- {
- if (drolemembers)
- ereport(ERROR,
- (errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("conflicting or redundant options")));
- drolemembers = defel;
- }
+ rolemembers = (List *) defel->arg;
else if (strcmp(defel->defname, "adminmembers") == 0)
- {
- if (dadminmembers)
- ereport(ERROR,
- (errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("conflicting or redundant options")));
- dadminmembers = defel;
- }
+ adminmembers = (List *) defel->arg;
else if (strcmp(defel->defname, "validUntil") == 0)
- {
- if (dvalidUntil)
- ereport(ERROR,
- (errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("conflicting or redundant options")));
- dvalidUntil = defel;
- }
+ validUntil = strVal(defel->arg);
else if (strcmp(defel->defname, "bypassrls") == 0)
- {
- if (dbypassRLS)
- ereport(ERROR,
- (errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("conflicting or redundant options")));
- dbypassRLS = defel;
- }
+ bypassrls = intVal(defel->arg) != 0;
else
elog(ERROR, "option \"%s\" not recognized",
defel->defname);
}
- if (dpassword && dpassword->arg)
- password = strVal(dpassword->arg);
- if (dissuper)
- issuper = intVal(dissuper->arg) != 0;
- if (dinherit)
- inherit = intVal(dinherit->arg) != 0;
- if (dcreaterole)
- createrole = intVal(dcreaterole->arg) != 0;
- if (dcreatedb)
- createdb = intVal(dcreatedb->arg) != 0;
- if (dcanlogin)
- canlogin = intVal(dcanlogin->arg) != 0;
- if (disreplication)
- isreplication = intVal(disreplication->arg) != 0;
- if (dconnlimit)
- {
- connlimit = intVal(dconnlimit->arg);
- if (connlimit < -1)
- ereport(ERROR,
- (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
- errmsg("invalid connection limit: %d", connlimit)));
- }
- if (daddroleto)
- addroleto = (List *) daddroleto->arg;
- if (drolemembers)
- rolemembers = (List *) drolemembers->arg;
- if (dadminmembers)
- adminmembers = (List *) dadminmembers->arg;
- if (dvalidUntil)
- validUntil = strVal(dvalidUntil->arg);
- if (dbypassRLS)
- bypassrls = intVal(dbypassRLS->arg) != 0;
-
/* Check some permissions first */
if (issuper)
{
@@ -522,6 +410,8 @@ AlterRole(AlterRoleStmt *stmt)
check_rolespec_name(stmt->role,
"Cannot alter reserved roles.");
+ DefElem_List_check_duplicates(stmt->options);
+
/* Extract options from the statement node tree */
foreach(option, stmt->options)
{
@@ -531,10 +421,6 @@ AlterRole(AlterRoleStmt *stmt)
strcmp(defel->defname, "encryptedPassword") == 0 ||
strcmp(defel->defname, "unencryptedPassword") == 0)
{
- if (dpassword)
- ereport(ERROR,
- (errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("conflicting or redundant options")));
dpassword = defel;
if (strcmp(defel->defname, "encryptedPassword") == 0)
encrypt_password = true;
@@ -542,86 +428,26 @@ AlterRole(AlterRoleStmt *stmt)
encrypt_password = false;
}
else if (strcmp(defel->defname, "superuser") == 0)
- {
- if (dissuper)
- ereport(ERROR,
- (errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("conflicting or redundant options")));
dissuper = defel;
- }
else if (strcmp(defel->defname, "inherit") == 0)
- {
- if (dinherit)
- ereport(ERROR,
- (errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("conflicting or redundant options")));
dinherit = defel;
- }
else if (strcmp(defel->defname, "createrole") == 0)
- {
- if (dcreaterole)
- ereport(ERROR,
- (errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("conflicting or redundant options")));
dcreaterole = defel;
- }
else if (strcmp(defel->defname, "createdb") == 0)
- {
- if (dcreatedb)
- ereport(ERROR,
- (errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("conflicting or redundant options")));
dcreatedb = defel;
- }
else if (strcmp(defel->defname, "canlogin") == 0)
- {
- if (dcanlogin)
- ereport(ERROR,
- (errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("conflicting or redundant options")));
dcanlogin = defel;
- }
else if (strcmp(defel->defname, "isreplication") == 0)
- {
- if (disreplication)
- ereport(ERROR,
- (errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("conflicting or redundant options")));
disreplication = defel;
- }
else if (strcmp(defel->defname, "connectionlimit") == 0)
- {
- if (dconnlimit)
- ereport(ERROR,
- (errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("conflicting or redundant options")));
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")));
drolemembers = defel;
- }
else if (strcmp(defel->defname, "validUntil") == 0)
- {
- if (dvalidUntil)
- ereport(ERROR,
- (errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("conflicting or redundant options")));
dvalidUntil = defel;
- }
else if (strcmp(defel->defname, "bypassrls") == 0)
- {
- if (dbypassRLS)
- ereport(ERROR,
- (errcode(ERRCODE_SYNTAX_ERROR),
- errmsg("conflicting or redundant options")));
dbypassRLS = defel;
- }
else
elog(ERROR, "option \"%s\" not recognized",
defel->defname);
diff --git a/src/backend/nodes/nodeFuncs.c b/src/backend/nodes/nodeFuncs.c
index cd39167..82129d7 100644
--- a/src/backend/nodes/nodeFuncs.c
+++ b/src/backend/nodes/nodeFuncs.c
@@ -3740,3 +3740,26 @@ planstate_walk_members(List *plans, PlanState **planstates,
return false;
}
+
+
+void
+DefElem_List_check_duplicates(List *list)
+{
+ ListCell *cell;
+
+ foreach(cell, list)
+ {
+ DefElem *defel = (DefElem *) lfirst(cell);
+ ListCell *cell2;
+
+ for_each_cell(cell2, lnext(cell))
+ {
+ DefElem *defel2 = (DefElem *) lfirst(cell2);
+
+ if (strcmp(defel->defname, defel2->defname) == 0)
+ ereport(ERROR,
+ (errcode(ERRCODE_SYNTAX_ERROR),
+ errmsg("conflicting or redundant options")));
+ }
+ }
+}
diff --git a/src/include/nodes/nodeFuncs.h b/src/include/nodes/nodeFuncs.h
index 97af142..3b30ca7 100644
--- a/src/include/nodes/nodeFuncs.h
+++ b/src/include/nodes/nodeFuncs.h
@@ -77,4 +77,6 @@ struct PlanState;
extern bool planstate_tree_walker(struct PlanState *planstate, bool (*walker) (),
void *context);
+extern void DefElem_List_check_duplicates(List *list);
+
#endif /* NODEFUNCS_H */
--
2.9.2
--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers
