From acb47e57a04633b80cbeb027fd9ece84a0b2d750 Mon Sep 17 00:00:00 2001 From: Peter Eisentraut 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