From 517ae4039ebe12806d76adfd3bc1fc6578fc8862 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Thu, 18 Dec 2008 18:20:35 +0000 Subject: [PATCH] Code review for function default parameters patch. Fix numerous problems as per recent discussions. In passing this also fixes a couple of bugs in the previous variadic-parameters patch. --- doc/src/sgml/catalogs.sgml | 25 +- doc/src/sgml/func.sgml | 9 +- doc/src/sgml/ref/create_function.sgml | 32 ++- doc/src/sgml/typeconv.sgml | 17 +- doc/src/sgml/xfunc.sgml | 33 ++- src/backend/catalog/namespace.c | 319 +++++++++++---------- src/backend/catalog/pg_aggregate.c | 8 +- src/backend/catalog/pg_proc.c | 63 +++- src/backend/commands/functioncmds.c | 66 +++-- src/backend/commands/proclang.c | 14 +- src/backend/optimizer/plan/planner.c | 25 +- src/backend/optimizer/util/clauses.c | 108 ++++++- src/backend/parser/gram.y | 23 +- src/backend/parser/parse_func.c | 93 ++++-- src/backend/utils/adt/regproc.c | 8 +- src/backend/utils/adt/ruleutils.c | 98 ++++--- src/bin/pg_dump/pg_dump.c | 13 +- src/include/catalog/namespace.h | 8 +- src/include/catalog/pg_attribute.h | 4 +- src/include/catalog/pg_proc.h | 16 +- src/include/catalog/pg_proc_fn.h | 6 +- src/include/nodes/parsenodes.h | 8 +- src/include/parser/parse_func.h | 5 +- src/test/regress/expected/opr_sanity.out | 8 +- src/test/regress/expected/polymorphism.out | 156 +++++++--- src/test/regress/sql/opr_sanity.sql | 8 +- src/test/regress/sql/polymorphism.sql | 75 +++-- 27 files changed, 803 insertions(+), 445 deletions(-) diff --git a/doc/src/sgml/catalogs.sgml b/doc/src/sgml/catalogs.sgml index 70d879e621..f5f26d035d 100644 --- a/doc/src/sgml/catalogs.sgml +++ b/doc/src/sgml/catalogs.sgml @@ -1,4 +1,4 @@ - + @@ -3653,7 +3653,14 @@ pronargs int2 - Number of arguments + Number of input arguments + + + + pronargdefaults + int2 + + Number of arguments that have defaults @@ -3720,6 +3727,20 @@ + + proargdefaults + text + + + Expression trees (in nodeToString() representation) + for default values. This is a list with + pronargdefaults elements, corresponding to the last + N input arguments (i.e., the last + N proargtypes positions). + If none of the arguments have defaults, this field will be null + + + prosrc text diff --git a/doc/src/sgml/func.sgml b/doc/src/sgml/func.sgml index 23241c2469..c08c480188 100644 --- a/doc/src/sgml/func.sgml +++ b/doc/src/sgml/func.sgml @@ -1,4 +1,4 @@ - + Functions and Operators @@ -11808,7 +11808,7 @@ SELECT pg_type_is_visible('myschema.widget'::regtype); pg_get_function_identity_arguments(func_oid) text - get argument list to identify a function (without argument names, default values) + get argument list to identify a function (without default values) pg_get_function_result(func_oid) @@ -11929,12 +11929,11 @@ SELECT pg_type_is_visible('myschema.widget'::regtype); of a function, in the form it would need to appear in within CREATE FUNCTION. pg_get_function_result similarly returns the - appropriate RETURNS clause for the function. + appropriate RETURNS clause for the function. pg_get_function_identity_arguments returns the argument list necessary to identify a function, in the form it would need to appear in within ALTER FUNCTION, for - instance. This form omits default values and argument names, for - example. + instance. This form omits default values. diff --git a/doc/src/sgml/ref/create_function.sgml b/doc/src/sgml/ref/create_function.sgml index 5de4967789..639647eab3 100644 --- a/doc/src/sgml/ref/create_function.sgml +++ b/doc/src/sgml/ref/create_function.sgml @@ -1,5 +1,5 @@ @@ -21,7 +21,7 @@ $PostgreSQL: pgsql/doc/src/sgml/ref/create_function.sgml,v 1.82 2008/12/04 17:51 CREATE [ OR REPLACE ] FUNCTION - name ( [ [ argmode ] [ argname ] argtype [ { DEFAULT | = } defexpr] [, ...] ] ) + name ( [ [ argmode ] [ argname ] argtype [ { DEFAULT | = } defexpr ] [, ...] ] ) [ RETURNS rettype | RETURNS TABLE ( colname coltype [, ...] ) ] { LANGUAGE langname @@ -37,7 +37,7 @@ CREATE [ OR REPLACE ] FUNCTION [ WITH ( attribute [, ...] ) ] - + Description @@ -133,7 +133,7 @@ CREATE [ OR REPLACE ] FUNCTION - The data type(s) of the function's arguments (optionally + The data type(s) of the function's arguments (optionally schema-qualified), if any. The argument types can be base, composite, or domain types, or can reference the type of a table column. @@ -160,10 +160,11 @@ CREATE [ OR REPLACE ] FUNCTION An expression to be used as default value if the parameter is - not specified. The expression has to be convertable to the - argument type of the parameter. All parameters after a - parameter with default value have to be parameters with default - values as well. + not specified. The expression has to be coercible to the + argument type of the parameter. + Only input (including INOUT) parameters can have a default + value. All input parameters following a + parameter with a default value must have default values as well. @@ -173,7 +174,7 @@ CREATE [ OR REPLACE ] FUNCTION - The return data type (optionally schema-qualified). The return type + The return data type (optionally schema-qualified). The return type can be a base, composite, or domain type, or can reference the type of a table column. Depending on the implementation language it might also be allowed @@ -496,6 +497,18 @@ CREATE FUNCTION foo(int, out text) ... + + Functions that have different argument type lists will not be considered + to conflict at creation time, but if defaults are provided they might + conflict in use. For example, consider + +CREATE FUNCTION foo(int) ... +CREATE FUNCTION foo(int, int default 42) ... + + A call foo(10) will fail due to the ambiguity about which + function should be called. + + When repeated CREATE FUNCTION calls refer to the same object file, the file is only loaded once per session. @@ -664,7 +677,6 @@ COMMIT; - Compatibility diff --git a/doc/src/sgml/typeconv.sgml b/doc/src/sgml/typeconv.sgml index 08b62c4e61..3829275276 100644 --- a/doc/src/sgml/typeconv.sgml +++ b/doc/src/sgml/typeconv.sgml @@ -1,4 +1,4 @@ - + Type Conversion @@ -496,7 +496,20 @@ of its element type, as needed to match the call. After such expansion the function might have effective argument types identical to some non-variadic function. In that case the function appearing earlier in the search path is used, or if the two functions are in the same schema, the non-variadic one is -selected. +preferred. + + + + +Functions that have default values for parameters are considered to match any +call that omits zero or more of the defaultable parameter positions. If more +than one such function matches a call, the one appearing earliest in the +search path is used. If there are two or more such functions in the same +schema with identical parameter types in the non-defaulted positions (which is +possible if they have different sets of defaultable parameters), the system +will not be able to determine which to prefer, and so an ambiguous +function call error will result if no better match to the call can be +found. diff --git a/doc/src/sgml/xfunc.sgml b/doc/src/sgml/xfunc.sgml index 863a17ff52..5a66bab452 100644 --- a/doc/src/sgml/xfunc.sgml +++ b/doc/src/sgml/xfunc.sgml @@ -1,4 +1,4 @@ - + User-Defined Functions @@ -663,25 +663,27 @@ SELECT mleast(VARIADIC ARRAY[10, -1, 5, 4.4]); - - <acronym>SQL</> Functions with Parameters Default Values + + <acronym>SQL</> Functions with Default Values for Arguments - default values + function + default values for arguments - Functions can be declared with parameters with default values or - expressions. The default expressions are used as parameter value - if the parameter is not explicitly specified in a function call. - All parameters after a a parameter with default value have to be - parameters with default values as well. + Functions can be declared with default values for some or all input + arguments. The default values are inserted whenever the function is + called with insufficiently many actual arguments. Since arguments + can only be omitted from the end of the actual argument list, all + parameters after a parameter with a default value have to have + default values as well. For example: -CREATE FUNCTION foo(a int DEFAULT 1, b int DEFAULT 2, c int DEFAULT 3) +CREATE FUNCTION foo(a int, b int DEFAULT 2, c int DEFAULT 3) RETURNS int LANGUAGE SQL AS $$ @@ -706,14 +708,11 @@ SELECT foo(10); 15 (1 row) -SELECT foo(); - foo ------ - 6 -(1 row) +SELECT foo(); -- fails since there is no default for the first argument +ERROR: function foo() does not exist - Instead of the key word DEFAULT, - the = sign can also be used. + The = sign can also be used in place of the + key word DEFAULT, diff --git a/src/backend/catalog/namespace.c b/src/backend/catalog/namespace.c index e665e9594d..892ca66591 100644 --- a/src/backend/catalog/namespace.c +++ b/src/backend/catalog/namespace.c @@ -13,7 +13,7 @@ * Portions Copyright (c) 1994, Regents of the University of California * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/catalog/namespace.c,v 1.114 2008/12/15 18:09:40 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/catalog/namespace.c,v 1.115 2008/12/18 18:20:33 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -562,7 +562,8 @@ TypeIsVisible(Oid typid) * retrieve a list of the possible matches. * * If nargs is -1, we return all functions matching the given name, - * regardless of argument count. (expand_variadic must be false in this case.) + * regardless of argument count. (expand_variadic and expand_defaults must be + * false in this case.) * * If expand_variadic is true, then variadic functions having the same number * or fewer arguments will be retrieved, with the variadic argument and any @@ -571,23 +572,43 @@ TypeIsVisible(Oid typid) * If expand_variadic is false, variadic arguments are not treated specially, * and the returned nvargs will always be zero. * - * If expand_variadic is true, functions with argument default values - * will also be retrieved. If expand_variadic is false, default - * values will not be taken into account and functions that do not - * have exactly nargs arguments in total will not be considered. + * If expand_defaults is true, functions that could match after insertion of + * default argument values will also be retrieved. In this case the returned + * structs could have nargs > passed-in nargs, and ndargs is set to the number + * of additional args (which can be retrieved from the function's + * proargdefaults entry). + * + * It is not possible for nvargs and ndargs to both be nonzero in the same + * list entry, since default insertion allows matches to functions with more + * than nargs arguments while the variadic transformation requires the same + * number or less. * * We search a single namespace if the function name is qualified, else - * all namespaces in the search path. The return list will never contain - * multiple entries with identical argument lists --- in the multiple- - * namespace case, we arrange for entries in earlier namespaces to mask - * identical entries in later namespaces. We also arrange for non-variadic - * functions to mask variadic ones if the expanded argument list is the same. + * all namespaces in the search path. In the multiple-namespace case, + * we arrange for entries in earlier namespaces to mask identical entries in + * later namespaces. + * + * When expanding variadics, we arrange for non-variadic functions to mask + * variadic ones if the expanded argument list is the same. It is still + * possible for there to be conflicts between different variadic functions, + * however. + * + * It is guaranteed that the return list will never contain multiple entries + * with identical argument lists. When expand_defaults is true, the entries + * could have more than nargs positions, but we still guarantee that they are + * distinct in the first nargs positions. However, if either expand_variadic + * or expand_defaults is true, there might be multiple candidate functions + * that expand to identical argument lists. Rather than throw error here, + * we report such situations by setting oid = 0 in the ambiguous entries. + * The caller might end up discarding such an entry anyway, but if it selects + * such an entry it should react as though the call were ambiguous. */ FuncCandidateList -FuncnameGetCandidates(List *names, int nargs, bool expand_variadic) +FuncnameGetCandidates(List *names, int nargs, + bool expand_variadic, bool expand_defaults) { FuncCandidateList resultList = NULL; - bool any_variadic = false; + bool any_special = false; char *schemaname; char *funcname; Oid namespaceId; @@ -595,7 +616,7 @@ FuncnameGetCandidates(List *names, int nargs, bool expand_variadic) int i; /* check for caller error */ - Assert(nargs >= 0 || !expand_variadic); + Assert(nargs >= 0 || !(expand_variadic | expand_defaults)); /* deconstruct the name list */ DeconstructQualifiedName(names, &schemaname, &funcname); @@ -625,41 +646,10 @@ FuncnameGetCandidates(List *names, int nargs, bool expand_variadic) int effective_nargs; int pathpos = 0; bool variadic; + bool use_defaults; Oid va_elem_type; - List *defaults = NIL; FuncCandidateList newResult; - /* - * Check if function has some parameter defaults if some - * parameters are missing. - */ - if (pronargs > nargs && expand_variadic) - { - bool isnull; - Datum proargdefaults; - char *str; - - /* skip when not enough default expressions */ - if (nargs + procform->pronargdefaults < pronargs) - continue; - - proargdefaults = SysCacheGetAttr(PROCOID, proctup, - Anum_pg_proc_proargdefaults, &isnull); - Assert(!isnull); - str = TextDatumGetCString(proargdefaults); - defaults = (List *) stringToNode(str); - - Assert(IsA(defaults, List)); - - /* - * If we don't have to use all default parameters, we skip - * some cells from the left. - */ - defaults = list_copy_tail(defaults, procform->pronargdefaults - pronargs + nargs); - - pfree(str); - } - /* * Check if function is variadic, and get variadic element type if so. * If expand_variadic is false, we should just ignore variadic-ness. @@ -668,6 +658,7 @@ FuncnameGetCandidates(List *names, int nargs, bool expand_variadic) { va_elem_type = procform->provariadic; variadic = OidIsValid(va_elem_type); + any_special |= variadic; } else { @@ -675,16 +666,24 @@ FuncnameGetCandidates(List *names, int nargs, bool expand_variadic) variadic = false; } - Assert(!variadic || !defaults); + /* + * Check if function can match by using parameter defaults. + */ + if (pronargs > nargs && expand_defaults) + { + /* Ignore if not enough default expressions */ + if (nargs + procform->pronargdefaults < pronargs) + continue; + use_defaults = true; + any_special = true; + } + else + use_defaults = false; /* Ignore if it doesn't match requested argument count */ - if (nargs >= 0 && - (variadic ? (pronargs > nargs) : (defaults ? (pronargs < nargs) : (pronargs != nargs)))) + if (nargs >= 0 && pronargs != nargs && !variadic && !use_defaults) continue; - Assert(!variadic || (pronargs <= nargs)); - Assert(!defaults || (pronargs > nargs)); - if (OidIsValid(namespaceId)) { /* Consider only procs in specified namespace */ @@ -723,7 +722,6 @@ FuncnameGetCandidates(List *names, int nargs, bool expand_variadic) newResult->pathpos = pathpos; newResult->oid = HeapTupleGetOid(proctup); newResult->nargs = effective_nargs; - newResult->argdefaults = defaults; memcpy(newResult->args, procform->proargtypes.values, pronargs * sizeof(Oid)); if (variadic) @@ -737,128 +735,140 @@ FuncnameGetCandidates(List *names, int nargs, bool expand_variadic) } else newResult->nvargs = 0; - - any_variadic = variadic || defaults; + newResult->ndargs = use_defaults ? pronargs - nargs : 0; /* * Does it have the same arguments as something we already accepted? - * If so, decide which one to keep. We can skip this check for the - * single-namespace case if no variadic match has been made, since - * then the unique index on pg_proc guarantees all the matches have - * different argument lists. + * If so, decide what to do to avoid returning duplicate argument + * lists. We can skip this check for the single-namespace case if no + * special (variadic or defaults) match has been made, since then the + * unique index on pg_proc guarantees all the matches have different + * argument lists. */ - if (any_variadic || !OidIsValid(namespaceId)) + if (resultList != NULL && + (any_special || !OidIsValid(namespaceId))) { - if (defaults) - effective_nargs = nargs; - /* * If we have an ordered list from SearchSysCacheList (the normal * case), then any conflicting proc must immediately adjoin this * one in the list, so we only need to look at the newest result * item. If we have an unordered list, we have to scan the whole * result list. Also, if either the current candidate or any - * previous candidate is a variadic match, we can't assume that + * previous candidate is a special match, we can't assume that * conflicts are adjacent. + * + * We ignore defaulted arguments in deciding what is a match. */ - if (resultList) - { - FuncCandidateList prevResult; + FuncCandidateList prevResult; - if (catlist->ordered && !any_variadic) + if (catlist->ordered && !any_special) + { + /* ndargs must be 0 if !any_special */ + if (effective_nargs == resultList->nargs && + memcmp(newResult->args, + resultList->args, + effective_nargs * sizeof(Oid)) == 0) + prevResult = resultList; + else + prevResult = NULL; + } + else + { + int cmp_nargs = newResult->nargs - newResult->ndargs; + + for (prevResult = resultList; + prevResult; + prevResult = prevResult->next) { - if (effective_nargs == resultList->nargs && + if (cmp_nargs == prevResult->nargs - prevResult->ndargs && memcmp(newResult->args, - resultList->args, - effective_nargs * sizeof(Oid)) == 0) - prevResult = resultList; - else - prevResult = NULL; + prevResult->args, + cmp_nargs * sizeof(Oid)) == 0) + break; + } + } + + if (prevResult) + { + /* + * We have a match with a previous result. Decide which one + * to keep, or mark it ambiguous if we can't decide. The + * logic here is preference > 0 means prefer the old result, + * preference < 0 means prefer the new, preference = 0 means + * ambiguous. + */ + int preference; + + if (pathpos != prevResult->pathpos) + { + /* + * Prefer the one that's earlier in the search path. + */ + preference = pathpos - prevResult->pathpos; + } + else if (variadic && prevResult->nvargs == 0) + { + /* + * With variadic functions we could have, for example, + * both foo(numeric) and foo(variadic numeric[]) in + * the same namespace; if so we prefer the + * non-variadic match on efficiency grounds. + */ + preference = 1; + } + else if (!variadic && prevResult->nvargs > 0) + { + preference = -1; } else { - for (prevResult = resultList; - prevResult; - prevResult = prevResult->next) - { - if (!defaults) - { - if (effective_nargs == prevResult->nargs && - memcmp(newResult->args, - prevResult->args, - effective_nargs * sizeof(Oid)) == 0) - break; - } - else - { - if (memcmp(newResult->args, - prevResult->args, - effective_nargs * sizeof(Oid)) == 0) - break; - } - } - } - if (prevResult) - { - /* - * We have a match with a previous result. Prefer the - * one that's earlier in the search path. + /*---------- + * We can't decide. This can happen with, for example, + * both foo(numeric, variadic numeric[]) and + * foo(variadic numeric[]) in the same namespace, or + * both foo(int) and foo (int, int default something) + * in the same namespace. + *---------- */ - if (pathpos > prevResult->pathpos) - { - pfree(newResult); - continue; /* keep previous result */ - } - else if (pathpos == prevResult->pathpos) - { - /* - * With variadic functions we could have, for example, - * both foo(numeric) and foo(variadic numeric[]) in - * the same namespace; if so we prefer the - * non-variadic match on efficiency grounds. It's - * also possible to have conflicting variadic - * functions, such as foo(numeric, variadic numeric[]) - * and foo(variadic numeric[]). If you're silly - * enough to do that, we throw an error. (XXX It'd be - * better to detect such conflicts when the functions - * are created.) - */ - if (variadic) - { - if (prevResult->nvargs > 0) - ereport(ERROR, - (errcode(ERRCODE_AMBIGUOUS_FUNCTION), - errmsg("variadic function %s conflicts with another", - func_signature_string(names, pronargs, - procform->proargtypes.values)))); - /* else, previous result wasn't variadic */ - pfree(newResult); - continue; /* keep previous result */ - } + preference = 0; + } - if (defaults) - { - if (prevResult->argdefaults != NIL) - ereport(ERROR, - (errcode(ERRCODE_AMBIGUOUS_FUNCTION), - errmsg("functions with parameter defaults %s and %s are ambiguous", - func_signature_string(names, pronargs, procform->proargtypes.values), - func_signature_string(names, prevResult->nargs, prevResult->args)))); - /* else, previous result didn't have defaults */ - pfree(newResult); - continue; /* keep previous result */ - } - - /* non-variadic can replace a previous variadic */ - Assert(prevResult->nvargs > 0); - } - /* replace previous result */ - prevResult->pathpos = pathpos; - prevResult->oid = newResult->oid; - prevResult->nvargs = newResult->nvargs; - prevResult->argdefaults = newResult->argdefaults; + if (preference > 0) + { + /* keep previous result */ pfree(newResult); - continue; /* args are same, of course */ + continue; + } + else if (preference < 0) + { + /* remove previous result from the list */ + if (prevResult == resultList) + resultList = prevResult->next; + else + { + FuncCandidateList prevPrevResult; + + for (prevPrevResult = resultList; + prevPrevResult; + prevPrevResult = prevPrevResult->next) + { + if (prevResult == prevPrevResult->next) + { + prevPrevResult->next = prevResult->next; + break; + } + } + Assert(prevPrevResult); /* assert we found it */ + } + pfree(prevResult); + /* fall through to add newResult to list */ + } + else + { + /* mark old result as ambiguous, discard new */ + prevResult->oid = InvalidOid; + pfree(newResult); + continue; } } } @@ -922,7 +932,7 @@ FunctionIsVisible(Oid funcid) visible = false; clist = FuncnameGetCandidates(list_make1(makeString(proname)), - nargs, false); + nargs, false, false); for (; clist; clist = clist->next) { @@ -1191,6 +1201,7 @@ OpernameGetCandidates(List *names, char oprkind) newResult->oid = HeapTupleGetOid(opertup); newResult->nargs = 2; newResult->nvargs = 0; + newResult->ndargs = 0; newResult->args[0] = operform->oprleft; newResult->args[1] = operform->oprright; newResult->next = resultList; diff --git a/src/backend/catalog/pg_aggregate.c b/src/backend/catalog/pg_aggregate.c index 7cd0d513ed..414366bc54 100644 --- a/src/backend/catalog/pg_aggregate.c +++ b/src/backend/catalog/pg_aggregate.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/catalog/pg_aggregate.c,v 1.98 2008/12/04 17:51:26 petere Exp $ + * $PostgreSQL: pgsql/src/backend/catalog/pg_aggregate.c,v 1.99 2008/12/18 18:20:33 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -227,10 +227,10 @@ AggregateCreate(const char *aggName, PointerGetDatum(NULL), /* allParamTypes */ PointerGetDatum(NULL), /* parameterModes */ PointerGetDatum(NULL), /* parameterNames */ + NIL, /* parameterDefaults */ PointerGetDatum(NULL), /* proconfig */ 1, /* procost */ - 0, /* prorows */ - NULL); /* parameterDefaults */ + 0); /* prorows */ /* * Okay to create the pg_aggregate entry. @@ -320,7 +320,7 @@ lookup_agg_function(List *fnName, * function's return value. it also returns the true argument types to * the function. */ - fdresult = func_get_detail(fnName, NIL, nargs, input_types, false, + fdresult = func_get_detail(fnName, NIL, nargs, input_types, false, false, &fnOid, rettype, &retset, &nvargs, &true_oid_array, NULL); diff --git a/src/backend/catalog/pg_proc.c b/src/backend/catalog/pg_proc.c index 1b414e3e5a..f9339fd2ce 100644 --- a/src/backend/catalog/pg_proc.c +++ b/src/backend/catalog/pg_proc.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/catalog/pg_proc.c,v 1.155 2008/12/04 17:51:26 petere Exp $ + * $PostgreSQL: pgsql/src/backend/catalog/pg_proc.c,v 1.156 2008/12/18 18:20:33 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -27,6 +27,7 @@ #include "funcapi.h" #include "mb/pg_wchar.h" #include "miscadmin.h" +#include "nodes/nodeFuncs.h" #include "parser/parse_type.h" #include "tcop/pquery.h" #include "tcop/tcopprot.h" @@ -73,10 +74,10 @@ ProcedureCreate(const char *procedureName, Datum allParameterTypes, Datum parameterModes, Datum parameterNames, + List *parameterDefaults, Datum proconfig, float4 procost, - float4 prorows, - List *parameterDefaults) + float4 prorows) { Oid retval; int parameterCount; @@ -311,11 +312,8 @@ ProcedureCreate(const char *procedureName, values[Anum_pg_proc_proargnames - 1] = parameterNames; else nulls[Anum_pg_proc_proargnames - 1] = true; - if (parameterDefaults != PointerGetDatum(NULL)) - { - Assert(list_length(parameterDefaults) > 0); + if (parameterDefaults != NIL) values[Anum_pg_proc_proargdefaults - 1] = CStringGetTextDatum(nodeToString(parameterDefaults)); - } else nulls[Anum_pg_proc_proargdefaults - 1] = true; values[Anum_pg_proc_prosrc - 1] = CStringGetTextDatum(prosrc); @@ -389,6 +387,57 @@ ProcedureCreate(const char *procedureName, errhint("Use DROP FUNCTION first."))); } + /* + * If there are existing defaults, check compatibility: redefinition + * must not remove any defaults nor change their types. (Removing + * a default might cause a function to fail to satisfy an existing + * call. Changing type would only be possible if the associated + * parameter is polymorphic, and in such cases a change of default + * type might alter the resolved output type of existing calls.) + */ + if (oldproc->pronargdefaults != 0) + { + Datum proargdefaults; + bool isnull; + List *oldDefaults; + ListCell *oldlc; + ListCell *newlc; + + if (list_length(parameterDefaults) < oldproc->pronargdefaults) + ereport(ERROR, + (errcode(ERRCODE_INVALID_FUNCTION_DEFINITION), + errmsg("cannot remove parameter defaults from existing function"), + errhint("Use DROP FUNCTION first."))); + + proargdefaults = SysCacheGetAttr(PROCNAMEARGSNSP, oldtup, + Anum_pg_proc_proargdefaults, + &isnull); + Assert(!isnull); + oldDefaults = (List *) stringToNode(TextDatumGetCString(proargdefaults)); + Assert(IsA(oldDefaults, List)); + Assert(list_length(oldDefaults) == oldproc->pronargdefaults); + + /* new list can have more defaults than old, advance over 'em */ + newlc = list_head(parameterDefaults); + for (i = list_length(parameterDefaults) - oldproc->pronargdefaults; + i > 0; + i--) + newlc = lnext(newlc); + + foreach(oldlc, oldDefaults) + { + Node *oldDef = (Node *) lfirst(oldlc); + Node *newDef = (Node *) lfirst(newlc); + + if (exprType(oldDef) != exprType(newDef)) + ereport(ERROR, + (errcode(ERRCODE_INVALID_FUNCTION_DEFINITION), + errmsg("cannot change data type of existing parameter default value"), + errhint("Use DROP FUNCTION first."))); + newlc = lnext(newlc); + } + } + /* Can't change aggregate status, either */ if (oldproc->proisagg != isAgg) { diff --git a/src/backend/commands/functioncmds.c b/src/backend/commands/functioncmds.c index 1509da7312..0a3de53e1e 100644 --- a/src/backend/commands/functioncmds.c +++ b/src/backend/commands/functioncmds.c @@ -10,7 +10,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/commands/functioncmds.c,v 1.102 2008/12/04 17:51:26 petere Exp $ + * $PostgreSQL: pgsql/src/backend/commands/functioncmds.c,v 1.103 2008/12/18 18:20:33 tgl Exp $ * * DESCRIPTION * These routines take the parse tree and pick out the @@ -48,11 +48,11 @@ #include "commands/defrem.h" #include "commands/proclang.h" #include "miscadmin.h" +#include "optimizer/var.h" #include "parser/parse_coerce.h" #include "parser/parse_expr.h" #include "parser/parse_func.h" #include "parser/parse_type.h" -#include "parser/parse_utilcmd.h" #include "utils/acl.h" #include "utils/builtins.h" #include "utils/fmgroids.h" @@ -162,13 +162,13 @@ compute_return_type(TypeName *returnType, Oid languageOid, */ static void examine_parameter_list(List *parameters, Oid languageOid, + const char *queryString, oidvector **parameterTypes, ArrayType **allParameterTypes, ArrayType **parameterModes, ArrayType **parameterNames, List **parameterDefaults, - Oid *requiredResultType, - const char *queryString) + Oid *requiredResultType) { int parameterCount = list_length(parameters); Oid *inTypes; @@ -179,9 +179,9 @@ examine_parameter_list(List *parameters, Oid languageOid, int outCount = 0; int varCount = 0; bool have_names = false; + bool have_defaults = false; ListCell *x; int i; - bool have_defaults = false; ParseState *pstate; *requiredResultType = InvalidOid; /* default result */ @@ -192,6 +192,7 @@ examine_parameter_list(List *parameters, Oid languageOid, paramNames = (Datum *) palloc0(parameterCount * sizeof(Datum)); *parameterDefaults = NIL; + /* may need a pstate for parse analysis of default exprs */ pstate = make_parsestate(NULL); pstate->p_sourcetext = queryString; @@ -201,6 +202,7 @@ examine_parameter_list(List *parameters, Oid languageOid, { FunctionParameter *fp = (FunctionParameter *) lfirst(x); TypeName *t = fp->argType; + bool isinput = false; Oid toid; Type typtup; @@ -247,6 +249,7 @@ examine_parameter_list(List *parameters, Oid languageOid, (errcode(ERRCODE_INVALID_FUNCTION_DEFINITION), errmsg("VARIADIC parameter must be the last input parameter"))); inTypes[inCount++] = toid; + isinput = true; } /* handle output parameters */ @@ -288,24 +291,46 @@ examine_parameter_list(List *parameters, Oid languageOid, if (fp->defexpr) { - if (fp->mode != FUNC_PARAM_IN && fp->mode != FUNC_PARAM_INOUT) + Node *def; + + if (!isinput) ereport(ERROR, (errcode(ERRCODE_INVALID_FUNCTION_DEFINITION), - errmsg("only IN and INOUT parameters can have default values"))); + errmsg("only input parameters can have default values"))); - *parameterDefaults = lappend(*parameterDefaults, - coerce_to_specific_type(NULL, - transformExpr(pstate, fp->defexpr), - toid, - "DEFAULT")); + def = transformExpr(pstate, fp->defexpr); + def = coerce_to_specific_type(pstate, def, toid, "DEFAULT"); + + /* + * Make sure no variables are referred to. + */ + if (list_length(pstate->p_rtable) != 0 || + contain_var_clause(def)) + ereport(ERROR, + (errcode(ERRCODE_INVALID_COLUMN_REFERENCE), + errmsg("cannot use table references in parameter default value"))); + + /* + * No subplans or aggregates, either... + */ + if (pstate->p_hasSubLinks) + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("cannot use subquery in parameter default value"))); + if (pstate->p_hasAggs) + ereport(ERROR, + (errcode(ERRCODE_GROUPING_ERROR), + errmsg("cannot use aggregate function in parameter default value"))); + + *parameterDefaults = lappend(*parameterDefaults, def); have_defaults = true; } else { - if (have_defaults) + if (isinput && have_defaults) ereport(ERROR, (errcode(ERRCODE_INVALID_FUNCTION_DEFINITION), - errmsg("parameter without default value specified after parameter with default value"))); + errmsg("input parameters after one with a default value must also have defaults"))); } i++; @@ -704,6 +729,7 @@ CreateFunction(CreateFunctionStmt *stmt, const char *queryString) ArrayType *allParameterTypes; ArrayType *parameterModes; ArrayType *parameterNames; + List *parameterDefaults; Oid requiredResultType; bool isStrict, security; @@ -714,7 +740,6 @@ CreateFunction(CreateFunctionStmt *stmt, const char *queryString) HeapTuple languageTuple; Form_pg_language languageStruct; List *as_clause; - List *defaults = NULL; /* Convert list of names to a name and namespace */ namespaceId = QualifiedNameGetCreationNamespace(stmt->funcname, @@ -783,14 +808,13 @@ CreateFunction(CreateFunctionStmt *stmt, const char *queryString) * Convert remaining parameters of CREATE to form wanted by * ProcedureCreate. */ - examine_parameter_list(stmt->parameters, languageOid, + examine_parameter_list(stmt->parameters, languageOid, queryString, ¶meterTypes, &allParameterTypes, ¶meterModes, ¶meterNames, - &defaults, - &requiredResultType, - queryString); + ¶meterDefaults, + &requiredResultType); if (stmt->returnType) { @@ -871,10 +895,10 @@ CreateFunction(CreateFunctionStmt *stmt, const char *queryString) PointerGetDatum(allParameterTypes), PointerGetDatum(parameterModes), PointerGetDatum(parameterNames), + parameterDefaults, PointerGetDatum(proconfig), procost, - prorows, - defaults); + prorows); } diff --git a/src/backend/commands/proclang.c b/src/backend/commands/proclang.c index 2166a1de5f..3188156c4d 100644 --- a/src/backend/commands/proclang.c +++ b/src/backend/commands/proclang.c @@ -7,7 +7,7 @@ * Portions Copyright (c) 1994, Regents of the University of California * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/commands/proclang.c,v 1.81 2008/12/04 17:51:26 petere Exp $ + * $PostgreSQL: pgsql/src/backend/commands/proclang.c,v 1.82 2008/12/18 18:20:33 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -147,10 +147,10 @@ CreateProceduralLanguage(CreatePLangStmt *stmt) PointerGetDatum(NULL), PointerGetDatum(NULL), PointerGetDatum(NULL), + NIL, PointerGetDatum(NULL), 1, - 0, - NULL); + 0); } /* @@ -181,10 +181,10 @@ CreateProceduralLanguage(CreatePLangStmt *stmt) PointerGetDatum(NULL), PointerGetDatum(NULL), PointerGetDatum(NULL), + NIL, PointerGetDatum(NULL), 1, - 0, - NULL); + 0); } } else @@ -548,9 +548,9 @@ AlterLanguageOwner(const char *name, Oid newOwnerId) errmsg("language \"%s\" does not exist", name))); AlterLanguageOwner_internal(tup, rel, newOwnerId); - + ReleaseSysCache(tup); - + heap_close(rel, RowExclusiveLock); } diff --git a/src/backend/optimizer/plan/planner.c b/src/backend/optimizer/plan/planner.c index 467ff39a31..7f91309032 100644 --- a/src/backend/optimizer/plan/planner.c +++ b/src/backend/optimizer/plan/planner.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/optimizer/plan/planner.c,v 1.246 2008/10/22 20:17:51 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/optimizer/plan/planner.c,v 1.247 2008/12/18 18:20:33 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -513,27 +513,18 @@ preprocess_expression(PlannerInfo *root, Node *expr, int kind) /* * Simplify constant expressions. * + * Note: one essential effect here is to insert the current actual values + * of any default arguments for functions. To ensure that happens, we + * *must* process all expressions here. Previous PG versions sometimes + * skipped const-simplification if it didn't seem worth the trouble, but + * we can't do that anymore. + * * Note: this also flattens nested AND and OR expressions into N-argument * form. All processing of a qual expression after this point must be * careful to maintain AND/OR flatness --- that is, do not generate a tree * with AND directly under AND, nor OR directly under OR. - * - * Because this is a relatively expensive process, we skip it when the - * query is trivial, such as "SELECT 2+2;" or "INSERT ... VALUES()". The - * expression will only be evaluated once anyway, so no point in - * pre-simplifying; we can't execute it any faster than the executor can, - * and we will waste cycles copying the tree. Notice however that we - * still must do it for quals (to get AND/OR flatness); and if we are in a - * subquery we should not assume it will be done only once. - * - * For VALUES lists we never do this at all, again on the grounds that we - * should optimize for one-time evaluation. */ - if (kind != EXPRKIND_VALUES && - (root->parse->jointree->fromlist != NIL || - kind == EXPRKIND_QUAL || - root->query_level > 1)) - expr = eval_const_expressions(root, expr); + expr = eval_const_expressions(root, expr); /* * If it's a qual or havingQual, canonicalize it. diff --git a/src/backend/optimizer/util/clauses.c b/src/backend/optimizer/util/clauses.c index c826ecb2ad..3c74831f4d 100644 --- a/src/backend/optimizer/util/clauses.c +++ b/src/backend/optimizer/util/clauses.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/optimizer/util/clauses.c,v 1.270 2008/10/21 20:42:53 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/optimizer/util/clauses.c,v 1.271 2008/12/18 18:20:34 tgl Exp $ * * HISTORY * AUTHOR DATE MAJOR EVENT @@ -36,6 +36,7 @@ #include "optimizer/var.h" #include "parser/analyze.h" #include "parser/parse_coerce.h" +#include "parser/parse_func.h" #include "rewrite/rewriteManip.h" #include "tcop/tcopprot.h" #include "utils/acl.h" @@ -91,9 +92,11 @@ static List *simplify_and_arguments(List *args, bool *haveNull, bool *forceFalse); static Expr *simplify_boolean_equality(List *args); static Expr *simplify_function(Oid funcid, - Oid result_type, int32 result_typmod, List *args, + Oid result_type, int32 result_typmod, List **args, bool allow_inline, eval_const_expressions_context *context); +static List *add_function_defaults(List *args, Oid result_type, + HeapTuple func_tuple); static Expr *evaluate_function(Oid funcid, Oid result_type, int32 result_typmod, List *args, HeapTuple func_tuple, @@ -2025,7 +2028,7 @@ eval_const_expressions_mutator(Node *node, */ simple = simplify_function(expr->funcid, expr->funcresulttype, exprTypmod(node), - args, + &args, true, context); if (simple) /* successfully simplified it */ return (Node *) simple; @@ -2072,7 +2075,7 @@ eval_const_expressions_mutator(Node *node, */ simple = simplify_function(expr->opfuncid, expr->opresulttype, -1, - args, + &args, true, context); if (simple) /* successfully simplified it */ return (Node *) simple; @@ -2163,7 +2166,7 @@ eval_const_expressions_mutator(Node *node, */ simple = simplify_function(expr->opfuncid, expr->opresulttype, -1, - args, + &args, false, context); if (simple) /* successfully simplified it */ { @@ -2329,6 +2332,7 @@ eval_const_expressions_mutator(Node *node, { CoerceViaIO *expr = (CoerceViaIO *) node; Expr *arg; + List *args; Oid outfunc; bool outtypisvarlena; Oid infunc; @@ -2341,6 +2345,7 @@ eval_const_expressions_mutator(Node *node, */ arg = (Expr *) eval_const_expressions_mutator((Node *) expr->arg, context); + args = list_make1(arg); /* * CoerceViaIO represents calling the source type's output function @@ -2353,7 +2358,7 @@ eval_const_expressions_mutator(Node *node, simple = simplify_function(outfunc, CSTRINGOID, -1, - list_make1(arg), + &args, true, context); if (simple) /* successfully simplified output fn */ { @@ -2361,8 +2366,6 @@ eval_const_expressions_mutator(Node *node, * Input functions may want 1 to 3 arguments. We always supply * all three, trusting that nothing downstream will complain. */ - List *args; - args = list_make3(simple, makeConst(OIDOID, -1, sizeof(Oid), ObjectIdGetDatum(intypioparam), @@ -2373,7 +2376,7 @@ eval_const_expressions_mutator(Node *node, simple = simplify_function(infunc, expr->resulttype, -1, - args, + &args, true, context); if (simple) /* successfully simplified input fn */ return (Node *) simple; @@ -3126,10 +3129,16 @@ simplify_boolean_equality(List *args) * * Returns a simplified expression if successful, or NULL if cannot * simplify the function call. + * + * This function is also responsible for adding any default argument + * expressions onto the function argument list; which is a bit grotty, + * but it avoids an extra fetch of the function's pg_proc tuple. For this + * reason, the args list is pass-by-reference, and it may get modified + * even if simplification fails. */ static Expr * simplify_function(Oid funcid, Oid result_type, int32 result_typmod, - List *args, + List **args, bool allow_inline, eval_const_expressions_context *context) { @@ -3150,11 +3159,15 @@ simplify_function(Oid funcid, Oid result_type, int32 result_typmod, if (!HeapTupleIsValid(func_tuple)) elog(ERROR, "cache lookup failed for function %u", funcid); - newexpr = evaluate_function(funcid, result_type, result_typmod, args, + /* While we have the tuple, check if we need to add defaults */ + if (((Form_pg_proc) GETSTRUCT(func_tuple))->pronargs > list_length(*args)) + *args = add_function_defaults(*args, result_type, func_tuple); + + newexpr = evaluate_function(funcid, result_type, result_typmod, *args, func_tuple, context); if (!newexpr && allow_inline) - newexpr = inline_function(funcid, result_type, args, + newexpr = inline_function(funcid, result_type, *args, func_tuple, context); ReleaseSysCache(func_tuple); @@ -3162,6 +3175,77 @@ simplify_function(Oid funcid, Oid result_type, int32 result_typmod, return newexpr; } +/* + * add_function_defaults: add missing function arguments from its defaults + * + * It is possible for some of the defaulted arguments to be polymorphic; + * therefore we can't assume that the default expressions have the correct + * data types already. We have to re-resolve polymorphics and do coercion + * just like the parser did. + */ +static List * +add_function_defaults(List *args, Oid result_type, HeapTuple func_tuple) +{ + Form_pg_proc funcform = (Form_pg_proc) GETSTRUCT(func_tuple); + Datum proargdefaults; + bool isnull; + char *str; + List *defaults; + int ndelete; + int nargs; + Oid actual_arg_types[FUNC_MAX_ARGS]; + Oid declared_arg_types[FUNC_MAX_ARGS]; + Oid rettype; + ListCell *lc; + + /* The error cases here shouldn't happen, but check anyway */ + proargdefaults = SysCacheGetAttr(PROCOID, func_tuple, + Anum_pg_proc_proargdefaults, + &isnull); + if (isnull) + elog(ERROR, "not enough default arguments"); + str = TextDatumGetCString(proargdefaults); + defaults = (List *) stringToNode(str); + Assert(IsA(defaults, List)); + pfree(str); + /* Delete any unused defaults from the list */ + ndelete = list_length(args) + list_length(defaults) - funcform->pronargs; + if (ndelete < 0) + elog(ERROR, "not enough default arguments"); + while (ndelete-- > 0) + defaults = list_delete_first(defaults); + /* And form the combined argument list */ + args = list_concat(args, defaults); + Assert(list_length(args) == funcform->pronargs); + + /* + * The rest of this should be a no-op if there are no polymorphic + * arguments, but we do it anyway to be sure. + */ + if (list_length(args) > FUNC_MAX_ARGS) + elog(ERROR, "too many function arguments"); + nargs = 0; + foreach(lc, args) + { + actual_arg_types[nargs++] = exprType((Node *) lfirst(lc)); + } + memcpy(declared_arg_types, funcform->proargtypes.values, + funcform->pronargs * sizeof(Oid)); + rettype = enforce_generic_type_consistency(actual_arg_types, + declared_arg_types, + nargs, + funcform->prorettype, + false); + /* let's just check we got the same answer as the parser did ... */ + if (rettype != result_type) + elog(ERROR, "function's resolved result type changed during planning"); + + /* perform any necessary typecasting of arguments */ + make_fn_arguments(NULL, args, actual_arg_types, declared_arg_types); + + return args; +} + /* * evaluate_function: try to pre-evaluate a function call * diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index e53333d30c..337af63327 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -11,7 +11,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/parser/gram.y,v 2.644 2008/12/17 09:15:02 heikki Exp $ + * $PostgreSQL: pgsql/src/backend/parser/gram.y,v 2.645 2008/12/18 18:20:34 tgl Exp $ * * HISTORY * AUTHOR DATE MAJOR EVENT @@ -3465,7 +3465,7 @@ opt_restart_seqs: * * COMMENT ON [ [ DATABASE | DOMAIN | INDEX | SEQUENCE | TABLE | TYPE | VIEW | * CONVERSION | LANGUAGE | OPERATOR CLASS | LARGE OBJECT | - * CAST | COLUMN | SCHEMA | TABLESPACE | ROLE | + * CAST | COLUMN | SCHEMA | TABLESPACE | ROLE | * TEXT SEARCH PARSER | TEXT SEARCH DICTIONARY | * TEXT SEARCH TEMPLATE | * TEXT SEARCH CONFIGURATION ] | @@ -4230,15 +4230,15 @@ func_args_list: */ func_args_with_defaults: '(' func_args_with_defaults_list ')' { $$ = $2; } - | '(' ')' { $$ = NIL; } + | '(' ')' { $$ = NIL; } ; func_args_with_defaults_list: - func_arg_with_default { $$ = list_make1( $1); } - | func_args_with_defaults_list ',' func_arg_with_default { $$ = lappend($1, $3); } + func_arg_with_default { $$ = list_make1($1); } + | func_args_with_defaults_list ',' func_arg_with_default + { $$ = lappend($1, $3); } ; - /* * The style with arg_class first is SQL99 standard, but Oracle puts * param_name first; accept both since it's likely people will try both @@ -4345,7 +4345,7 @@ func_type: Typename { $$ = $1; } func_arg_with_default: func_arg - { + { $$ = $1; } | func_arg DEFAULT a_expr @@ -4459,6 +4459,7 @@ table_func_column: param_name func_type n->name = $1; n->argType = $2; n->mode = FUNC_PARAM_TABLE; + n->defexpr = NULL; $$ = n; } ; @@ -5755,7 +5756,7 @@ AlterTSConfigurationStmt: n->replace = false; $$ = (Node*)n; } - | ALTER TEXT_P SEARCH CONFIGURATION any_name ALTER MAPPING REPLACE any_name WITH any_name + | ALTER TEXT_P SEARCH CONFIGURATION any_name ALTER MAPPING REPLACE any_name WITH any_name { AlterTSConfigurationStmt *n = makeNode(AlterTSConfigurationStmt); n->cfgname = $5; @@ -5765,7 +5766,7 @@ AlterTSConfigurationStmt: n->replace = true; $$ = (Node*)n; } - | ALTER TEXT_P SEARCH CONFIGURATION any_name ALTER MAPPING FOR name_list REPLACE any_name WITH any_name + | ALTER TEXT_P SEARCH CONFIGURATION any_name ALTER MAPPING FOR name_list REPLACE any_name WITH any_name { AlterTSConfigurationStmt *n = makeNode(AlterTSConfigurationStmt); n->cfgname = $5; @@ -5775,7 +5776,7 @@ AlterTSConfigurationStmt: n->replace = true; $$ = (Node*)n; } - | ALTER TEXT_P SEARCH CONFIGURATION any_name DROP MAPPING FOR name_list + | ALTER TEXT_P SEARCH CONFIGURATION any_name DROP MAPPING FOR name_list { AlterTSConfigurationStmt *n = makeNode(AlterTSConfigurationStmt); n->cfgname = $5; @@ -5783,7 +5784,7 @@ AlterTSConfigurationStmt: n->missing_ok = false; $$ = (Node*)n; } - | ALTER TEXT_P SEARCH CONFIGURATION any_name DROP MAPPING IF_P EXISTS FOR name_list + | ALTER TEXT_P SEARCH CONFIGURATION any_name DROP MAPPING IF_P EXISTS FOR name_list { AlterTSConfigurationStmt *n = makeNode(AlterTSConfigurationStmt); n->cfgname = $5; diff --git a/src/backend/parser/parse_func.c b/src/backend/parser/parse_func.c index 1171d367c6..d0b74ff5d9 100644 --- a/src/backend/parser/parse_func.c +++ b/src/backend/parser/parse_func.c @@ -8,7 +8,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/parser/parse_func.c,v 1.208 2008/12/04 17:51:26 petere Exp $ + * $PostgreSQL: pgsql/src/backend/parser/parse_func.c,v 1.209 2008/12/18 18:20:34 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -71,13 +71,14 @@ ParseFuncOrColumn(ParseState *pstate, List *funcname, List *fargs, ListCell *nextl; Node *first_arg = NULL; int nargs; + int nargsplusdefs; Oid actual_arg_types[FUNC_MAX_ARGS]; Oid *declared_arg_types; + List *argdefaults; Node *retval; bool retset; int nvargs; FuncDetailCode fdresult; - List *argdefaults; /* * Most of the rest of the parser just assumes that functions do not have @@ -157,13 +158,13 @@ ParseFuncOrColumn(ParseState *pstate, List *funcname, List *fargs, * disambiguation for polymorphic functions, handles inheritance, and * returns the funcid and type and set or singleton status of the * function's return value. It also returns the true argument types to - * the function. (In the case of a variadic function call, the reported + * the function. In the case of a variadic function call, the reported * "true" types aren't really what is in pg_proc: the variadic argument is * replaced by a suitable number of copies of its element type. We'll fix - * it up below.) + * it up below. We may also have to deal with default arguments. */ fdresult = func_get_detail(funcname, fargs, nargs, actual_arg_types, - !func_variadic, + !func_variadic, true, &funcid, &rettype, &retset, &nvargs, &declared_arg_types, &argdefaults); if (fdresult == FUNCDETAIL_COERCION) @@ -235,21 +236,29 @@ ParseFuncOrColumn(ParseState *pstate, List *funcname, List *fargs, parser_errposition(pstate, location))); } - /* add stored expressions as called values for arguments with defaults */ - if (argdefaults) + /* + * If there are default arguments, we have to include their types in + * actual_arg_types for the purpose of checking generic type consistency. + * However, we do NOT put them into the generated parse node, because + * their actual values might change before the query gets run. The + * planner has to insert the up-to-date values at plan time. + */ + nargsplusdefs = nargs; + foreach(l, argdefaults) { - ListCell *lc; + Node *expr = (Node *) lfirst(l); - foreach(lc, argdefaults) - { - Node *expr = (Node *) lfirst(lc); - - fargs = lappend(fargs, expr); - actual_arg_types[nargs++] = exprType(expr); - } + /* probably shouldn't happen ... */ + if (nargsplusdefs >= FUNC_MAX_ARGS) + ereport(ERROR, + (errcode(ERRCODE_TOO_MANY_ARGUMENTS), + errmsg("cannot pass more than %d arguments to a function", + FUNC_MAX_ARGS), + parser_errposition(pstate, location))); + + actual_arg_types[nargsplusdefs++] = exprType(expr); } - /* * enforce consistency with polymorphic argument and return types, * possibly adjusting return type or declared_arg_types (which will be @@ -257,7 +266,7 @@ ParseFuncOrColumn(ParseState *pstate, List *funcname, List *fargs, */ rettype = enforce_generic_type_consistency(actual_arg_types, declared_arg_types, - nargs, + nargsplusdefs, rettype, false); @@ -741,18 +750,20 @@ func_get_detail(List *funcname, int nargs, Oid *argtypes, bool expand_variadic, + bool expand_defaults, Oid *funcid, /* return value */ Oid *rettype, /* return value */ bool *retset, /* return value */ int *nvargs, /* return value */ Oid **true_typeids, /* return value */ - List **argdefaults) /* return value */ + List **argdefaults) /* optional return value */ { FuncCandidateList raw_candidates; FuncCandidateList best_candidate; /* Get list of possible candidates from namespace search */ - raw_candidates = FuncnameGetCandidates(funcname, nargs, expand_variadic); + raw_candidates = FuncnameGetCandidates(funcname, nargs, + expand_variadic, expand_defaults); /* * Quickly check if there is an exact match to the input datatypes (there @@ -884,11 +895,17 @@ func_get_detail(List *funcname, Form_pg_proc pform; FuncDetailCode result; + /* + * If expanding variadics or defaults, the "best candidate" might + * represent multiple equivalently good functions; treat this case + * as ambiguous. + */ + if (!OidIsValid(best_candidate->oid)) + return FUNCDETAIL_MULTIPLE; + *funcid = best_candidate->oid; *nvargs = best_candidate->nvargs; *true_typeids = best_candidate->args; - if (argdefaults) - *argdefaults = best_candidate->argdefaults; ftup = SearchSysCache(PROCOID, ObjectIdGetDatum(best_candidate->oid), @@ -899,6 +916,38 @@ func_get_detail(List *funcname, pform = (Form_pg_proc) GETSTRUCT(ftup); *rettype = pform->prorettype; *retset = pform->proretset; + /* fetch default args if caller wants 'em */ + if (argdefaults) + { + if (best_candidate->ndargs > 0) + { + Datum proargdefaults; + bool isnull; + char *str; + List *defaults; + int ndelete; + + /* shouldn't happen, FuncnameGetCandidates messed up */ + if (best_candidate->ndargs > pform->pronargdefaults) + elog(ERROR, "not enough default arguments"); + + proargdefaults = SysCacheGetAttr(PROCOID, ftup, + Anum_pg_proc_proargdefaults, + &isnull); + Assert(!isnull); + str = TextDatumGetCString(proargdefaults); + defaults = (List *) stringToNode(str); + Assert(IsA(defaults, List)); + pfree(str); + /* Delete any unused defaults from the returned list */ + ndelete = list_length(defaults) - best_candidate->ndargs; + while (ndelete-- > 0) + defaults = list_delete_first(defaults); + *argdefaults = defaults; + } + else + *argdefaults = NIL; + } result = pform->proisagg ? FUNCDETAIL_AGGREGATE : FUNCDETAIL_NORMAL; ReleaseSysCache(ftup); return result; @@ -1243,7 +1292,7 @@ LookupFuncName(List *funcname, int nargs, const Oid *argtypes, bool noError) { FuncCandidateList clist; - clist = FuncnameGetCandidates(funcname, nargs, false); + clist = FuncnameGetCandidates(funcname, nargs, false, false); while (clist) { diff --git a/src/backend/utils/adt/regproc.c b/src/backend/utils/adt/regproc.c index d50dc23d77..b690c19273 100644 --- a/src/backend/utils/adt/regproc.c +++ b/src/backend/utils/adt/regproc.c @@ -13,7 +13,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/utils/adt/regproc.c,v 1.108 2008/07/16 01:30:22 tgl Exp $ + * $PostgreSQL: pgsql/src/backend/utils/adt/regproc.c,v 1.109 2008/12/18 18:20:34 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -131,7 +131,7 @@ regprocin(PG_FUNCTION_ARGS) * pg_proc entries in the current search path. */ names = stringToQualifiedNameList(pro_name_or_oid); - clist = FuncnameGetCandidates(names, -1, false); + clist = FuncnameGetCandidates(names, -1, false, false); if (clist == NULL) ereport(ERROR, @@ -190,7 +190,7 @@ regprocout(PG_FUNCTION_ARGS) * qualify it. */ clist = FuncnameGetCandidates(list_make1(makeString(proname)), - -1, false); + -1, false, false); if (clist != NULL && clist->next == NULL && clist->oid == proid) nspname = NULL; @@ -277,7 +277,7 @@ regprocedurein(PG_FUNCTION_ARGS) */ parseNameAndArgTypes(pro_name_or_oid, false, &names, &nargs, argtypes); - clist = FuncnameGetCandidates(names, nargs, false); + clist = FuncnameGetCandidates(names, nargs, false, false); for (; clist; clist = clist->next) { diff --git a/src/backend/utils/adt/ruleutils.c b/src/backend/utils/adt/ruleutils.c index afa19b384e..5a6540b88e 100644 --- a/src/backend/utils/adt/ruleutils.c +++ b/src/backend/utils/adt/ruleutils.c @@ -9,7 +9,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/utils/adt/ruleutils.c,v 1.288 2008/12/04 17:51:27 petere Exp $ + * $PostgreSQL: pgsql/src/backend/utils/adt/ruleutils.c,v 1.289 2008/12/18 18:20:34 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -141,8 +141,7 @@ static char *pg_get_constraintdef_worker(Oid constraintId, bool fullCommand, static char *pg_get_expr_worker(text *expr, Oid relid, char *relname, int prettyFlags); static int print_function_arguments(StringInfo buf, HeapTuple proctup, - bool print_table_args, - bool full); + bool print_table_args, bool print_defaults); static void print_function_rettype(StringInfo buf, HeapTuple proctup); static void make_ruledef(StringInfo buf, HeapTuple ruletup, TupleDesc rulettc, int prettyFlags); @@ -1610,7 +1609,7 @@ pg_get_function_arguments(PG_FUNCTION_ARGS) * pg_get_function_identity_arguments * Get a formatted list of arguments for a function. * This is everything that would go between the parentheses in - * ALTER FUNCTION, etc. skip names and defaults/ + * ALTER FUNCTION, etc. In particular, don't print defaults. */ Datum pg_get_function_identity_arguments(PG_FUNCTION_ARGS) @@ -1634,8 +1633,6 @@ pg_get_function_identity_arguments(PG_FUNCTION_ARGS) PG_RETURN_TEXT_P(string_to_text(buf.data)); } - - /* * pg_get_function_result * Get a nicely-formatted version of the result type of a function. @@ -1680,7 +1677,7 @@ print_function_rettype(StringInfo buf, HeapTuple proctup) { /* It might be a table function; try to print the arguments */ appendStringInfoString(&rbuf, "TABLE("); - ntabargs = print_function_arguments(&rbuf, proctup, true, true); + ntabargs = print_function_arguments(&rbuf, proctup, true, false); if (ntabargs > 0) appendStringInfoString(&rbuf, ")"); else @@ -1702,100 +1699,112 @@ print_function_rettype(StringInfo buf, HeapTuple proctup) * Common code for pg_get_function_arguments and pg_get_function_result: * append the desired subset of arguments to buf. We print only TABLE * arguments when print_table_args is true, and all the others when it's false. + * We print argument defaults only if print_defaults is true. * Function return value is the number of arguments printed. - * When full is false, then don't print argument names and argument defaults. */ static int print_function_arguments(StringInfo buf, HeapTuple proctup, - bool print_table_args, - bool full) + bool print_table_args, bool print_defaults) { + Form_pg_proc proc = (Form_pg_proc) GETSTRUCT(proctup); int numargs; Oid *argtypes; char **argnames; char *argmodes; int argsprinted; + int inputargno; + int nlackdefaults; + ListCell *nextargdefault = NULL; int i; - Datum proargdefaults; - List *argdefaults; - int nargdefaults; - bool isnull; - List *dcontext = NIL; numargs = get_func_arg_info(proctup, &argtypes, &argnames, &argmodes); - proargdefaults = SysCacheGetAttr(PROCOID, proctup, - Anum_pg_proc_proargdefaults, &isnull); - if (!isnull) + nlackdefaults = numargs; + if (print_defaults && proc->pronargdefaults > 0) { - char *str; + Datum proargdefaults; + bool isnull; - str = TextDatumGetCString(proargdefaults); - argdefaults = (List *) stringToNode(str); - Assert(IsA(argdefaults, List)); - nargdefaults = list_length(argdefaults); - - /* we will need deparse context */ - //dcontext = deparse_context_for("", InvalidOid); - dcontext = NULL; - pfree(str); - } - else - { - argdefaults = NIL; - nargdefaults = 0; + proargdefaults = SysCacheGetAttr(PROCOID, proctup, + Anum_pg_proc_proargdefaults, + &isnull); + if (!isnull) + { + char *str; + List *argdefaults; + + str = TextDatumGetCString(proargdefaults); + argdefaults = (List *) stringToNode(str); + Assert(IsA(argdefaults, List)); + pfree(str); + nextargdefault = list_head(argdefaults); + /* nlackdefaults counts only *input* arguments lacking defaults */ + nlackdefaults = proc->pronargs - list_length(argdefaults); + } } argsprinted = 0; + inputargno = 0; for (i = 0; i < numargs; i++) { Oid argtype = argtypes[i]; char *argname = argnames ? argnames[i] : NULL; char argmode = argmodes ? argmodes[i] : PROARGMODE_IN; const char *modename; - - if (print_table_args != (argmode == PROARGMODE_TABLE)) - continue; + bool isinput; switch (argmode) { case PROARGMODE_IN: modename = ""; + isinput = true; break; case PROARGMODE_INOUT: modename = "INOUT "; + isinput = true; break; case PROARGMODE_OUT: modename = "OUT "; + isinput = false; break; case PROARGMODE_VARIADIC: modename = "VARIADIC "; + isinput = true; break; case PROARGMODE_TABLE: modename = ""; + isinput = false; break; default: elog(ERROR, "invalid parameter mode '%c'", argmode); modename = NULL; /* keep compiler quiet */ + isinput = false; break; } + if (isinput) + inputargno++; /* this is a 1-based counter */ + + if (print_table_args != (argmode == PROARGMODE_TABLE)) + continue; + if (argsprinted) appendStringInfoString(buf, ", "); appendStringInfoString(buf, modename); - if (argname && argname[0] && full) + if (argname && argname[0]) appendStringInfo(buf, "%s ", argname); appendStringInfoString(buf, format_type_be(argtype)); - - /* search given default expression, expect less numargs */ - if (nargdefaults > 0 && i >= (numargs - nargdefaults) && full) + if (print_defaults && isinput && inputargno > nlackdefaults) { Node *expr; - expr = (Node *) list_nth(argdefaults, i - (numargs - nargdefaults)); - appendStringInfo(buf, " DEFAULT %s", deparse_expression(expr, dcontext, false, false)); - } + Assert(nextargdefault != NULL); + expr = (Node *) lfirst(nextargdefault); + nextargdefault = lnext(nextargdefault); + appendStringInfo(buf, " DEFAULT %s", + deparse_expression(expr, NIL, false, false)); + } argsprinted++; } @@ -6062,7 +6071,6 @@ generate_function_name(Oid funcid, int nargs, Oid *argtypes, elog(ERROR, "cache lookup failed for function %u", funcid); procform = (Form_pg_proc) GETSTRUCT(proctup); proname = NameStr(procform->proname); - Assert(nargs >= procform->pronargs); /* * The idea here is to schema-qualify only if the parser would fail to @@ -6070,7 +6078,7 @@ generate_function_name(Oid funcid, int nargs, Oid *argtypes, * specified argtypes. */ p_result = func_get_detail(list_make1(makeString(proname)), - NIL, nargs, argtypes, false, + NIL, nargs, argtypes, false, true, &p_funcid, &p_rettype, &p_retset, &p_nvargs, &p_true_typeids, NULL); if ((p_result == FUNCDETAIL_NORMAL || p_result == FUNCDETAIL_AGGREGATE) && diff --git a/src/bin/pg_dump/pg_dump.c b/src/bin/pg_dump/pg_dump.c index 0f7419a653..04758761d4 100644 --- a/src/bin/pg_dump/pg_dump.c +++ b/src/bin/pg_dump/pg_dump.c @@ -12,7 +12,7 @@ * by PostgreSQL * * IDENTIFICATION - * $PostgreSQL: pgsql/src/bin/pg_dump/pg_dump.c,v 1.507 2008/12/11 07:34:08 petere Exp $ + * $PostgreSQL: pgsql/src/bin/pg_dump/pg_dump.c,v 1.508 2008/12/18 18:20:34 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -591,7 +591,7 @@ main(int argc, char **argv) */ if (g_fout->remoteVersion >= 70300) do_sql_command(g_conn, "SET statement_timeout = 0"); - + /* * Start serializable transaction to dump consistent data. */ @@ -6621,7 +6621,7 @@ static char *format_function_arguments(FuncInfo *finfo, char *funcargs) * is never qualified. * * This is used only with pre-8.4 servers, so we aren't expecting to see - * VARIADIC or TABLE arguments. + * VARIADIC or TABLE arguments, nor are there any defaults for arguments. * * Any or all of allargtypes, argmodes, argnames may be NULL. */ @@ -7011,14 +7011,15 @@ dumpFunc(Archive *fout, FuncInfo *finfo) } } - /* funcargs and funciargs are supported from 8.4 */ - if (funciargs) + if (funcargs) { - funcsig = format_function_arguments(finfo, funciargs); + /* 8.4 or later; we rely on server-side code for most of the work */ funcfullsig = format_function_arguments(finfo, funcargs); + funcsig = format_function_arguments(finfo, funciargs); } else { + /* pre-8.4, do it ourselves */ funcsig = format_function_arguments_old(finfo, nallargs, allargtypes, argmodes, argnames); funcfullsig = funcsig; diff --git a/src/include/catalog/namespace.h b/src/include/catalog/namespace.h index 12d8ef61b8..8918acba09 100644 --- a/src/include/catalog/namespace.h +++ b/src/include/catalog/namespace.h @@ -7,7 +7,7 @@ * Portions Copyright (c) 1996-2008, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $PostgreSQL: pgsql/src/include/catalog/namespace.h,v 1.56 2008/12/04 17:51:27 petere Exp $ + * $PostgreSQL: pgsql/src/include/catalog/namespace.h,v 1.57 2008/12/18 18:20:34 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -22,6 +22,7 @@ * found by namespace lookup. Each function/operator is identified * by OID and by argument types; the list must be pruned by type * resolution rules that are embodied in the parser, not here. + * See FuncnameGetCandidates's comments for more info. */ typedef struct _FuncCandidateList { @@ -30,7 +31,7 @@ typedef struct _FuncCandidateList Oid oid; /* the function or operator's OID */ int nargs; /* number of arg types returned */ int nvargs; /* number of args to become variadic array */ - List *argdefaults; /* list of parameter defaults */ + int ndargs; /* number of defaulted args */ Oid args[1]; /* arg types --- VARIABLE LENGTH ARRAY */ } *FuncCandidateList; /* VARIABLE LENGTH STRUCT */ @@ -54,7 +55,8 @@ extern Oid TypenameGetTypid(const char *typname); extern bool TypeIsVisible(Oid typid); extern FuncCandidateList FuncnameGetCandidates(List *names, int nargs, - bool expand_variadic); + bool expand_variadic, + bool expand_defaults); extern bool FunctionIsVisible(Oid funcid); extern Oid OpernameGetOprid(List *names, Oid oprleft, Oid oprright); diff --git a/src/include/catalog/pg_attribute.h b/src/include/catalog/pg_attribute.h index 27d207413b..18666339a4 100644 --- a/src/include/catalog/pg_attribute.h +++ b/src/include/catalog/pg_attribute.h @@ -8,7 +8,7 @@ * Portions Copyright (c) 1996-2008, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $PostgreSQL: pgsql/src/include/catalog/pg_attribute.h,v 1.142 2008/12/04 17:51:27 petere Exp $ + * $PostgreSQL: pgsql/src/include/catalog/pg_attribute.h,v 1.143 2008/12/18 18:20:34 tgl Exp $ * * NOTES * the genbki.sh script reads this file and generates .bki @@ -300,7 +300,7 @@ DATA(insert ( 1247 tableoid 26 0 4 -7 0 -1 -1 t p i t f f t 0)); { 1255, {"proretset"}, 16, -1, 1, 11, 0, -1, -1, true, 'p', 'c', true, false, false, true, 0 }, \ { 1255, {"provolatile"}, 18, -1, 1, 12, 0, -1, -1, true, 'p', 'c', true, false, false, true, 0 }, \ { 1255, {"pronargs"}, 21, -1, 2, 13, 0, -1, -1, true, 'p', 's', true, false, false, true, 0 }, \ -{ 1255, {"pronargdefaults"}, 21, -1, 2, 14, 0, -1, -1, true, 'p', 's', true, false, false, true, 0 }, \ +{ 1255, {"pronargdefaults"}, 21, -1, 2, 14, 0, -1, -1, true, 'p', 's', true, false, false, true, 0 }, \ { 1255, {"prorettype"}, 26, -1, 4, 15, 0, -1, -1, true, 'p', 'i', true, false, false, true, 0 }, \ { 1255, {"proargtypes"}, 30, -1, -1, 16, 1, -1, -1, false, 'p', 'i', true, false, false, true, 0 }, \ { 1255, {"proallargtypes"}, 1028, -1, -1, 17, 1, -1, -1, false, 'x', 'i', false, false, false, true, 0 }, \ diff --git a/src/include/catalog/pg_proc.h b/src/include/catalog/pg_proc.h index 7acc570664..9d4e6dcfff 100644 --- a/src/include/catalog/pg_proc.h +++ b/src/include/catalog/pg_proc.h @@ -7,7 +7,7 @@ * Portions Copyright (c) 1996-2008, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $PostgreSQL: pgsql/src/include/catalog/pg_proc.h,v 1.530 2008/12/04 17:51:27 petere Exp $ + * $PostgreSQL: pgsql/src/include/catalog/pg_proc.h,v 1.531 2008/12/18 18:20:34 tgl Exp $ * * NOTES * The script catalog/genbki.sh reads this file and generates .bki @@ -47,14 +47,16 @@ CATALOG(pg_proc,1255) BKI_BOOTSTRAP bool proretset; /* returns a set? */ char provolatile; /* see PROVOLATILE_ categories below */ int2 pronargs; /* number of arguments */ - int2 pronargdefaults;/* number of arguments defaults */ + int2 pronargdefaults; /* number of arguments with defaults */ Oid prorettype; /* OID of result type */ + /* VARIABLE LENGTH FIELDS: */ oidvector proargtypes; /* parameter types (excludes OUT params) */ Oid proallargtypes[1]; /* all param types (NULL if IN only) */ char proargmodes[1]; /* parameter modes (NULL if IN only) */ text proargnames[1]; /* parameter names (NULL if no names) */ - text proargdefaults; /* list of argument defaults */ + text proargdefaults; /* list of expression trees for argument + * defaults (NULL if none) */ text prosrc; /* procedure source text */ bytea probin; /* secondary procedure info (can be NULL) */ text proconfig[1]; /* procedure-local GUC settings */ @@ -2886,11 +2888,11 @@ DATA(insert OID = 1911 ( int2up PGNSP PGUID 12 1 0 0 f f t f i 1 0 21 "21" DESCR("unary plus"); DATA(insert OID = 1912 ( int4up PGNSP PGUID 12 1 0 0 f f t f i 1 0 23 "23" _null_ _null_ _null_ _null_ int4up _null_ _null_ _null_ )); DESCR("unary plus"); -DATA(insert OID = 1913 ( float4up PGNSP PGUID 12 1 0 0 f f t f i 1 0 700 "700" _null_ _null_ _null_ _null_ float4up _null_ _null_ _null_ )); +DATA(insert OID = 1913 ( float4up PGNSP PGUID 12 1 0 0 f f t f i 1 0 700 "700" _null_ _null_ _null_ _null_ float4up _null_ _null_ _null_ )); DESCR("unary plus"); -DATA(insert OID = 1914 ( float8up PGNSP PGUID 12 1 0 0 f f t f i 1 0 701 "701" _null_ _null_ _null_ _null_ float8up _null_ _null_ _null_ )); +DATA(insert OID = 1914 ( float8up PGNSP PGUID 12 1 0 0 f f t f i 1 0 701 "701" _null_ _null_ _null_ _null_ float8up _null_ _null_ _null_ )); DESCR("unary plus"); -DATA(insert OID = 1915 ( numeric_uplus PGNSP PGUID 12 1 0 0 f f t f i 1 0 1700 "1700" _null_ _null_ _null_ _null_ numeric_uplus _null_ _null_ _null_ )); +DATA(insert OID = 1915 ( numeric_uplus PGNSP PGUID 12 1 0 0 f f t f i 1 0 1700 "1700" _null_ _null_ _null_ _null_ numeric_uplus _null_ _null_ _null_ )); DESCR("unary plus"); DATA(insert OID = 1922 ( has_table_privilege PGNSP PGUID 12 1 0 0 f f t f s 3 0 16 "19 25 25" _null_ _null_ _null_ _null_ has_table_privilege_name_name _null_ _null_ _null_ )); @@ -3991,7 +3993,7 @@ DATA(insert OID = 2510 ( pg_prepared_statement PGNSP PGUID 12 1 1000 0 f f t t DESCR("get the prepared statements for this session"); DATA(insert OID = 2511 ( pg_cursor PGNSP PGUID 12 1 1000 0 f f t t s 0 0 2249 "" "{25,25,16,16,16,1184}" "{o,o,o,o,o,o}" "{name,statement,is_holdable,is_binary,is_scrollable,creation_time}" _null_ pg_cursor _null_ _null_ _null_ )); DESCR("get the open cursors for this session"); -DATA(insert OID = 2599 ( pg_timezone_abbrevs PGNSP PGUID 12 1 1000 0 f f t t s 0 0 2249 "" "{25,1186,16}" "{o,o,o}" "{abbrev,utc_offset,is_dst}" _null_ pg_timezone_abbrevs _null_ _null_ _null_ )); +DATA(insert OID = 2599 ( pg_timezone_abbrevs PGNSP PGUID 12 1 1000 0 f f t t s 0 0 2249 "" "{25,1186,16}" "{o,o,o}" "{abbrev,utc_offset,is_dst}" _null_ pg_timezone_abbrevs _null_ _null_ _null_ )); DESCR("get the available time zone abbreviations"); DATA(insert OID = 2856 ( pg_timezone_names PGNSP PGUID 12 1 1000 0 f f t t s 0 0 2249 "" "{25,25,1186,16}" "{o,o,o,o}" "{name,abbrev,utc_offset,is_dst}" _null_ pg_timezone_names _null_ _null_ _null_ )); DESCR("get the available time zone names"); diff --git a/src/include/catalog/pg_proc_fn.h b/src/include/catalog/pg_proc_fn.h index 82206749f0..cab31d45e1 100644 --- a/src/include/catalog/pg_proc_fn.h +++ b/src/include/catalog/pg_proc_fn.h @@ -7,7 +7,7 @@ * Portions Copyright (c) 1996-2008, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $PostgreSQL: pgsql/src/include/catalog/pg_proc_fn.h,v 1.3 2008/12/12 22:56:00 alvherre Exp $ + * $PostgreSQL: pgsql/src/include/catalog/pg_proc_fn.h,v 1.4 2008/12/18 18:20:35 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -33,10 +33,10 @@ extern Oid ProcedureCreate(const char *procedureName, Datum allParameterTypes, Datum parameterModes, Datum parameterNames, + List *parameterDefaults, Datum proconfig, float4 procost, - float4 prorows, - List *parameterDefaults); + float4 prorows); extern bool function_parse_error_transpose(const char *prosrc); diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h index 47e3f37206..c84a77585f 100644 --- a/src/include/nodes/parsenodes.h +++ b/src/include/nodes/parsenodes.h @@ -13,7 +13,7 @@ * Portions Copyright (c) 1996-2008, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $PostgreSQL: pgsql/src/include/nodes/parsenodes.h,v 1.382 2008/12/06 23:22:46 momjian Exp $ + * $PostgreSQL: pgsql/src/include/nodes/parsenodes.h,v 1.383 2008/12/18 18:20:35 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -1677,7 +1677,7 @@ typedef enum FunctionParameterMode FUNC_PARAM_IN = 'i', /* input only */ FUNC_PARAM_OUT = 'o', /* output only */ FUNC_PARAM_INOUT = 'b', /* both */ - FUNC_PARAM_VARIADIC = 'v', /* variadic */ + FUNC_PARAM_VARIADIC = 'v', /* variadic (always input) */ FUNC_PARAM_TABLE = 't' /* table function output column */ } FunctionParameterMode; @@ -1686,8 +1686,8 @@ typedef struct FunctionParameter NodeTag type; char *name; /* parameter name, or NULL if not given */ TypeName *argType; /* TypeName for parameter type */ - FunctionParameterMode mode; /* IN/OUT/INOUT */ - Node *defexpr; /* Default expression, or NULL if not given */ + FunctionParameterMode mode; /* IN/OUT/etc */ + Node *defexpr; /* raw default expr, or NULL if not given */ } FunctionParameter; typedef struct AlterFunctionStmt diff --git a/src/include/parser/parse_func.h b/src/include/parser/parse_func.h index a8f2de16ee..8507a4ed7d 100644 --- a/src/include/parser/parse_func.h +++ b/src/include/parser/parse_func.h @@ -7,7 +7,7 @@ * Portions Copyright (c) 1996-2008, PostgreSQL Global Development Group * Portions Copyright (c) 1994, Regents of the University of California * - * $PostgreSQL: pgsql/src/include/parser/parse_func.h,v 1.61 2008/12/04 17:51:27 petere Exp $ + * $PostgreSQL: pgsql/src/include/parser/parse_func.h,v 1.62 2008/12/18 18:20:35 tgl Exp $ * *------------------------------------------------------------------------- */ @@ -47,7 +47,8 @@ extern Node *ParseFuncOrColumn(ParseState *pstate, bool is_column, int location); extern FuncDetailCode func_get_detail(List *funcname, List *fargs, - int nargs, Oid *argtypes, bool expand_variadic, + int nargs, Oid *argtypes, + bool expand_variadic, bool expand_defaults, Oid *funcid, Oid *rettype, bool *retset, int *nvargs, Oid **true_typeids, List **argdefaults); diff --git a/src/test/regress/expected/opr_sanity.out b/src/test/regress/expected/opr_sanity.out index f1143fe16d..fbec4d696c 100644 --- a/src/test/regress/expected/opr_sanity.out +++ b/src/test/regress/expected/opr_sanity.out @@ -44,6 +44,8 @@ SELECT p1.oid, p1.proname FROM pg_proc as p1 WHERE p1.prolang = 0 OR p1.prorettype = 0 OR p1.pronargs < 0 OR + p1.pronargdefaults < 0 OR + p1.pronargdefaults > p1.pronargs OR array_lower(p1.proargtypes, 1) != 0 OR array_upper(p1.proargtypes, 1) != p1.pronargs-1 OR 0::oid = ANY (p1.proargtypes) OR @@ -62,9 +64,9 @@ WHERE prosrc IS NULL OR prosrc = '' OR prosrc = '-'; (0 rows) -- pronargdefaults should be 0 iff proargdefaults is null -SELECT p.oid, p.proname -FROM pg_proc AS p -WHERE pronargdefaults <> 0 OR proargdefaults IS NOT NULL; +SELECT p1.oid, p1.proname +FROM pg_proc AS p1 +WHERE (pronargdefaults <> 0) != (proargdefaults IS NOT NULL); oid | proname -----+--------- (0 rows) diff --git a/src/test/regress/expected/polymorphism.out b/src/test/regress/expected/polymorphism.out index 69e0ea4746..6fafc0343c 100644 --- a/src/test/regress/expected/polymorphism.out +++ b/src/test/regress/expected/polymorphism.out @@ -787,7 +787,7 @@ select pg_typeof(myleast(10, 1, 20, 33)); -- polymorphic input integer (1 row) --- test functions with parameter defaults +-- test functions with default parameters -- test basic functionality create function dfunc(a int = 1, int = 2) returns int as $$ select $1 + $2; @@ -810,17 +810,41 @@ select dfunc(10, 20); 30 (1 row) +select dfunc(10, 20, 30); -- fail +ERROR: function dfunc(integer, integer, integer) does not exist +LINE 1: select dfunc(10, 20, 30); + ^ +HINT: No function matches the given name and argument types. You might need to add explicit type casts. drop function dfunc(); -- fail ERROR: function dfunc() does not exist drop function dfunc(int); -- fail ERROR: function dfunc(integer) does not exist drop function dfunc(int, int); -- ok --- fail, gap in arguments with defaults +-- fail: defaults must be at end of argument list create function dfunc(a int = 1, b int) returns int as $$ select $1 + $2; $$ language sql; -ERROR: parameter without default value specified after parameter with default value --- check implicit coercion +ERROR: input parameters after one with a default value must also have defaults +-- however, this should work: +create function dfunc(a int = 1, out sum int, b int = 2) as $$ + select $1 + $2; +$$ language sql; +select dfunc(); + dfunc +------- + 3 +(1 row) + +-- verify it lists properly +\df dfunc + List of functions + Schema | Name | Result data type | Argument data types +--------+-------+------------------+----------------------------------------------------------- + public | dfunc | integer | a integer DEFAULT 1, OUT sum integer, b integer DEFAULT 2 +(1 row) + +drop function dfunc(int, int); +-- check implicit coercion create function dfunc(a int DEFAULT 1.0, int DEFAULT '-1') returns int as $$ select $1 + $2; $$ language sql; @@ -833,8 +857,11 @@ select dfunc(); create function dfunc(a text DEFAULT 'Hello', b text DEFAULT 'World') returns text as $$ select $1 || ', ' || $2; $$ language sql; -select dfunc(); -- fail; which dfunc should be called? int or text -ERROR: functions with parameter defaults dfunc(text, text) and dfunc(integer, integer) are ambiguous +select dfunc(); -- fail: which dfunc should be called? int or text +ERROR: function dfunc() is not unique +LINE 1: select dfunc(); + ^ +HINT: Could not choose a best candidate function. You might need to add explicit type casts. select dfunc('Hi'); -- ok dfunc ----------- @@ -862,24 +889,28 @@ select dfunc(10, 20); -- ok drop function dfunc(int, int); drop function dfunc(text, text); create function dfunc(int = 1, int = 2) returns int as $$ - select 2; + select 2; $$ language sql; create function dfunc(int = 1, int = 2, int = 3, int = 4) returns int as $$ select 4; $$ language sql; -- Now, dfunc(nargs = 2) and dfunc(nargs = 4) are ambiguous when called --- with 0 or 1 arguments. For 2 arguments, a normall call of --- dfunc(nargs = 2) takes place. +-- with 0 to 2 arguments. select dfunc(); -- fail -ERROR: functions with parameter defaults dfunc(integer, integer, integer, integer) and dfunc(integer, integer) are ambiguous +ERROR: function dfunc() is not unique +LINE 1: select dfunc(); + ^ +HINT: Could not choose a best candidate function. You might need to add explicit type casts. select dfunc(1); -- fail -ERROR: functions with parameter defaults dfunc(integer, integer, integer, integer) and dfunc(integer, integer) are ambiguous -select dfunc(1, 2); -- ok - dfunc -------- - 2 -(1 row) - +ERROR: function dfunc(integer) is not unique +LINE 1: select dfunc(1); + ^ +HINT: Could not choose a best candidate function. You might need to add explicit type casts. +select dfunc(1, 2); -- fail +ERROR: function dfunc(integer, integer) is not unique +LINE 1: select dfunc(1, 2); + ^ +HINT: Could not choose a best candidate function. You might need to add explicit type casts. select dfunc(1, 2, 3); -- ok dfunc ------- @@ -896,9 +927,9 @@ drop function dfunc(int, int); drop function dfunc(int, int, int, int); -- default values are not allowed for output parameters create function dfunc(out int = 20) returns int as $$ - select 1; + select 1; $$ language sql; -ERROR: only IN and INOUT parameters can have default values +ERROR: only input parameters can have default values -- polymorphic parameter test create function dfunc(anyelement = 'World'::text) returns text as $$ select 'Hello, ' || $1::text; @@ -928,49 +959,82 @@ select dfunc('City'::text); (1 row) drop function dfunc(anyelement); --- check null values -create function dfunc(int = null, int = null, int = null) returns int[] as $$ - select array[$1, $2, $3]; -$$ language sql; -select dfunc(1); - dfunc ---------------- - {1,NULL,NULL} +-- check defaults for variadics +create function dfunc(a variadic int[]) returns int as +$$ select array_upper($1, 1) $$ language sql; +select dfunc(); -- fail +ERROR: function dfunc() does not exist +LINE 1: select dfunc(); + ^ +HINT: No function matches the given name and argument types. You might need to add explicit type casts. +select dfunc(10); + dfunc +------- + 1 (1 row) -select dfunc(1, 2); - dfunc ------------- - {1,2,NULL} +select dfunc(10,20); + dfunc +------- + 2 (1 row) -select dfunc(1, 2, 3); - dfunc ---------- - {1,2,3} +create or replace function dfunc(a variadic int[] default array[]::int[]) returns int as +$$ select array_upper($1, 1) $$ language sql; +select dfunc(); -- now ok + dfunc +------- + (1 row) -drop function dfunc(int, int, int); --- The conflict detection algorithm doesn't consider the actual parameter --- types. It detects any possible conflict for n arguments for some --- function. This is unwanted behavior, but solving it needs a move of --- coercion routines. +select dfunc(10); + dfunc +------- + 1 +(1 row) + +select dfunc(10,20); + dfunc +------- + 2 +(1 row) + +-- can't remove the default once it exists +create or replace function dfunc(a variadic int[]) returns int as +$$ select array_upper($1, 1) $$ language sql; +ERROR: cannot remove parameter defaults from existing function +HINT: Use DROP FUNCTION first. +\df dfunc + List of functions + Schema | Name | Result data type | Argument data types +--------+-------+------------------+-------------------------------------- + public | dfunc | integer | VARIADIC a integer[] DEFAULT ARRAY[] +(1 row) + +drop function dfunc(a variadic int[]); +-- Ambiguity should be reported only if there's not a better match available create function dfunc(int = 1, int = 2, int = 3) returns int as $$ select 3; $$ language sql; create function dfunc(int = 1, int = 2) returns int as $$ select 2; $$ language sql; --- for n = 1 dfunc(narg=2) and dfunc(narg=3) are ambiguous -select dfunc(1); -- fail -ERROR: functions with parameter defaults dfunc(integer, integer, integer) and dfunc(integer, integer) are ambiguous create function dfunc(text) returns text as $$ select $1; $$ language sql; --- Will fail, it detects ambiguity between dfunc(int, int, int) and --- dfunc(int, int), but dfunc(text) isn't in conflict with either. +-- dfunc(narg=2) and dfunc(narg=3) are ambiguous +select dfunc(1); -- fail +ERROR: function dfunc(integer) is not unique +LINE 1: select dfunc(1); + ^ +HINT: Could not choose a best candidate function. You might need to add explicit type casts. +-- but this works since the ambiguous functions aren't preferred anyway select dfunc('Hi'); -ERROR: functions with parameter defaults dfunc(integer, integer, integer) and dfunc(integer, integer) are ambiguous + dfunc +------- + Hi +(1 row) + drop function dfunc(int, int, int); drop function dfunc(int, int); drop function dfunc(text); diff --git a/src/test/regress/sql/opr_sanity.sql b/src/test/regress/sql/opr_sanity.sql index 2eb56a2230..5da297a875 100644 --- a/src/test/regress/sql/opr_sanity.sql +++ b/src/test/regress/sql/opr_sanity.sql @@ -51,6 +51,8 @@ SELECT p1.oid, p1.proname FROM pg_proc as p1 WHERE p1.prolang = 0 OR p1.prorettype = 0 OR p1.pronargs < 0 OR + p1.pronargdefaults < 0 OR + p1.pronargdefaults > p1.pronargs OR array_lower(p1.proargtypes, 1) != 0 OR array_upper(p1.proargtypes, 1) != p1.pronargs-1 OR 0::oid = ANY (p1.proargtypes) OR @@ -63,9 +65,9 @@ FROM pg_proc as p1 WHERE prosrc IS NULL OR prosrc = '' OR prosrc = '-'; -- pronargdefaults should be 0 iff proargdefaults is null -SELECT p.oid, p.proname -FROM pg_proc AS p -WHERE pronargdefaults <> 0 OR proargdefaults IS NOT NULL; +SELECT p1.oid, p1.proname +FROM pg_proc AS p1 +WHERE (pronargdefaults <> 0) != (proargdefaults IS NOT NULL); -- probin should be non-empty for C functions, null everywhere else SELECT p1.oid, p1.proname diff --git a/src/test/regress/sql/polymorphism.sql b/src/test/regress/sql/polymorphism.sql index 6c7b1813f9..bbac7d11a5 100644 --- a/src/test/regress/sql/polymorphism.sql +++ b/src/test/regress/sql/polymorphism.sql @@ -488,7 +488,8 @@ select pg_typeof(pg_typeof(0)); -- regtype select pg_typeof(array[1.2,55.5]); -- numeric[] select pg_typeof(myleast(10, 1, 20, 33)); -- polymorphic input --- test functions with parameter defaults +-- test functions with default parameters + -- test basic functionality create function dfunc(a int = 1, int = 2) returns int as $$ select $1 + $2; @@ -497,26 +498,40 @@ $$ language sql; select dfunc(); select dfunc(10); select dfunc(10, 20); +select dfunc(10, 20, 30); -- fail drop function dfunc(); -- fail drop function dfunc(int); -- fail drop function dfunc(int, int); -- ok --- fail, gap in arguments with defaults +-- fail: defaults must be at end of argument list create function dfunc(a int = 1, b int) returns int as $$ select $1 + $2; $$ language sql; --- check implicit coercion +-- however, this should work: +create function dfunc(a int = 1, out sum int, b int = 2) as $$ + select $1 + $2; +$$ language sql; + +select dfunc(); + +-- verify it lists properly +\df dfunc + +drop function dfunc(int, int); + +-- check implicit coercion create function dfunc(a int DEFAULT 1.0, int DEFAULT '-1') returns int as $$ select $1 + $2; $$ language sql; select dfunc(); + create function dfunc(a text DEFAULT 'Hello', b text DEFAULT 'World') returns text as $$ select $1 || ', ' || $2; $$ language sql; -select dfunc(); -- fail; which dfunc should be called? int or text +select dfunc(); -- fail: which dfunc should be called? int or text select dfunc('Hi'); -- ok select dfunc('Hi', 'City'); -- ok select dfunc(0); -- ok @@ -526,7 +541,7 @@ drop function dfunc(int, int); drop function dfunc(text, text); create function dfunc(int = 1, int = 2) returns int as $$ - select 2; + select 2; $$ language sql; create function dfunc(int = 1, int = 2, int = 3, int = 4) returns int as $$ @@ -534,12 +549,11 @@ create function dfunc(int = 1, int = 2, int = 3, int = 4) returns int as $$ $$ language sql; -- Now, dfunc(nargs = 2) and dfunc(nargs = 4) are ambiguous when called --- with 0 or 1 arguments. For 2 arguments, a normall call of --- dfunc(nargs = 2) takes place. +-- with 0 to 2 arguments. select dfunc(); -- fail select dfunc(1); -- fail -select dfunc(1, 2); -- ok +select dfunc(1, 2); -- fail select dfunc(1, 2, 3); -- ok select dfunc(1, 2, 3, 4); -- ok @@ -548,7 +562,7 @@ drop function dfunc(int, int, int, int); -- default values are not allowed for output parameters create function dfunc(out int = 20) returns int as $$ - select 1; + select 1; $$ language sql; -- polymorphic parameter test @@ -563,21 +577,31 @@ select dfunc('City'::text); drop function dfunc(anyelement); --- check null values -create function dfunc(int = null, int = null, int = null) returns int[] as $$ - select array[$1, $2, $3]; -$$ language sql; +-- check defaults for variadics -select dfunc(1); -select dfunc(1, 2); -select dfunc(1, 2, 3); +create function dfunc(a variadic int[]) returns int as +$$ select array_upper($1, 1) $$ language sql; -drop function dfunc(int, int, int); +select dfunc(); -- fail +select dfunc(10); +select dfunc(10,20); --- The conflict detection algorithm doesn't consider the actual parameter --- types. It detects any possible conflict for n arguments for some --- function. This is unwanted behavior, but solving it needs a move of --- coercion routines. +create or replace function dfunc(a variadic int[] default array[]::int[]) returns int as +$$ select array_upper($1, 1) $$ language sql; + +select dfunc(); -- now ok +select dfunc(10); +select dfunc(10,20); + +-- can't remove the default once it exists +create or replace function dfunc(a variadic int[]) returns int as +$$ select array_upper($1, 1) $$ language sql; + +\df dfunc + +drop function dfunc(a variadic int[]); + +-- Ambiguity should be reported only if there's not a better match available create function dfunc(int = 1, int = 2, int = 3) returns int as $$ select 3; @@ -587,15 +611,14 @@ create function dfunc(int = 1, int = 2) returns int as $$ select 2; $$ language sql; --- for n = 1 dfunc(narg=2) and dfunc(narg=3) are ambiguous -select dfunc(1); -- fail - create function dfunc(text) returns text as $$ select $1; $$ language sql; --- Will fail, it detects ambiguity between dfunc(int, int, int) and --- dfunc(int, int), but dfunc(text) isn't in conflict with either. +-- dfunc(narg=2) and dfunc(narg=3) are ambiguous +select dfunc(1); -- fail + +-- but this works since the ambiguous functions aren't preferred anyway select dfunc('Hi'); drop function dfunc(int, int, int);