Here is a patch series that implements several changes in the internal grammar and node representation of function signatures. They are not necessarily meant to be applied separately, but they explain the progression of the changes nicely, so I left them like that for review.
The end goal is to make some user-visible changes in DROP FUNCTION and possibly other commands that refer to functions. With these patches, it is now possible to use DROP FUNCTION to drop multiple functions at once: DROP FUNCTION func1(), func2(), func3(). Other DROP commands already supported that, but DROP FUNCTION didn't because the internal representation was complicated and couldn't handle it. The next step after this would be to allow referring to functions without having to supply the arguments, if the name is unique. This is an SQL-standard feature and would be very useful for dealing "business logic" functions with 10+ arguments. The details of that are to be worked out, but with the help of the present changes, this would be a quite localized change, because the grammar representation is well encapsulated. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
From 0a5d27cf9ab91bb03daa956d5167c5c993dfb857 Mon Sep 17 00:00:00 2001 From: Peter Eisentraut <pete...@gmx.net> Date: Thu, 15 Sep 2016 12:00:00 -0500 Subject: [PATCH 1/7] Use grammar symbol function_with_argtypes consistently Instead of sometimes referring to a function signature like func_name func_args, use the existing function_with_argtypes symbol, which combines the two. --- src/backend/parser/gram.y | 44 ++++++++++++++++++++++---------------------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index 5547fc8..755b387 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -5366,21 +5366,21 @@ opclass_item: n->order_family = $5; $$ = (Node *) n; } - | FUNCTION Iconst func_name func_args + | FUNCTION Iconst function_with_argtypes { CreateOpClassItem *n = makeNode(CreateOpClassItem); n->itemtype = OPCLASS_ITEM_FUNCTION; - n->name = $3; - n->args = extractArgTypes($4); + n->name = $3->funcname; + n->args = $3->funcargs; n->number = $2; $$ = (Node *) n; } - | FUNCTION Iconst '(' type_list ')' func_name func_args + | FUNCTION Iconst '(' type_list ')' function_with_argtypes { CreateOpClassItem *n = makeNode(CreateOpClassItem); n->itemtype = OPCLASS_ITEM_FUNCTION; - n->name = $6; - n->args = extractArgTypes($7); + n->name = $6->funcname; + n->args = $6->funcargs; n->number = $2; n->class_args = $4; $$ = (Node *) n; @@ -5780,13 +5780,13 @@ CommentStmt: n->comment = $7; $$ = (Node *) n; } - | COMMENT ON FUNCTION func_name func_args IS comment_text + | COMMENT ON FUNCTION function_with_argtypes IS comment_text { CommentStmt *n = makeNode(CommentStmt); n->objtype = OBJECT_FUNCTION; - n->objname = $4; - n->objargs = extractArgTypes($5); - n->comment = $7; + n->objname = $4->funcname; + n->objargs = $4->funcargs; + n->comment = $6; $$ = (Node *) n; } | COMMENT ON OPERATOR any_operator oper_argtypes IS comment_text @@ -5998,15 +5998,15 @@ SecLabelStmt: n->label = $9; $$ = (Node *) n; } - | SECURITY LABEL opt_provider ON FUNCTION func_name func_args + | SECURITY LABEL opt_provider ON FUNCTION function_with_argtypes IS security_label { SecLabelStmt *n = makeNode(SecLabelStmt); n->provider = $3; n->objtype = OBJECT_FUNCTION; - n->objname = $6; - n->objargs = extractArgTypes($7); - n->label = $9; + n->objname = $6->funcname; + n->objargs = $6->funcargs; + n->label = $8; $$ = (Node *) n; } | SECURITY LABEL opt_provider ON LARGE_P OBJECT_P NumericOnly @@ -7236,24 +7236,24 @@ opt_restrict: *****************************************************************************/ RemoveFuncStmt: - DROP FUNCTION func_name func_args opt_drop_behavior + DROP FUNCTION function_with_argtypes opt_drop_behavior { DropStmt *n = makeNode(DropStmt); n->removeType = OBJECT_FUNCTION; - n->objects = list_make1($3); - n->arguments = list_make1(extractArgTypes($4)); - n->behavior = $5; + n->objects = list_make1($3->funcname); + n->arguments = list_make1($3->funcargs); + n->behavior = $4; n->missing_ok = false; n->concurrent = false; $$ = (Node *)n; } - | DROP FUNCTION IF_P EXISTS func_name func_args opt_drop_behavior + | DROP FUNCTION IF_P EXISTS function_with_argtypes opt_drop_behavior { DropStmt *n = makeNode(DropStmt); n->removeType = OBJECT_FUNCTION; - n->objects = list_make1($5); - n->arguments = list_make1(extractArgTypes($6)); - n->behavior = $7; + n->objects = list_make1($5->funcname); + n->arguments = list_make1($5->funcargs); + n->behavior = $6; n->missing_ok = true; n->concurrent = false; $$ = (Node *)n; -- 2.10.2
From 1026aa3068e2a77b85aeb5f5f36e1d427bd1d6d9 Mon Sep 17 00:00:00 2001 From: Peter Eisentraut <pete...@gmx.net> Date: Thu, 15 Sep 2016 12:00:00 -0500 Subject: [PATCH 2/7] Move function_with_argtypes to a better location It was apparently added for use by GRANT/REVOKE, but move it closer to where other function signature related things are kept. --- src/backend/parser/gram.y | 32 ++++++++++++++++---------------- 1 file changed, 16 insertions(+), 16 deletions(-) diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index 755b387..37b5151 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -6485,22 +6485,6 @@ opt_grant_grant_option: | /*EMPTY*/ { $$ = FALSE; } ; -function_with_argtypes_list: - function_with_argtypes { $$ = list_make1($1); } - | function_with_argtypes_list ',' function_with_argtypes - { $$ = lappend($1, $3); } - ; - -function_with_argtypes: - func_name func_args - { - FuncWithArgs *n = makeNode(FuncWithArgs); - n->funcname = $1; - n->funcargs = extractArgTypes($2); - $$ = n; - } - ; - /***************************************************************************** * * GRANT and REVOKE ROLE statements @@ -6853,6 +6837,22 @@ func_args_list: | func_args_list ',' func_arg { $$ = lappend($1, $3); } ; +function_with_argtypes_list: + function_with_argtypes { $$ = list_make1($1); } + | function_with_argtypes_list ',' function_with_argtypes + { $$ = lappend($1, $3); } + ; + +function_with_argtypes: + func_name func_args + { + FuncWithArgs *n = makeNode(FuncWithArgs); + n->funcname = $1; + n->funcargs = extractArgTypes($2); + $$ = n; + } + ; + /* * func_args_with_defaults is separate because we only want to accept * defaults in CREATE FUNCTION, not in ALTER etc. -- 2.10.2
From 61c392466d132ce8f4ef9309cce31b403eed4fe6 Mon Sep 17 00:00:00 2001 From: Peter Eisentraut <pete...@gmx.net> Date: Thu, 15 Sep 2016 12:00:00 -0500 Subject: [PATCH 3/7] Add aggregate_with_argtypes and use it consistently This works like function_with_argtypes, but aggregates allow slightly different arguments. --- src/backend/parser/gram.y | 74 +++++++++++++++++++++++++++-------------------- 1 file changed, 42 insertions(+), 32 deletions(-) diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index 37b5151..d4d5467 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -338,7 +338,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query); %type <accesspriv> privilege %type <list> privileges privilege_list %type <privtarget> privilege_target -%type <funwithargs> function_with_argtypes +%type <funwithargs> function_with_argtypes aggregate_with_argtypes %type <list> function_with_argtypes_list %type <ival> defacl_privilege_target %type <defelt> DefACLOption @@ -3940,14 +3940,14 @@ AlterExtensionContentsStmt: n->objname = list_make1(makeString($7)); $$ = (Node *)n; } - | ALTER EXTENSION name add_drop AGGREGATE func_name aggr_args + | ALTER EXTENSION name add_drop AGGREGATE aggregate_with_argtypes { AlterExtensionContentsStmt *n = makeNode(AlterExtensionContentsStmt); n->extname = $3; n->action = $4; n->objtype = OBJECT_AGGREGATE; - n->objname = $6; - n->objargs = extractAggrArgTypes($7); + n->objname = $6->funcname; + n->objargs = $6->funcargs; $$ = (Node *)n; } | ALTER EXTENSION name add_drop CAST '(' Typename AS Typename ')' @@ -5771,13 +5771,13 @@ CommentStmt: n->comment = $6; $$ = (Node *) n; } - | COMMENT ON AGGREGATE func_name aggr_args IS comment_text + | COMMENT ON AGGREGATE aggregate_with_argtypes IS comment_text { CommentStmt *n = makeNode(CommentStmt); n->objtype = OBJECT_AGGREGATE; - n->objname = $4; - n->objargs = extractAggrArgTypes($5); - n->comment = $7; + n->objname = $4->funcname; + n->objargs = $4->funcargs; + n->comment = $6; $$ = (Node *) n; } | COMMENT ON FUNCTION function_with_argtypes IS comment_text @@ -5987,15 +5987,15 @@ SecLabelStmt: n->label = $8; $$ = (Node *) n; } - | SECURITY LABEL opt_provider ON AGGREGATE func_name aggr_args + | SECURITY LABEL opt_provider ON AGGREGATE aggregate_with_argtypes IS security_label { SecLabelStmt *n = makeNode(SecLabelStmt); n->provider = $3; n->objtype = OBJECT_AGGREGATE; - n->objname = $6; - n->objargs = extractAggrArgTypes($7); - n->label = $9; + n->objname = $6->funcname; + n->objargs = $6->funcargs; + n->label = $8; $$ = (Node *) n; } | SECURITY LABEL opt_provider ON FUNCTION function_with_argtypes @@ -7055,6 +7055,16 @@ aggr_args_list: | aggr_args_list ',' aggr_arg { $$ = lappend($1, $3); } ; +aggregate_with_argtypes: + func_name aggr_args + { + FuncWithArgs *n = makeNode(FuncWithArgs); + n->funcname = $1; + n->funcargs = extractAggrArgTypes($2); + $$ = n; + } + ; + createfunc_opt_list: /* Must be at least one to prevent conflict */ createfunc_opt_item { $$ = list_make1($1); } @@ -7261,24 +7271,24 @@ RemoveFuncStmt: ; RemoveAggrStmt: - DROP AGGREGATE func_name aggr_args opt_drop_behavior + DROP AGGREGATE aggregate_with_argtypes opt_drop_behavior { DropStmt *n = makeNode(DropStmt); n->removeType = OBJECT_AGGREGATE; - n->objects = list_make1($3); - n->arguments = list_make1(extractAggrArgTypes($4)); - n->behavior = $5; + n->objects = list_make1($3->funcname); + n->arguments = list_make1($3->funcargs); + n->behavior = $4; n->missing_ok = false; n->concurrent = false; $$ = (Node *)n; } - | DROP AGGREGATE IF_P EXISTS func_name aggr_args opt_drop_behavior + | DROP AGGREGATE IF_P EXISTS aggregate_with_argtypes opt_drop_behavior { DropStmt *n = makeNode(DropStmt); n->removeType = OBJECT_AGGREGATE; - n->objects = list_make1($5); - n->arguments = list_make1(extractAggrArgTypes($6)); - n->behavior = $7; + n->objects = list_make1($5->funcname); + n->arguments = list_make1($5->funcargs); + n->behavior = $6; n->missing_ok = true; n->concurrent = false; $$ = (Node *)n; @@ -7577,13 +7587,13 @@ AlterTblSpcStmt: * *****************************************************************************/ -RenameStmt: ALTER AGGREGATE func_name aggr_args RENAME TO name +RenameStmt: ALTER AGGREGATE aggregate_with_argtypes RENAME TO name { RenameStmt *n = makeNode(RenameStmt); n->renameType = OBJECT_AGGREGATE; - n->object = $3; - n->objarg = extractAggrArgTypes($4); - n->newname = $7; + n->object = $3->funcname; + n->objarg = $3->funcargs; + n->newname = $6; n->missing_ok = false; $$ = (Node *)n; } @@ -8109,13 +8119,13 @@ AlterObjectDependsStmt: *****************************************************************************/ AlterObjectSchemaStmt: - ALTER AGGREGATE func_name aggr_args SET SCHEMA name + ALTER AGGREGATE aggregate_with_argtypes SET SCHEMA name { AlterObjectSchemaStmt *n = makeNode(AlterObjectSchemaStmt); n->objectType = OBJECT_AGGREGATE; - n->object = $3; - n->objarg = extractAggrArgTypes($4); - n->newschema = $7; + n->object = $3->funcname; + n->objarg = $3->funcargs; + n->newschema = $6; n->missing_ok = false; $$ = (Node *)n; } @@ -8363,13 +8373,13 @@ operator_def_elem: ColLabel '=' NONE * *****************************************************************************/ -AlterOwnerStmt: ALTER AGGREGATE func_name aggr_args OWNER TO RoleSpec +AlterOwnerStmt: ALTER AGGREGATE aggregate_with_argtypes OWNER TO RoleSpec { AlterOwnerStmt *n = makeNode(AlterOwnerStmt); n->objectType = OBJECT_AGGREGATE; - n->object = $3; - n->objarg = extractAggrArgTypes($4); - n->newowner = $7; + n->object = $3->funcname; + n->objarg = $3->funcargs; + n->newowner = $6; $$ = (Node *)n; } | ALTER COLLATION any_name OWNER TO RoleSpec -- 2.10.2
From 8f86ed1aceec8cff7e402e59f1dd49a140daf260 Mon Sep 17 00:00:00 2001 From: Peter Eisentraut <pete...@gmx.net> Date: Thu, 15 Sep 2016 12:00:00 -0400 Subject: [PATCH 4/7] Pass around function signatures as FuncWithArgs Use the existing FuncWithArgs node type for passing around function signatures, instead of always passing around name and argument list as two separate variables. This makes some things simpler, because only one value has to be carried around. --- src/backend/catalog/objectaddress.c | 38 ++++++++++++++++------- src/backend/commands/dropcmds.c | 30 +++++++++++-------- src/backend/parser/gram.y | 60 ++++++++++++++++--------------------- 3 files changed, 71 insertions(+), 57 deletions(-) diff --git a/src/backend/catalog/objectaddress.c b/src/backend/catalog/objectaddress.c index d531d17..905a488 100644 --- a/src/backend/catalog/objectaddress.c +++ b/src/backend/catalog/objectaddress.c @@ -820,17 +820,23 @@ get_object_address(ObjectType objtype, List *objname, List *objargs, address = get_object_address_type(objtype, list_head(objname), missing_ok); break; case OBJECT_AGGREGATE: - address.classId = ProcedureRelationId; - address.objectId = - LookupAggNameTypeNames(objname, objargs, missing_ok); - address.objectSubId = 0; - break; + { + FuncWithArgs *fwa = (FuncWithArgs *) linitial(objname); + address.classId = ProcedureRelationId; + address.objectId = + LookupAggNameTypeNames(fwa->funcname, fwa->funcargs, missing_ok); + address.objectSubId = 0; + break; + } case OBJECT_FUNCTION: - address.classId = ProcedureRelationId; - address.objectId = - LookupFuncNameTypeNames(objname, objargs, missing_ok); - address.objectSubId = 0; - break; + { + FuncWithArgs *fwa = (FuncWithArgs *) linitial(objname); + address.classId = ProcedureRelationId; + address.objectId = + LookupFuncNameTypeNames(fwa->funcname, fwa->funcargs, missing_ok); + address.objectSubId = 0; + break; + } case OBJECT_OPERATOR: Assert(list_length(objargs) == 2); address.classId = OperatorRelationId; @@ -1992,6 +1998,16 @@ pg_get_object_address(PG_FUNCTION_ARGS) args = textarray_to_strvaluelist(argsarr); } + if (type == OBJECT_FUNCTION || type == OBJECT_AGGREGATE) + { + FuncWithArgs *fwa = makeNode(FuncWithArgs); + + fwa->funcname = name; + fwa->funcargs = args; + name = list_make1(fwa); + args = NIL; + } + /* * get_object_name is pretty sensitive to the length its input lists; * check that they're what it wants. @@ -2100,7 +2116,7 @@ check_object_ownership(Oid roleid, ObjectType objtype, ObjectAddress address, case OBJECT_FUNCTION: if (!pg_proc_ownercheck(address.objectId, roleid)) aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_PROC, - NameListToString(objname)); + NameListToString(((FuncWithArgs *) linitial(objname))->funcname)); break; case OBJECT_OPERATOR: if (!pg_oper_ownercheck(address.objectId, roleid)) diff --git a/src/backend/commands/dropcmds.c b/src/backend/commands/dropcmds.c index 61ff8f2..ecf1428 100644 --- a/src/backend/commands/dropcmds.c +++ b/src/backend/commands/dropcmds.c @@ -110,7 +110,7 @@ RemoveObjects(DropStmt *stmt) ereport(ERROR, (errcode(ERRCODE_WRONG_OBJECT_TYPE), errmsg("\"%s\" is an aggregate function", - NameListToString(objname)), + NameListToString(((FuncWithArgs *) linitial(objname))->funcname)), errhint("Use DROP AGGREGATE to drop aggregate functions."))); ReleaseSysCache(tup); @@ -329,21 +329,27 @@ does_not_exist_skipping(ObjectType objtype, List *objname, List *objargs) name = NameListToString(objname); break; case OBJECT_FUNCTION: - if (!schema_does_not_exist_skipping(objname, &msg, &name) && - !type_in_list_does_not_exist_skipping(objargs, &msg, &name)) { - msg = gettext_noop("function %s(%s) does not exist, skipping"); - name = NameListToString(objname); - args = TypeNameListToString(objargs); + FuncWithArgs *fwa = (FuncWithArgs *) linitial(objname); + if (!schema_does_not_exist_skipping(fwa->funcname, &msg, &name) && + !type_in_list_does_not_exist_skipping(fwa->funcargs, &msg, &name)) + { + msg = gettext_noop("function %s(%s) does not exist, skipping"); + name = NameListToString(fwa->funcname); + args = TypeNameListToString(fwa->funcargs); + } + break; } - break; case OBJECT_AGGREGATE: - if (!schema_does_not_exist_skipping(objname, &msg, &name) && - !type_in_list_does_not_exist_skipping(objargs, &msg, &name)) { - msg = gettext_noop("aggregate %s(%s) does not exist, skipping"); - name = NameListToString(objname); - args = TypeNameListToString(objargs); + FuncWithArgs *fwa = (FuncWithArgs *) linitial(objname); + if (!schema_does_not_exist_skipping(fwa->funcname, &msg, &name) && + !type_in_list_does_not_exist_skipping(fwa->funcargs, &msg, &name)) + { + msg = gettext_noop("aggregate %s(%s) does not exist, skipping"); + name = NameListToString(fwa->funcname); + args = TypeNameListToString(fwa->funcargs); + } } break; case OBJECT_OPERATOR: diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index d4d5467..27110a5 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -3946,8 +3946,7 @@ AlterExtensionContentsStmt: n->extname = $3; n->action = $4; n->objtype = OBJECT_AGGREGATE; - n->objname = $6->funcname; - n->objargs = $6->funcargs; + n->objname = list_make1($6); $$ = (Node *)n; } | ALTER EXTENSION name add_drop CAST '(' Typename AS Typename ')' @@ -3993,8 +3992,7 @@ AlterExtensionContentsStmt: n->extname = $3; n->action = $4; n->objtype = OBJECT_FUNCTION; - n->objname = $6->funcname; - n->objargs = $6->funcargs; + n->objname = list_make1($6); $$ = (Node *)n; } | ALTER EXTENSION name add_drop opt_procedural LANGUAGE name @@ -5775,8 +5773,8 @@ CommentStmt: { CommentStmt *n = makeNode(CommentStmt); n->objtype = OBJECT_AGGREGATE; - n->objname = $4->funcname; - n->objargs = $4->funcargs; + n->objname = list_make1($4); + n->objargs = NIL; n->comment = $6; $$ = (Node *) n; } @@ -5784,8 +5782,8 @@ CommentStmt: { CommentStmt *n = makeNode(CommentStmt); n->objtype = OBJECT_FUNCTION; - n->objname = $4->funcname; - n->objargs = $4->funcargs; + n->objname = list_make1($4); + n->objargs = NIL; n->comment = $6; $$ = (Node *) n; } @@ -5993,8 +5991,8 @@ SecLabelStmt: SecLabelStmt *n = makeNode(SecLabelStmt); n->provider = $3; n->objtype = OBJECT_AGGREGATE; - n->objname = $6->funcname; - n->objargs = $6->funcargs; + n->objname = list_make1($6); + n->objargs = NIL; n->label = $8; $$ = (Node *) n; } @@ -6004,8 +6002,8 @@ SecLabelStmt: SecLabelStmt *n = makeNode(SecLabelStmt); n->provider = $3; n->objtype = OBJECT_FUNCTION; - n->objname = $6->funcname; - n->objargs = $6->funcargs; + n->objname = list_make1($6); + n->objargs = NIL; n->label = $8; $$ = (Node *) n; } @@ -7250,8 +7248,8 @@ RemoveFuncStmt: { DropStmt *n = makeNode(DropStmt); n->removeType = OBJECT_FUNCTION; - n->objects = list_make1($3->funcname); - n->arguments = list_make1($3->funcargs); + n->objects = list_make1(list_make1($3)); + n->arguments = NIL; n->behavior = $4; n->missing_ok = false; n->concurrent = false; @@ -7261,8 +7259,8 @@ RemoveFuncStmt: { DropStmt *n = makeNode(DropStmt); n->removeType = OBJECT_FUNCTION; - n->objects = list_make1($5->funcname); - n->arguments = list_make1($5->funcargs); + n->objects = list_make1(list_make1($5)); + n->arguments = NIL; n->behavior = $6; n->missing_ok = true; n->concurrent = false; @@ -7275,8 +7273,8 @@ RemoveAggrStmt: { DropStmt *n = makeNode(DropStmt); n->removeType = OBJECT_AGGREGATE; - n->objects = list_make1($3->funcname); - n->arguments = list_make1($3->funcargs); + n->objects = list_make1(list_make1($3)); + n->arguments = NIL; n->behavior = $4; n->missing_ok = false; n->concurrent = false; @@ -7286,8 +7284,8 @@ RemoveAggrStmt: { DropStmt *n = makeNode(DropStmt); n->removeType = OBJECT_AGGREGATE; - n->objects = list_make1($5->funcname); - n->arguments = list_make1($5->funcargs); + n->objects = list_make1(list_make1($5)); + n->arguments = NIL; n->behavior = $6; n->missing_ok = true; n->concurrent = false; @@ -7591,8 +7589,7 @@ RenameStmt: ALTER AGGREGATE aggregate_with_argtypes RENAME TO name { RenameStmt *n = makeNode(RenameStmt); n->renameType = OBJECT_AGGREGATE; - n->object = $3->funcname; - n->objarg = $3->funcargs; + n->object = list_make1($3); n->newname = $6; n->missing_ok = false; $$ = (Node *)n; @@ -7655,8 +7652,7 @@ RenameStmt: ALTER AGGREGATE aggregate_with_argtypes RENAME TO name { RenameStmt *n = makeNode(RenameStmt); n->renameType = OBJECT_FUNCTION; - n->object = $3->funcname; - n->objarg = $3->funcargs; + n->object = list_make1($3); n->newname = $6; n->missing_ok = false; $$ = (Node *)n; @@ -8075,8 +8071,8 @@ AlterObjectDependsStmt: AlterObjectDependsStmt *n = makeNode(AlterObjectDependsStmt); n->objectType = OBJECT_FUNCTION; n->relation = NULL; - n->objname = $3->funcname; - n->objargs = $3->funcargs; + n->objname = list_make1($3); + n->objargs = NIL; n->extname = makeString($7); $$ = (Node *)n; } @@ -8123,8 +8119,7 @@ AlterObjectSchemaStmt: { AlterObjectSchemaStmt *n = makeNode(AlterObjectSchemaStmt); n->objectType = OBJECT_AGGREGATE; - n->object = $3->funcname; - n->objarg = $3->funcargs; + n->object = list_make1($3); n->newschema = $6; n->missing_ok = false; $$ = (Node *)n; @@ -8169,8 +8164,7 @@ AlterObjectSchemaStmt: { AlterObjectSchemaStmt *n = makeNode(AlterObjectSchemaStmt); n->objectType = OBJECT_FUNCTION; - n->object = $3->funcname; - n->objarg = $3->funcargs; + n->object = list_make1($3); n->newschema = $6; n->missing_ok = false; $$ = (Node *)n; @@ -8377,8 +8371,7 @@ AlterOwnerStmt: ALTER AGGREGATE aggregate_with_argtypes OWNER TO RoleSpec { AlterOwnerStmt *n = makeNode(AlterOwnerStmt); n->objectType = OBJECT_AGGREGATE; - n->object = $3->funcname; - n->objarg = $3->funcargs; + n->object = list_make1($3); n->newowner = $6; $$ = (Node *)n; } @@ -8418,8 +8411,7 @@ AlterOwnerStmt: ALTER AGGREGATE aggregate_with_argtypes OWNER TO RoleSpec { AlterOwnerStmt *n = makeNode(AlterOwnerStmt); n->objectType = OBJECT_FUNCTION; - n->object = $3->funcname; - n->objarg = $3->funcargs; + n->object = list_make1($3); n->newowner = $6; $$ = (Node *)n; } -- 2.10.2
From 34e8ebae9a45ffa8f30fdee8d0b94c9589c5ca89 Mon Sep 17 00:00:00 2001 From: Peter Eisentraut <pete...@gmx.net> Date: Thu, 15 Sep 2016 12:00:00 -0400 Subject: [PATCH 5/7] Allow dropping multiple functions at once The generic drop support already supported dropping multiple objects of the same kind at once. But the previous representation of function signatures across two grammar symbols and structure members made this cumbersome to do for functions, so it was not supported. Now that function signatures are represented by a single structure, it's trivial to add this support. --- doc/src/sgml/ref/drop_aggregate.sgml | 2 +- doc/src/sgml/ref/drop_function.sgml | 2 +- src/backend/commands/dropcmds.c | 3 +++ src/backend/parser/gram.y | 24 +++++++++++++++--------- src/test/regress/expected/create_function_3.out | 6 ++---- src/test/regress/sql/create_function_3.sql | 2 ++ 6 files changed, 24 insertions(+), 15 deletions(-) diff --git a/doc/src/sgml/ref/drop_aggregate.sgml b/doc/src/sgml/ref/drop_aggregate.sgml index c27c5ea..f239d39 100644 --- a/doc/src/sgml/ref/drop_aggregate.sgml +++ b/doc/src/sgml/ref/drop_aggregate.sgml @@ -21,7 +21,7 @@ <refsynopsisdiv> <synopsis> -DROP AGGREGATE [ IF EXISTS ] <replaceable>name</replaceable> ( <replaceable>aggregate_signature</replaceable> ) [ CASCADE | RESTRICT ] +DROP AGGREGATE [ IF EXISTS ] <replaceable>name</replaceable> ( <replaceable>aggregate_signature</replaceable> ) [, ...] [ CASCADE | RESTRICT ] <phrase>where <replaceable>aggregate_signature</replaceable> is:</phrase> diff --git a/doc/src/sgml/ref/drop_function.sgml b/doc/src/sgml/ref/drop_function.sgml index 5883d13..ad76c95 100644 --- a/doc/src/sgml/ref/drop_function.sgml +++ b/doc/src/sgml/ref/drop_function.sgml @@ -21,7 +21,7 @@ <refsynopsisdiv> <synopsis> -DROP FUNCTION [ IF EXISTS ] <replaceable class="parameter">name</replaceable> ( [ [ <replaceable class="parameter">argmode</replaceable> ] [ <replaceable class="parameter">argname</replaceable> ] <replaceable class="parameter">argtype</replaceable> [, ...] ] ) +DROP FUNCTION [ IF EXISTS ] <replaceable class="parameter">name</replaceable> ( [ [ <replaceable class="parameter">argmode</replaceable> ] [ <replaceable class="parameter">argname</replaceable> ] <replaceable class="parameter">argtype</replaceable> [, ...] ] ) [, ...] [ CASCADE | RESTRICT ] </synopsis> </refsynopsisdiv> diff --git a/src/backend/commands/dropcmds.c b/src/backend/commands/dropcmds.c index ecf1428..32fe2e9 100644 --- a/src/backend/commands/dropcmds.c +++ b/src/backend/commands/dropcmds.c @@ -73,6 +73,9 @@ RemoveObjects(DropStmt *stmt) objargs = lfirst(cell2); } + if (stmt->removeType == OBJECT_AGGREGATE || stmt->removeType == OBJECT_FUNCTION) + objname = list_make1(objname); + /* Get an ObjectAddress for the object. */ address = get_object_address(stmt->removeType, objname, objargs, diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index 27110a5..215e686 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -339,7 +339,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query); %type <list> privileges privilege_list %type <privtarget> privilege_target %type <funwithargs> function_with_argtypes aggregate_with_argtypes -%type <list> function_with_argtypes_list +%type <list> function_with_argtypes_list aggregate_with_argtypes_list %type <ival> defacl_privilege_target %type <defelt> DefACLOption %type <list> DefACLOptionList @@ -7063,6 +7063,12 @@ aggregate_with_argtypes: } ; +aggregate_with_argtypes_list: + aggregate_with_argtypes { $$ = list_make1($1); } + | aggregate_with_argtypes_list ',' aggregate_with_argtypes + { $$ = lappend($1, $3); } + ; + createfunc_opt_list: /* Must be at least one to prevent conflict */ createfunc_opt_item { $$ = list_make1($1); } @@ -7244,22 +7250,22 @@ opt_restrict: *****************************************************************************/ RemoveFuncStmt: - DROP FUNCTION function_with_argtypes opt_drop_behavior + DROP FUNCTION function_with_argtypes_list opt_drop_behavior { DropStmt *n = makeNode(DropStmt); n->removeType = OBJECT_FUNCTION; - n->objects = list_make1(list_make1($3)); + n->objects = $3; n->arguments = NIL; n->behavior = $4; n->missing_ok = false; n->concurrent = false; $$ = (Node *)n; } - | DROP FUNCTION IF_P EXISTS function_with_argtypes opt_drop_behavior + | DROP FUNCTION IF_P EXISTS function_with_argtypes_list opt_drop_behavior { DropStmt *n = makeNode(DropStmt); n->removeType = OBJECT_FUNCTION; - n->objects = list_make1(list_make1($5)); + n->objects = $5; n->arguments = NIL; n->behavior = $6; n->missing_ok = true; @@ -7269,22 +7275,22 @@ RemoveFuncStmt: ; RemoveAggrStmt: - DROP AGGREGATE aggregate_with_argtypes opt_drop_behavior + DROP AGGREGATE aggregate_with_argtypes_list opt_drop_behavior { DropStmt *n = makeNode(DropStmt); n->removeType = OBJECT_AGGREGATE; - n->objects = list_make1(list_make1($3)); + n->objects = $3; n->arguments = NIL; n->behavior = $4; n->missing_ok = false; n->concurrent = false; $$ = (Node *)n; } - | DROP AGGREGATE IF_P EXISTS aggregate_with_argtypes opt_drop_behavior + | DROP AGGREGATE IF_P EXISTS aggregate_with_argtypes_list opt_drop_behavior { DropStmt *n = makeNode(DropStmt); n->removeType = OBJECT_AGGREGATE; - n->objects = list_make1(list_make1($5)); + n->objects = $5; n->arguments = NIL; n->behavior = $6; n->missing_ok = true; diff --git a/src/test/regress/expected/create_function_3.out b/src/test/regress/expected/create_function_3.out index 7bb957b..cc4e98a 100644 --- a/src/test/regress/expected/create_function_3.out +++ b/src/test/regress/expected/create_function_3.out @@ -217,9 +217,10 @@ SELECT routine_name, ordinal_position, parameter_name, parameter_default functest_is_3 | 2 | b | (7 rows) +DROP FUNCTION functest_IS_1(int, int, text), functest_IS_2(int), functest_IS_3(int); -- Cleanups DROP SCHEMA temp_func_test CASCADE; -NOTICE: drop cascades to 19 other objects +NOTICE: drop cascades to 16 other objects DETAIL: drop cascades to function functest_a_1(text,date) drop cascades to function functest_a_2(text[]) drop cascades to function functest_a_3() @@ -236,8 +237,5 @@ drop cascades to function functext_f_1(integer) drop cascades to function functext_f_2(integer) drop cascades to function functext_f_3(integer) drop cascades to function functext_f_4(integer) -drop cascades to function functest_is_1(integer,integer,text) -drop cascades to function functest_is_2(integer) -drop cascades to function functest_is_3(integer) DROP USER regress_unpriv_user; RESET search_path; diff --git a/src/test/regress/sql/create_function_3.sql b/src/test/regress/sql/create_function_3.sql index 43454ef..66a463b 100644 --- a/src/test/regress/sql/create_function_3.sql +++ b/src/test/regress/sql/create_function_3.sql @@ -156,6 +156,8 @@ CREATE FUNCTION functest_IS_3(a int default 1, out b int) WHERE routine_schema = 'temp_func_test' AND routine_name ~ '^functest_is_' ORDER BY 1, 2; +DROP FUNCTION functest_IS_1(int, int, text), functest_IS_2(int), functest_IS_3(int); + -- Cleanups DROP SCHEMA temp_func_test CASCADE; -- 2.10.2
From e51b44b9d13e1a8173d2731e1b1ce1b40c8ff0af Mon Sep 17 00:00:00 2001 From: Peter Eisentraut <pete...@gmx.net> Date: Mon, 19 Sep 2016 12:00:00 -0400 Subject: [PATCH 6/7] Replace LookupFuncNameTypeNames() with LookupFuncWithArgs() The old function took function name and function argument list as separate arguments. Now that all function signatures are passed around as a FuncWithArgs struct, this is no longer necessary and can be replaced by a function that takes FuncWithArgs directly. --- src/backend/catalog/aclchk.c | 3 +-- src/backend/catalog/objectaddress.c | 6 ++---- src/backend/commands/functioncmds.c | 12 ++++-------- src/backend/commands/opclasscmds.c | 28 +++++++++++++--------------- src/backend/nodes/copyfuncs.c | 1 - src/backend/nodes/equalfuncs.c | 1 - src/backend/parser/gram.y | 22 ++++++++++++---------- src/backend/parser/parse_func.c | 32 ++++++++++++++++---------------- src/include/nodes/parsenodes.h | 4 +--- src/include/parser/parse_func.h | 4 ++-- 10 files changed, 51 insertions(+), 62 deletions(-) diff --git a/src/backend/catalog/aclchk.c b/src/backend/catalog/aclchk.c index c0df671..16449cf 100644 --- a/src/backend/catalog/aclchk.c +++ b/src/backend/catalog/aclchk.c @@ -661,8 +661,7 @@ objectNamesToOids(GrantObjectType objtype, List *objnames) FuncWithArgs *func = (FuncWithArgs *) lfirst(cell); Oid funcid; - funcid = LookupFuncNameTypeNames(func->funcname, - func->funcargs, false); + funcid = LookupFuncWithArgs(func, false); objects = lappend_oid(objects, funcid); } break; diff --git a/src/backend/catalog/objectaddress.c b/src/backend/catalog/objectaddress.c index 905a488..a682460 100644 --- a/src/backend/catalog/objectaddress.c +++ b/src/backend/catalog/objectaddress.c @@ -823,8 +823,7 @@ get_object_address(ObjectType objtype, List *objname, List *objargs, { FuncWithArgs *fwa = (FuncWithArgs *) linitial(objname); address.classId = ProcedureRelationId; - address.objectId = - LookupAggNameTypeNames(fwa->funcname, fwa->funcargs, missing_ok); + address.objectId = LookupAggWithArgs(fwa, missing_ok); address.objectSubId = 0; break; } @@ -832,8 +831,7 @@ get_object_address(ObjectType objtype, List *objname, List *objargs, { FuncWithArgs *fwa = (FuncWithArgs *) linitial(objname); address.classId = ProcedureRelationId; - address.objectId = - LookupFuncNameTypeNames(fwa->funcname, fwa->funcargs, missing_ok); + address.objectId = LookupFuncWithArgs(fwa, missing_ok); address.objectSubId = 0; break; } diff --git a/src/backend/commands/functioncmds.c b/src/backend/commands/functioncmds.c index becafc3..94aa348 100644 --- a/src/backend/commands/functioncmds.c +++ b/src/backend/commands/functioncmds.c @@ -1184,9 +1184,7 @@ AlterFunction(ParseState *pstate, AlterFunctionStmt *stmt) rel = heap_open(ProcedureRelationId, RowExclusiveLock); - funcOid = LookupFuncNameTypeNames(stmt->func->funcname, - stmt->func->funcargs, - false); + funcOid = LookupFuncWithArgs(stmt->func, false); tup = SearchSysCacheCopy1(PROCOID, ObjectIdGetDatum(funcOid)); if (!HeapTupleIsValid(tup)) /* should not happen */ @@ -1461,9 +1459,7 @@ CreateCast(CreateCastStmt *stmt) { Form_pg_proc procstruct; - funcid = LookupFuncNameTypeNames(stmt->func->funcname, - stmt->func->funcargs, - false); + funcid = LookupFuncWithArgs(stmt->func, false); tuple = SearchSysCache1(PROCOID, ObjectIdGetDatum(funcid)); if (!HeapTupleIsValid(tuple)) @@ -1843,7 +1839,7 @@ CreateTransform(CreateTransformStmt *stmt) */ if (stmt->fromsql) { - fromsqlfuncid = LookupFuncNameTypeNames(stmt->fromsql->funcname, stmt->fromsql->funcargs, false); + fromsqlfuncid = LookupFuncWithArgs(stmt->fromsql, false); if (!pg_proc_ownercheck(fromsqlfuncid, GetUserId())) aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_PROC, NameListToString(stmt->fromsql->funcname)); @@ -1868,7 +1864,7 @@ CreateTransform(CreateTransformStmt *stmt) if (stmt->tosql) { - tosqlfuncid = LookupFuncNameTypeNames(stmt->tosql->funcname, stmt->tosql->funcargs, false); + tosqlfuncid = LookupFuncWithArgs(stmt->tosql, false); if (!pg_proc_ownercheck(tosqlfuncid, GetUserId())) aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_PROC, NameListToString(stmt->tosql->funcname)); diff --git a/src/backend/commands/opclasscmds.c b/src/backend/commands/opclasscmds.c index f4dfdb9..db45f79 100644 --- a/src/backend/commands/opclasscmds.c +++ b/src/backend/commands/opclasscmds.c @@ -478,19 +478,19 @@ DefineOpClass(CreateOpClassStmt *stmt) errmsg("invalid operator number %d," " must be between 1 and %d", item->number, maxOpNumber))); - if (item->args != NIL) + if (item->name->funcargs != NIL) { - TypeName *typeName1 = (TypeName *) linitial(item->args); - TypeName *typeName2 = (TypeName *) lsecond(item->args); + TypeName *typeName1 = (TypeName *) linitial(item->name->funcargs); + TypeName *typeName2 = (TypeName *) lsecond(item->name->funcargs); - operOid = LookupOperNameTypeNames(NULL, item->name, + operOid = LookupOperNameTypeNames(NULL, item->name->funcname, typeName1, typeName2, false, -1); } else { /* Default to binary op on input datatype */ - operOid = LookupOperName(NULL, item->name, + operOid = LookupOperName(NULL, item->name->funcname, typeoid, typeoid, false, -1); } @@ -529,8 +529,7 @@ DefineOpClass(CreateOpClassStmt *stmt) errmsg("invalid procedure number %d," " must be between 1 and %d", item->number, maxProcNumber))); - funcOid = LookupFuncNameTypeNames(item->name, item->args, - false); + funcOid = LookupFuncWithArgs(item->name, false); #ifdef NOT_USED /* XXX this is unnecessary given the superuser check above */ /* Caller must own function */ @@ -863,12 +862,12 @@ AlterOpFamilyAdd(AlterOpFamilyStmt *stmt, Oid amoid, Oid opfamilyoid, errmsg("invalid operator number %d," " must be between 1 and %d", item->number, maxOpNumber))); - if (item->args != NIL) + if (item->name->funcargs != NIL) { - TypeName *typeName1 = (TypeName *) linitial(item->args); - TypeName *typeName2 = (TypeName *) lsecond(item->args); + TypeName *typeName1 = (TypeName *) linitial(item->name->funcargs); + TypeName *typeName2 = (TypeName *) lsecond(item->name->funcargs); - operOid = LookupOperNameTypeNames(NULL, item->name, + operOid = LookupOperNameTypeNames(NULL, item->name->funcname, typeName1, typeName2, false, -1); } @@ -914,8 +913,7 @@ AlterOpFamilyAdd(AlterOpFamilyStmt *stmt, Oid amoid, Oid opfamilyoid, errmsg("invalid procedure number %d," " must be between 1 and %d", item->number, maxProcNumber))); - funcOid = LookupFuncNameTypeNames(item->name, item->args, - false); + funcOid = LookupFuncWithArgs(item->name, false); #ifdef NOT_USED /* XXX this is unnecessary given the superuser check above */ /* Caller must own function */ @@ -996,7 +994,7 @@ AlterOpFamilyDrop(AlterOpFamilyStmt *stmt, Oid amoid, Oid opfamilyoid, errmsg("invalid operator number %d," " must be between 1 and %d", item->number, maxOpNumber))); - processTypesSpec(item->args, &lefttype, &righttype); + processTypesSpec(item->class_args, &lefttype, &righttype); /* Save the info */ member = (OpFamilyMember *) palloc0(sizeof(OpFamilyMember)); member->number = item->number; @@ -1011,7 +1009,7 @@ AlterOpFamilyDrop(AlterOpFamilyStmt *stmt, Oid amoid, Oid opfamilyoid, errmsg("invalid procedure number %d," " must be between 1 and %d", item->number, maxProcNumber))); - processTypesSpec(item->args, &lefttype, &righttype); + processTypesSpec(item->class_args, &lefttype, &righttype); /* Save the info */ member = (OpFamilyMember *) palloc0(sizeof(OpFamilyMember)); member->number = item->number; diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c index 71714bc..0da30b1 100644 --- a/src/backend/nodes/copyfuncs.c +++ b/src/backend/nodes/copyfuncs.c @@ -3445,7 +3445,6 @@ _copyCreateOpClassItem(const CreateOpClassItem *from) COPY_SCALAR_FIELD(itemtype); COPY_NODE_FIELD(name); - COPY_NODE_FIELD(args); COPY_SCALAR_FIELD(number); COPY_NODE_FIELD(order_family); COPY_NODE_FIELD(class_args); diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c index 29a090f..ecff265 100644 --- a/src/backend/nodes/equalfuncs.c +++ b/src/backend/nodes/equalfuncs.c @@ -1525,7 +1525,6 @@ _equalCreateOpClassItem(const CreateOpClassItem *a, const CreateOpClassItem *b) { COMPARE_SCALAR_FIELD(itemtype); COMPARE_NODE_FIELD(name); - COMPARE_NODE_FIELD(args); COMPARE_SCALAR_FIELD(number); COMPARE_NODE_FIELD(order_family); COMPARE_NODE_FIELD(class_args); diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index 215e686..3681208 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -5346,9 +5346,11 @@ opclass_item: OPERATOR Iconst any_operator opclass_purpose opt_recheck { CreateOpClassItem *n = makeNode(CreateOpClassItem); + FuncWithArgs *fwa = makeNode(FuncWithArgs); + fwa->funcname = $3; + fwa->funcargs = NIL; n->itemtype = OPCLASS_ITEM_OPERATOR; - n->name = $3; - n->args = NIL; + n->name = fwa; n->number = $2; n->order_family = $4; $$ = (Node *) n; @@ -5357,9 +5359,11 @@ opclass_item: opt_recheck { CreateOpClassItem *n = makeNode(CreateOpClassItem); + FuncWithArgs *fwa = makeNode(FuncWithArgs); + fwa->funcname = $3; + fwa->funcargs = $4; n->itemtype = OPCLASS_ITEM_OPERATOR; - n->name = $3; - n->args = $4; + n->name = fwa; n->number = $2; n->order_family = $5; $$ = (Node *) n; @@ -5368,8 +5372,7 @@ opclass_item: { CreateOpClassItem *n = makeNode(CreateOpClassItem); n->itemtype = OPCLASS_ITEM_FUNCTION; - n->name = $3->funcname; - n->args = $3->funcargs; + n->name = $3; n->number = $2; $$ = (Node *) n; } @@ -5377,8 +5380,7 @@ opclass_item: { CreateOpClassItem *n = makeNode(CreateOpClassItem); n->itemtype = OPCLASS_ITEM_FUNCTION; - n->name = $6->funcname; - n->args = $6->funcargs; + n->name = $6; n->number = $2; n->class_args = $4; $$ = (Node *) n; @@ -5465,7 +5467,7 @@ opclass_drop: CreateOpClassItem *n = makeNode(CreateOpClassItem); n->itemtype = OPCLASS_ITEM_OPERATOR; n->number = $2; - n->args = $4; + n->class_args = $4; $$ = (Node *) n; } | FUNCTION Iconst '(' type_list ')' @@ -5473,7 +5475,7 @@ opclass_drop: CreateOpClassItem *n = makeNode(CreateOpClassItem); n->itemtype = OPCLASS_ITEM_FUNCTION; n->number = $2; - n->args = $4; + n->class_args = $4; $$ = (Node *) n; } ; diff --git a/src/backend/parser/parse_func.c b/src/backend/parser/parse_func.c index 56c9a42..b3938e4 100644 --- a/src/backend/parser/parse_func.c +++ b/src/backend/parser/parse_func.c @@ -1933,19 +1933,19 @@ LookupFuncName(List *funcname, int nargs, const Oid *argtypes, bool noError) } /* - * LookupFuncNameTypeNames + * LookupFuncWithArgs * Like LookupFuncName, but the argument types are specified by a - * list of TypeName nodes. + * FuncWithArgs node. */ Oid -LookupFuncNameTypeNames(List *funcname, List *argtypes, bool noError) +LookupFuncWithArgs(FuncWithArgs *func, bool noError) { Oid argoids[FUNC_MAX_ARGS]; int argcount; int i; ListCell *args_item; - argcount = list_length(argtypes); + argcount = list_length(func->funcargs); if (argcount > FUNC_MAX_ARGS) ereport(ERROR, (errcode(ERRCODE_TOO_MANY_ARGUMENTS), @@ -1954,7 +1954,7 @@ LookupFuncNameTypeNames(List *funcname, List *argtypes, bool noError) FUNC_MAX_ARGS, FUNC_MAX_ARGS))); - args_item = list_head(argtypes); + args_item = list_head(func->funcargs); for (i = 0; i < argcount; i++) { TypeName *t = (TypeName *) lfirst(args_item); @@ -1963,19 +1963,19 @@ LookupFuncNameTypeNames(List *funcname, List *argtypes, bool noError) args_item = lnext(args_item); } - return LookupFuncName(funcname, argcount, argoids, noError); + return LookupFuncName(func->funcname, argcount, argoids, noError); } /* - * LookupAggNameTypeNames - * Find an aggregate function given a name and list of TypeName nodes. + * LookupAggWithArgs + * Find an aggregate function from a given FuncWithArgs node. * - * This is almost like LookupFuncNameTypeNames, but the error messages refer + * This is almost like LookupFuncWithArgs, but the error messages refer * to aggregates rather than plain functions, and we verify that the found * function really is an aggregate. */ Oid -LookupAggNameTypeNames(List *aggname, List *argtypes, bool noError) +LookupAggWithArgs(FuncWithArgs *agg, bool noError) { Oid argoids[FUNC_MAX_ARGS]; int argcount; @@ -1985,7 +1985,7 @@ LookupAggNameTypeNames(List *aggname, List *argtypes, bool noError) HeapTuple ftup; Form_pg_proc pform; - argcount = list_length(argtypes); + argcount = list_length(agg->funcargs); if (argcount > FUNC_MAX_ARGS) ereport(ERROR, (errcode(ERRCODE_TOO_MANY_ARGUMENTS), @@ -1995,7 +1995,7 @@ LookupAggNameTypeNames(List *aggname, List *argtypes, bool noError) FUNC_MAX_ARGS))); i = 0; - foreach(lc, argtypes) + foreach(lc, agg->funcargs) { TypeName *t = (TypeName *) lfirst(lc); @@ -2003,7 +2003,7 @@ LookupAggNameTypeNames(List *aggname, List *argtypes, bool noError) i++; } - oid = LookupFuncName(aggname, argcount, argoids, true); + oid = LookupFuncName(agg->funcname, argcount, argoids, true); if (!OidIsValid(oid)) { @@ -2013,12 +2013,12 @@ LookupAggNameTypeNames(List *aggname, List *argtypes, bool noError) ereport(ERROR, (errcode(ERRCODE_UNDEFINED_FUNCTION), errmsg("aggregate %s(*) does not exist", - NameListToString(aggname)))); + NameListToString(agg->funcname)))); else ereport(ERROR, (errcode(ERRCODE_UNDEFINED_FUNCTION), errmsg("aggregate %s does not exist", - func_signature_string(aggname, argcount, + func_signature_string(agg->funcname, argcount, NIL, argoids)))); } @@ -2037,7 +2037,7 @@ LookupAggNameTypeNames(List *aggname, List *argtypes, bool noError) ereport(ERROR, (errcode(ERRCODE_WRONG_OBJECT_TYPE), errmsg("function %s is not an aggregate", - func_signature_string(aggname, argcount, + func_signature_string(agg->funcname, argcount, NIL, argoids)))); } diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h index 6de2cab..564ed50 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -2271,9 +2271,7 @@ typedef struct CreateOpClassItem { NodeTag type; int itemtype; /* see codes above */ - /* fields used for an operator or function item: */ - List *name; /* operator or function name */ - List *args; /* argument types */ + FuncWithArgs *name; /* operator or function name and args */ int number; /* strategy num or support proc num */ List *order_family; /* only used for ordering operators */ List *class_args; /* only used for functions */ diff --git a/src/include/parser/parse_func.h b/src/include/parser/parse_func.h index ed16d36..b1a5494 100644 --- a/src/include/parser/parse_func.h +++ b/src/include/parser/parse_func.h @@ -62,9 +62,9 @@ extern const char *func_signature_string(List *funcname, int nargs, extern Oid LookupFuncName(List *funcname, int nargs, const Oid *argtypes, bool noError); -extern Oid LookupFuncNameTypeNames(List *funcname, List *argtypes, +extern Oid LookupFuncWithArgs(FuncWithArgs *func, bool noError); -extern Oid LookupAggNameTypeNames(List *aggname, List *argtypes, +extern Oid LookupAggWithArgs(FuncWithArgs *agg, bool noError); extern void check_srf_call_placement(ParseState *pstate, int location); -- 2.10.2
From ca25c97a62d8ae0722e17b4b5d975c7adc885601 Mon Sep 17 00:00:00 2001 From: Peter Eisentraut <pete...@gmx.net> Date: Mon, 19 Sep 2016 12:00:00 -0400 Subject: [PATCH 7/7] Pass operators around with FuncWithArgs as well This unifies the operator handling with the previous changes for functions and aggregates. --- doc/src/sgml/ref/drop_operator.sgml | 2 +- src/backend/catalog/objectaddress.c | 38 +++++++++--------- src/backend/commands/dropcmds.c | 13 ++++--- src/backend/commands/opclasscmds.c | 18 +-------- src/backend/commands/operatorcmds.c | 5 +-- src/backend/nodes/copyfuncs.c | 1 - src/backend/nodes/equalfuncs.c | 1 - src/backend/parser/gram.y | 77 +++++++++++++++++++++---------------- src/backend/parser/parse_oper.c | 25 ++++++------ src/include/nodes/parsenodes.h | 3 +- src/include/parser/parse_oper.h | 5 +-- 11 files changed, 90 insertions(+), 98 deletions(-) diff --git a/doc/src/sgml/ref/drop_operator.sgml b/doc/src/sgml/ref/drop_operator.sgml index 13dd974..e196547 100644 --- a/doc/src/sgml/ref/drop_operator.sgml +++ b/doc/src/sgml/ref/drop_operator.sgml @@ -21,7 +21,7 @@ <refsynopsisdiv> <synopsis> -DROP OPERATOR [ IF EXISTS ] <replaceable class="PARAMETER">name</replaceable> ( { <replaceable class="PARAMETER">left_type</replaceable> | NONE } , { <replaceable class="PARAMETER">right_type</replaceable> | NONE } ) [ CASCADE | RESTRICT ] +DROP OPERATOR [ IF EXISTS ] <replaceable class="PARAMETER">name</replaceable> ( { <replaceable class="PARAMETER">left_type</replaceable> | NONE } , { <replaceable class="PARAMETER">right_type</replaceable> | NONE } ) [, ...] [ CASCADE | RESTRICT ] </synopsis> </refsynopsisdiv> diff --git a/src/backend/catalog/objectaddress.c b/src/backend/catalog/objectaddress.c index a682460..c5d2e60 100644 --- a/src/backend/catalog/objectaddress.c +++ b/src/backend/catalog/objectaddress.c @@ -836,15 +836,13 @@ get_object_address(ObjectType objtype, List *objname, List *objargs, break; } case OBJECT_OPERATOR: - Assert(list_length(objargs) == 2); - address.classId = OperatorRelationId; - address.objectId = - LookupOperNameTypeNames(NULL, objname, - (TypeName *) linitial(objargs), - (TypeName *) lsecond(objargs), - missing_ok, -1); - address.objectSubId = 0; - break; + { + FuncWithArgs *fwa = (FuncWithArgs *) linitial(objname); + address.classId = OperatorRelationId; + address.objectId = LookupOperWithArgs(fwa, missing_ok); + address.objectSubId = 0; + break; + } case OBJECT_COLLATION: address.classId = CollationRelationId; address.objectId = get_collation_oid(objname, missing_ok); @@ -1996,16 +1994,6 @@ pg_get_object_address(PG_FUNCTION_ARGS) args = textarray_to_strvaluelist(argsarr); } - if (type == OBJECT_FUNCTION || type == OBJECT_AGGREGATE) - { - FuncWithArgs *fwa = makeNode(FuncWithArgs); - - fwa->funcname = name; - fwa->funcargs = args; - name = list_make1(fwa); - args = NIL; - } - /* * get_object_name is pretty sensitive to the length its input lists; * check that they're what it wants. @@ -2046,6 +2034,16 @@ pg_get_object_address(PG_FUNCTION_ARGS) break; } + if (type == OBJECT_FUNCTION || type == OBJECT_AGGREGATE || type == OBJECT_OPERATOR) + { + FuncWithArgs *fwa = makeNode(FuncWithArgs); + + fwa->funcname = name; + fwa->funcargs = args; + name = list_make1(fwa); + args = NIL; + } + addr = get_object_address(type, name, args, &relation, AccessShareLock, false); @@ -2119,7 +2117,7 @@ check_object_ownership(Oid roleid, ObjectType objtype, ObjectAddress address, case OBJECT_OPERATOR: if (!pg_oper_ownercheck(address.objectId, roleid)) aclcheck_error(ACLCHECK_NOT_OWNER, ACL_KIND_OPER, - NameListToString(objname)); + NameListToString(((FuncWithArgs *) linitial(objname))->funcname)); break; case OBJECT_SCHEMA: if (!pg_namespace_ownercheck(address.objectId, roleid)) diff --git a/src/backend/commands/dropcmds.c b/src/backend/commands/dropcmds.c index 32fe2e9..6c9786e 100644 --- a/src/backend/commands/dropcmds.c +++ b/src/backend/commands/dropcmds.c @@ -73,7 +73,7 @@ RemoveObjects(DropStmt *stmt) objargs = lfirst(cell2); } - if (stmt->removeType == OBJECT_AGGREGATE || stmt->removeType == OBJECT_FUNCTION) + if (stmt->removeType == OBJECT_AGGREGATE || stmt->removeType == OBJECT_FUNCTION || stmt->removeType == OBJECT_OPERATOR) objname = list_make1(objname); /* Get an ObjectAddress for the object. */ @@ -356,11 +356,14 @@ does_not_exist_skipping(ObjectType objtype, List *objname, List *objargs) } break; case OBJECT_OPERATOR: - if (!schema_does_not_exist_skipping(objname, &msg, &name) && - !type_in_list_does_not_exist_skipping(objargs, &msg, &name)) { - msg = gettext_noop("operator %s does not exist, skipping"); - name = NameListToString(objname); + FuncWithArgs *fwa = (FuncWithArgs *) linitial(objname); + if (!schema_does_not_exist_skipping(fwa->funcname, &msg, &name) && + !type_in_list_does_not_exist_skipping(fwa->funcargs, &msg, &name)) + { + msg = gettext_noop("operator %s does not exist, skipping"); + name = NameListToString(fwa->funcname); + } } break; case OBJECT_LANGUAGE: diff --git a/src/backend/commands/opclasscmds.c b/src/backend/commands/opclasscmds.c index db45f79..cfda571 100644 --- a/src/backend/commands/opclasscmds.c +++ b/src/backend/commands/opclasscmds.c @@ -479,14 +479,7 @@ DefineOpClass(CreateOpClassStmt *stmt) " must be between 1 and %d", item->number, maxOpNumber))); if (item->name->funcargs != NIL) - { - TypeName *typeName1 = (TypeName *) linitial(item->name->funcargs); - TypeName *typeName2 = (TypeName *) lsecond(item->name->funcargs); - - operOid = LookupOperNameTypeNames(NULL, item->name->funcname, - typeName1, typeName2, - false, -1); - } + operOid = LookupOperWithArgs(item->name, false); else { /* Default to binary op on input datatype */ @@ -863,14 +856,7 @@ AlterOpFamilyAdd(AlterOpFamilyStmt *stmt, Oid amoid, Oid opfamilyoid, " must be between 1 and %d", item->number, maxOpNumber))); if (item->name->funcargs != NIL) - { - TypeName *typeName1 = (TypeName *) linitial(item->name->funcargs); - TypeName *typeName2 = (TypeName *) lsecond(item->name->funcargs); - - operOid = LookupOperNameTypeNames(NULL, item->name->funcname, - typeName1, typeName2, - false, -1); - } + operOid = LookupOperWithArgs(item->name, false); else { ereport(ERROR, diff --git a/src/backend/commands/operatorcmds.c b/src/backend/commands/operatorcmds.c index 67d08d8..35b18b0 100644 --- a/src/backend/commands/operatorcmds.c +++ b/src/backend/commands/operatorcmds.c @@ -402,10 +402,7 @@ AlterOperator(AlterOperatorStmt *stmt) Oid joinOid; /* Look up the operator */ - oprId = LookupOperNameTypeNames(NULL, stmt->opername, - (TypeName *) linitial(stmt->operargs), - (TypeName *) lsecond(stmt->operargs), - false, -1); + oprId = LookupOperWithArgs(stmt->opername, false); catalog = heap_open(OperatorRelationId, RowExclusiveLock); tup = SearchSysCacheCopy1(OPEROID, ObjectIdGetDatum(oprId)); if (tup == NULL) diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c index 0da30b1..e794375 100644 --- a/src/backend/nodes/copyfuncs.c +++ b/src/backend/nodes/copyfuncs.c @@ -3272,7 +3272,6 @@ _copyAlterOperatorStmt(const AlterOperatorStmt *from) AlterOperatorStmt *newnode = makeNode(AlterOperatorStmt); COPY_NODE_FIELD(opername); - COPY_NODE_FIELD(operargs); COPY_NODE_FIELD(options); return newnode; diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c index ecff265..6ab3b84 100644 --- a/src/backend/nodes/equalfuncs.c +++ b/src/backend/nodes/equalfuncs.c @@ -1380,7 +1380,6 @@ static bool _equalAlterOperatorStmt(const AlterOperatorStmt *a, const AlterOperatorStmt *b) { COMPARE_NODE_FIELD(opername); - COMPARE_NODE_FIELD(operargs); COMPARE_NODE_FIELD(options); return true; diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index 3681208..07086c6 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -338,8 +338,8 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query); %type <accesspriv> privilege %type <list> privileges privilege_list %type <privtarget> privilege_target -%type <funwithargs> function_with_argtypes aggregate_with_argtypes -%type <list> function_with_argtypes_list aggregate_with_argtypes_list +%type <funwithargs> function_with_argtypes aggregate_with_argtypes operator_with_argtypes +%type <list> function_with_argtypes_list aggregate_with_argtypes_list operator_with_argtypes_list %type <ival> defacl_privilege_target %type <defelt> DefACLOption %type <list> DefACLOptionList @@ -4004,14 +4004,13 @@ AlterExtensionContentsStmt: n->objname = list_make1(makeString($7)); $$ = (Node *)n; } - | ALTER EXTENSION name add_drop OPERATOR any_operator oper_argtypes + | ALTER EXTENSION name add_drop OPERATOR operator_with_argtypes { AlterExtensionContentsStmt *n = makeNode(AlterExtensionContentsStmt); n->extname = $3; n->action = $4; n->objtype = OBJECT_OPERATOR; - n->objname = $6; - n->objargs = $7; + n->objname = list_make1($6); $$ = (Node *)n; } | ALTER EXTENSION name add_drop OPERATOR CLASS any_name USING access_method @@ -5355,17 +5354,14 @@ opclass_item: n->order_family = $4; $$ = (Node *) n; } - | OPERATOR Iconst any_operator oper_argtypes opclass_purpose + | OPERATOR Iconst operator_with_argtypes opclass_purpose opt_recheck { CreateOpClassItem *n = makeNode(CreateOpClassItem); - FuncWithArgs *fwa = makeNode(FuncWithArgs); - fwa->funcname = $3; - fwa->funcargs = $4; n->itemtype = OPCLASS_ITEM_OPERATOR; - n->name = fwa; + n->name = $3; n->number = $2; - n->order_family = $5; + n->order_family = $4; $$ = (Node *) n; } | FUNCTION Iconst function_with_argtypes @@ -5789,13 +5785,13 @@ CommentStmt: n->comment = $6; $$ = (Node *) n; } - | COMMENT ON OPERATOR any_operator oper_argtypes IS comment_text + | COMMENT ON OPERATOR operator_with_argtypes IS comment_text { CommentStmt *n = makeNode(CommentStmt); n->objtype = OBJECT_OPERATOR; - n->objname = $4; - n->objargs = $5; - n->comment = $7; + n->objname = list_make1($4); + n->objargs = NIL; + n->comment = $6; $$ = (Node *) n; } | COMMENT ON CONSTRAINT name ON any_name IS comment_text @@ -7302,24 +7298,24 @@ RemoveAggrStmt: ; RemoveOperStmt: - DROP OPERATOR any_operator oper_argtypes opt_drop_behavior + DROP OPERATOR operator_with_argtypes_list opt_drop_behavior { DropStmt *n = makeNode(DropStmt); n->removeType = OBJECT_OPERATOR; - n->objects = list_make1($3); - n->arguments = list_make1($4); - n->behavior = $5; + n->objects = $3; + n->arguments = NIL; + n->behavior = $4; n->missing_ok = false; n->concurrent = false; $$ = (Node *)n; } - | DROP OPERATOR IF_P EXISTS any_operator oper_argtypes opt_drop_behavior + | DROP OPERATOR IF_P EXISTS operator_with_argtypes_list opt_drop_behavior { DropStmt *n = makeNode(DropStmt); n->removeType = OBJECT_OPERATOR; - n->objects = list_make1($5); - n->arguments = list_make1($6); - n->behavior = $7; + n->objects = $5; + n->arguments = NIL; + n->behavior = $6; n->missing_ok = true; n->concurrent = false; $$ = (Node *)n; @@ -7350,6 +7346,22 @@ any_operator: { $$ = lcons(makeString($1), $3); } ; +operator_with_argtypes_list: + operator_with_argtypes { $$ = list_make1($1); } + | operator_with_argtypes_list ',' operator_with_argtypes + { $$ = lappend($1, $3); } + ; + +operator_with_argtypes: + any_operator oper_argtypes + { + FuncWithArgs *n = makeNode(FuncWithArgs); + n->funcname = $1; + n->funcargs = $2; + $$ = n; + } + ; + /***************************************************************************** * * DO <anonymous code block> [ LANGUAGE language ] @@ -8177,13 +8189,12 @@ AlterObjectSchemaStmt: n->missing_ok = false; $$ = (Node *)n; } - | ALTER OPERATOR any_operator oper_argtypes SET SCHEMA name + | ALTER OPERATOR operator_with_argtypes SET SCHEMA name { AlterObjectSchemaStmt *n = makeNode(AlterObjectSchemaStmt); n->objectType = OBJECT_OPERATOR; - n->object = $3; - n->objarg = $4; - n->newschema = $7; + n->object = list_make1($3); + n->newschema = $6; n->missing_ok = false; $$ = (Node *)n; } @@ -8349,12 +8360,11 @@ AlterObjectSchemaStmt: *****************************************************************************/ AlterOperatorStmt: - ALTER OPERATOR any_operator oper_argtypes SET '(' operator_def_list ')' + ALTER OPERATOR operator_with_argtypes SET '(' operator_def_list ')' { AlterOperatorStmt *n = makeNode(AlterOperatorStmt); n->opername = $3; - n->operargs = $4; - n->options = $7; + n->options = $6; $$ = (Node *)n; } ; @@ -8439,13 +8449,12 @@ AlterOwnerStmt: ALTER AGGREGATE aggregate_with_argtypes OWNER TO RoleSpec n->newowner = $7; $$ = (Node *)n; } - | ALTER OPERATOR any_operator oper_argtypes OWNER TO RoleSpec + | ALTER OPERATOR operator_with_argtypes OWNER TO RoleSpec { AlterOwnerStmt *n = makeNode(AlterOwnerStmt); n->objectType = OBJECT_OPERATOR; - n->object = $3; - n->objarg = $4; - n->newowner = $7; + n->object = list_make1($3); + n->newowner = $6; $$ = (Node *)n; } | ALTER OPERATOR CLASS any_name USING access_method OWNER TO RoleSpec diff --git a/src/backend/parser/parse_oper.c b/src/backend/parser/parse_oper.c index aecda6d..006d978 100644 --- a/src/backend/parser/parse_oper.c +++ b/src/backend/parser/parse_oper.c @@ -132,32 +132,35 @@ LookupOperName(ParseState *pstate, List *opername, Oid oprleft, Oid oprright, } /* - * LookupOperNameTypeNames + * LookupOperWithArgs * Like LookupOperName, but the argument types are specified by - * TypeName nodes. - * - * Pass oprleft = NULL for a prefix op, oprright = NULL for a postfix op. + * a FuncWithArg node. */ Oid -LookupOperNameTypeNames(ParseState *pstate, List *opername, - TypeName *oprleft, TypeName *oprright, - bool noError, int location) +LookupOperWithArgs(FuncWithArgs *oper, + bool noError) { + TypeName *oprleft, + *oprright; Oid leftoid, rightoid; + Assert(list_length(oper->funcargs) == 2); + oprleft = linitial(oper->funcargs); + oprright = lsecond(oper->funcargs); + if (oprleft == NULL) leftoid = InvalidOid; else - leftoid = LookupTypeNameOid(pstate, oprleft, noError); + leftoid = LookupTypeNameOid(NULL, oprleft, noError); if (oprright == NULL) rightoid = InvalidOid; else - rightoid = LookupTypeNameOid(pstate, oprright, noError); + rightoid = LookupTypeNameOid(NULL, oprright, noError); - return LookupOperName(pstate, opername, leftoid, rightoid, - noError, location); + return LookupOperName(NULL, oper->funcname, leftoid, rightoid, + noError, -1); } /* diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h index 564ed50..8a1f530 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -2587,8 +2587,7 @@ typedef struct AlterOwnerStmt typedef struct AlterOperatorStmt { NodeTag type; - List *opername; /* operator name */ - List *operargs; /* operator's argument TypeNames */ + FuncWithArgs *opername; /* operator name and argument types */ List *options; /* List of DefElem nodes */ } AlterOperatorStmt; diff --git a/src/include/parser/parse_oper.h b/src/include/parser/parse_oper.h index 03bb5b9..51c1247 100644 --- a/src/include/parser/parse_oper.h +++ b/src/include/parser/parse_oper.h @@ -15,6 +15,7 @@ #define PARSE_OPER_H #include "access/htup.h" +#include "nodes/parsenodes.h" #include "parser/parse_node.h" @@ -24,9 +25,7 @@ typedef HeapTuple Operator; extern Oid LookupOperName(ParseState *pstate, List *opername, Oid oprleft, Oid oprright, bool noError, int location); -extern Oid LookupOperNameTypeNames(ParseState *pstate, List *opername, - TypeName *oprleft, TypeName *oprright, - bool noError, int location); +extern Oid LookupOperWithArgs(FuncWithArgs *oper, bool noError); /* Routines to find operators matching a name and given input types */ /* NB: the selected operator may require coercion of the input types! */ -- 2.10.2
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers