On 19/01/12 20:28, Hitoshi Harada wrote: >> (Now it occurred to me that forgetting the #include parse_func.h might >> hit this breakage..., so I'll fix it here and continue to test, but if >> you'll fix it yourself, let me know) > > I fixed it here and it now works with my environment.
Well spotted; that's exactly what I'd done. :/ > The regression tests pass, the feature seems working as aimed, but it > seems to me that it needs more test cases and documentation. For the > tests, I believe at least we need "ambiguous" case given upthread, so > that we can ensure to keep compatibility. For the document, it should > describe the name resolution rule, as stated in the patch comment. Attached are a new pair of patches, fixing the missing include (and the other warning), plus addressing the above. I'm still not sure whether to just revise (almost) all the SQL function examples to use parameter names, and declare them the "right" choice; as it's currently written, named parameters still seem rather second-class. > Aside from them, I wondered at first what if the function is > schema-qualified. Say, > > CREATE FUNCTION s.f(a int) RETURNS int AS $$ > SELECT b FROM t WHERE a = s.f.a > $$ LANGUAGE sql; > > It actually errors out, since function-name-qualified parameter only > accepts function name without schema name, but it looked weird to me > at first. No better idea from me at the moment, though. By my reading of (a draft of) the spec, Subclause 6.6, "<identifier chain>", Syntax Rules 8.b.i-iii, the current behaviour is correct. But I join you in wondering whether we should permit the function name to be schema-qualified anyway. Matthew -- matt...@trebex.net
diff --git a/doc/src/sgml/xfunc.sgml b/doc/src/sgml/xfunc.sgml new file mode 100644 index 7064312..cc5b5ef *** a/doc/src/sgml/xfunc.sgml --- b/doc/src/sgml/xfunc.sgml *************** SELECT getname(new_emp()); *** 538,556 **** <programlisting> CREATE FUNCTION tf1 (acct_no integer, debit numeric) RETURNS numeric AS $$ UPDATE bank ! SET balance = balance - $2 ! WHERE accountno = $1 RETURNING balance; $$ LANGUAGE SQL; </programlisting> Here the first parameter has been given the name <literal>acct_no</>, and the second parameter the name <literal>debit</>. ! So far as the SQL function itself is concerned, these names are just ! decoration; you must still refer to the parameters as <literal>$1</>, ! <literal>$2</>, etc within the function body. (Some procedural ! languages let you use the parameter names instead.) However, ! attaching names to the parameters is useful for documentation purposes. When a function has many parameters, it is also useful to use the names while calling the function, as described in <xref linkend="sql-syntax-calling-funcs">. --- 538,580 ---- <programlisting> CREATE FUNCTION tf1 (acct_no integer, debit numeric) RETURNS numeric AS $$ UPDATE bank ! SET balance = balance - debit ! WHERE accountno = acct_no RETURNING balance; $$ LANGUAGE SQL; </programlisting> Here the first parameter has been given the name <literal>acct_no</>, and the second parameter the name <literal>debit</>. ! Named parameters can still be referenced as ! <literal>$<replaceable>n</></>; in this example, the second ! parameter can be referenced as <literal>$2</>, <literal>debit</>, ! or <literal>tf1.debit</>. ! </para> ! ! <para> ! If a parameter is given the same name as a column that is available ! in the query, the name will refer to the column. To explicitly ! refer to the parameter, you can qualify its name with the name of ! the containing function. For example, ! ! <programlisting> ! CREATE FUNCTION tf1 (accountno integer, debit numeric) RETURNS numeric AS $$ ! UPDATE bank ! SET balance = balance - debit ! WHERE accountno = tf1.accountno ! RETURNING balance; ! $$ LANGUAGE SQL; ! </programlisting> ! ! This time, the first parameter has been given the ambiguous name ! <literal>accountno</>. ! Notice that inside the function body, <literal>accountno</> still ! refers to <literal>bank.accountno</>, so <literal>tf1.accountno</> ! must be used to refer to the parameter. ! </para> ! ! <para> When a function has many parameters, it is also useful to use the names while calling the function, as described in <xref linkend="sql-syntax-calling-funcs">.
diff --git a/src/backend/executor/functions.c b/src/backend/executor/functions.c new file mode 100644 index 5642687..fe87990 *** a/src/backend/executor/functions.c --- b/src/backend/executor/functions.c *************** *** 23,28 **** --- 23,29 ---- #include "nodes/makefuncs.h" #include "nodes/nodeFuncs.h" #include "parser/parse_coerce.h" + #include "parser/parse_func.h" #include "tcop/utility.h" #include "utils/builtins.h" #include "utils/datum.h" *************** typedef SQLFunctionCache *SQLFunctionCac *** 115,121 **** --- 116,124 ---- */ typedef struct SQLFunctionParseInfo { + char *name; /* function's name */ Oid *argtypes; /* resolved types of input arguments */ + char **argnames; /* names of input arguments */ int nargs; /* number of input arguments */ Oid collation; /* function's input collation, if known */ } SQLFunctionParseInfo; *************** typedef struct SQLFunctionParseInfo *** 123,128 **** --- 126,133 ---- /* non-export function prototypes */ static Node *sql_fn_param_ref(ParseState *pstate, ParamRef *pref); + static Node *sql_fn_post_column_ref(ParseState *pstate, ColumnRef *cref, Node *var); + static Node *sql_fn_param_ref_num(ParseState *pstate, SQLFunctionParseInfoPtr pinfo, int paramno, int location); static List *init_execution_state(List *queryTree_list, SQLFunctionCachePtr fcache, bool lazyEvalOK); *************** prepare_sql_fn_parse_info(HeapTuple proc *** 162,167 **** --- 167,173 ---- int nargs; pinfo = (SQLFunctionParseInfoPtr) palloc0(sizeof(SQLFunctionParseInfo)); + pinfo->name = NameStr(procedureStruct->proname); /* Save the function's input collation */ pinfo->collation = inputCollation; *************** prepare_sql_fn_parse_info(HeapTuple proc *** 200,205 **** --- 206,240 ---- pinfo->argtypes = argOidVect; } + if (nargs > 0) + { + Datum proargnames; + Datum proargmodes; + int n_arg_names; + bool isNull; + + proargnames = SysCacheGetAttr(PROCNAMEARGSNSP, procedureTuple, + Anum_pg_proc_proargnames, + &isNull); + if (isNull) + proargnames = PointerGetDatum(NULL); /* just to be sure */ + + proargmodes = SysCacheGetAttr(PROCNAMEARGSNSP, procedureTuple, + Anum_pg_proc_proargmodes, + &isNull); + if (isNull) + proargmodes = PointerGetDatum(NULL); /* just to be sure */ + + n_arg_names = get_func_input_arg_names(proargnames, proargmodes, &pinfo->argnames); + + if (n_arg_names < nargs) + pinfo->argnames = NULL; + } + else + { + pinfo->argnames = NULL; + } + return pinfo; } *************** prepare_sql_fn_parse_info(HeapTuple proc *** 209,222 **** void sql_fn_parser_setup(struct ParseState *pstate, SQLFunctionParseInfoPtr pinfo) { - /* Later we might use these hooks to support parameter names */ pstate->p_pre_columnref_hook = NULL; ! pstate->p_post_columnref_hook = NULL; pstate->p_paramref_hook = sql_fn_param_ref; /* no need to use p_coerce_param_hook */ pstate->p_ref_hook_state = (void *) pinfo; } /* * sql_fn_param_ref parser callback for ParamRefs ($n symbols) */ --- 244,353 ---- void sql_fn_parser_setup(struct ParseState *pstate, SQLFunctionParseInfoPtr pinfo) { pstate->p_pre_columnref_hook = NULL; ! pstate->p_post_columnref_hook = sql_fn_post_column_ref; pstate->p_paramref_hook = sql_fn_param_ref; /* no need to use p_coerce_param_hook */ pstate->p_ref_hook_state = (void *) pinfo; } + static Node * + sql_fn_resolve_name(ParseState *pstate, SQLFunctionParseInfoPtr pinfo, const char *paramname, int location) + { + int i; + for (i = 0; i < pinfo->nargs; i++) + if (pinfo->argnames[i] && strcmp(pinfo->argnames[i], paramname) == 0) + return sql_fn_param_ref_num(pstate, pinfo, i + 1, location); + + return NULL; + } + + /* + * sql_fn_post_column_ref parser callback for ColumnRefs + */ + static Node * + sql_fn_post_column_ref(ParseState *pstate, ColumnRef *cref, Node *var) + { + SQLFunctionParseInfoPtr pinfo = (SQLFunctionParseInfoPtr) pstate->p_ref_hook_state; + int names; + Node *field1; + Node *subfield = NULL; + const char *pname; + Node *param; + + if (var != NULL) + return NULL; /* there's a table column, prefer that */ + + /* + * The allowed syntaxes are: + * + * A A = parameter name + * A.B A = (record-typed) parameter name, B = field reference, + * OR A = function name, B = parameter name + * A.B.C A = function name, B = (record-typed) parameter name, + * C = field reference + */ + names = list_length(cref->fields); + + if (names > 3) + return NULL; + + field1 = (Node *) linitial(cref->fields); + if (names > 1) + subfield = (Node *) lsecond(cref->fields); + Assert(IsA(field1, String)); + pname = strVal(field1); + + if (names == 3) + { + /* + * Function-qualified reference: if the first name doesn't match + * the function, we can fail immediately. Otherwise, discard the + * first name, and continue. + */ + if (strcmp(pname, pinfo->name) != 0) + return NULL; + + Assert(IsA(subfield, String)); + pname = strVal(subfield); + param = sql_fn_resolve_name(pstate, pinfo, pname, cref->location); + subfield = (Node *) lthird(cref->fields); + } + else + { + param = sql_fn_resolve_name(pstate, pinfo, pname, cref->location); + + if (!param && names == 2 && strcmp(pname, pinfo->name) == 0) + { + /* + * We have a two-part name, the first part matches the name + * of our containing function, and did not match a + * parameter; discard the first name, and try again. + */ + Assert(IsA(subfield, String)); + pname = strVal(subfield); + param = sql_fn_resolve_name(pstate, pinfo, pname, cref->location); + subfield = NULL; + } + } + + if (!param) + return NULL; + + if (subfield) + { + Assert(IsA(subfield, String)); + + param = ParseFuncOrColumn(pstate, + list_make1(subfield), + list_make1(param), + NIL, false, false, false, + NULL, true, cref->location); + } + + return param; + } + /* * sql_fn_param_ref parser callback for ParamRefs ($n symbols) */ *************** sql_fn_param_ref(ParseState *pstate, Par *** 225,230 **** --- 356,371 ---- { SQLFunctionParseInfoPtr pinfo = (SQLFunctionParseInfoPtr) pstate->p_ref_hook_state; int paramno = pref->number; + + return sql_fn_param_ref_num(pstate, pinfo, paramno, pref->location); + } + + /* + * sql_fn_param_ref_num construct a Param node for the given paramno + */ + static Node * + sql_fn_param_ref_num(ParseState *pstate, SQLFunctionParseInfoPtr pinfo, int paramno, int location) + { Param *param; /* Check parameter number is valid */ *************** sql_fn_param_ref(ParseState *pstate, Par *** 237,243 **** param->paramtype = pinfo->argtypes[paramno - 1]; param->paramtypmod = -1; param->paramcollid = get_typcollation(param->paramtype); ! param->location = pref->location; /* * If we have a function input collation, allow it to override the --- 378,384 ---- param->paramtype = pinfo->argtypes[paramno - 1]; param->paramtypmod = -1; param->paramcollid = get_typcollation(param->paramtype); ! param->location = location; /* * If we have a function input collation, allow it to override the diff --git a/src/test/regress/input/create_function_2.source b/src/test/regress/input/create_function_2.source new file mode 100644 index 6aed5f0..1b013ae *** a/src/test/regress/input/create_function_2.source --- b/src/test/regress/input/create_function_2.source *************** CREATE FUNCTION hobby_construct(text, te *** 13,18 **** --- 13,24 ---- LANGUAGE SQL; + CREATE FUNCTION hobby_construct_named(name text, hobby text) + RETURNS hobbies_r + AS 'select name, hobby' + LANGUAGE SQL; + + CREATE FUNCTION hobbies_by_name(hobbies_r.name%TYPE) RETURNS hobbies_r.person%TYPE AS 'select person from hobbies_r where name = $1' *************** CREATE FUNCTION equipment(hobbies_r) *** 25,30 **** --- 31,67 ---- LANGUAGE SQL; + CREATE FUNCTION equipment_named(hobby hobbies_r) + RETURNS setof equipment_r + AS 'select * from equipment_r where equipment_r.hobby = equipment_named.hobby.name' + LANGUAGE SQL; + + CREATE FUNCTION equipment_named_ambiguous_1a(hobby hobbies_r) + RETURNS setof equipment_r + AS 'select * from equipment_r where hobby = equipment_named_ambiguous_1a.hobby.name' + LANGUAGE SQL; + + CREATE FUNCTION equipment_named_ambiguous_1b(hobby hobbies_r) + RETURNS setof equipment_r + AS 'select * from equipment_r where equipment_r.hobby = hobby.name' + LANGUAGE SQL; + + CREATE FUNCTION equipment_named_ambiguous_1c(hobby hobbies_r) + RETURNS setof equipment_r + AS 'select * from equipment_r where hobby = hobby.name' + LANGUAGE SQL; + + CREATE FUNCTION equipment_named_ambiguous_2a(hobby text) + RETURNS setof equipment_r + AS 'select * from equipment_r where hobby = equipment_named_ambiguous_2a.hobby' + LANGUAGE SQL; + + CREATE FUNCTION equipment_named_ambiguous_2b(hobby text) + RETURNS setof equipment_r + AS 'select * from equipment_r where equipment_r.hobby = hobby' + LANGUAGE SQL; + + CREATE FUNCTION user_relns() RETURNS setof name AS 'select relname diff --git a/src/test/regress/input/misc.source b/src/test/regress/input/misc.source new file mode 100644 index 7cd26cb..e16dc21 *** a/src/test/regress/input/misc.source --- b/src/test/regress/input/misc.source *************** SELECT user_relns() AS user_relns *** 223,228 **** --- 223,242 ---- SELECT name(equipment(hobby_construct(text 'skywalking', text 'mer'))); + SELECT name(equipment(hobby_construct_named(text 'skywalking', text 'mer'))); + + SELECT name(equipment_named(hobby_construct_named(text 'skywalking', text 'mer'))); + + SELECT name(equipment_named_ambiguous_1a(hobby_construct_named(text 'skywalking', text 'mer'))); + + SELECT name(equipment_named_ambiguous_1b(hobby_construct_named(text 'skywalking', text 'mer'))); + + SELECT name(equipment_named_ambiguous_1c(hobby_construct_named(text 'skywalking', text 'mer'))); + + SELECT name(equipment_named_ambiguous_2a(text 'skywalking')); + + SELECT name(equipment_named_ambiguous_2b(text 'skywalking')); + SELECT hobbies_by_name('basketball'); SELECT name, overpaid(emp.*) FROM emp; diff --git a/src/test/regress/output/create_function_2.source b/src/test/regress/output/create_function_2.source new file mode 100644 index 94ab7eb..98e1c29 *** a/src/test/regress/output/create_function_2.source --- b/src/test/regress/output/create_function_2.source *************** CREATE FUNCTION hobby_construct(text, te *** 9,14 **** --- 9,18 ---- RETURNS hobbies_r AS 'select $1 as name, $2 as hobby' LANGUAGE SQL; + CREATE FUNCTION hobby_construct_named(name text, hobby text) + RETURNS hobbies_r + AS 'select name, hobby' + LANGUAGE SQL; CREATE FUNCTION hobbies_by_name(hobbies_r.name%TYPE) RETURNS hobbies_r.person%TYPE AS 'select person from hobbies_r where name = $1' *************** CREATE FUNCTION equipment(hobbies_r) *** 19,24 **** --- 23,52 ---- RETURNS setof equipment_r AS 'select * from equipment_r where hobby = $1.name' LANGUAGE SQL; + CREATE FUNCTION equipment_named(hobby hobbies_r) + RETURNS setof equipment_r + AS 'select * from equipment_r where equipment_r.hobby = equipment_named.hobby.name' + LANGUAGE SQL; + CREATE FUNCTION equipment_named_ambiguous_1a(hobby hobbies_r) + RETURNS setof equipment_r + AS 'select * from equipment_r where hobby = equipment_named_ambiguous_1a.hobby.name' + LANGUAGE SQL; + CREATE FUNCTION equipment_named_ambiguous_1b(hobby hobbies_r) + RETURNS setof equipment_r + AS 'select * from equipment_r where equipment_r.hobby = hobby.name' + LANGUAGE SQL; + CREATE FUNCTION equipment_named_ambiguous_1c(hobby hobbies_r) + RETURNS setof equipment_r + AS 'select * from equipment_r where hobby = hobby.name' + LANGUAGE SQL; + CREATE FUNCTION equipment_named_ambiguous_2a(hobby text) + RETURNS setof equipment_r + AS 'select * from equipment_r where hobby = equipment_named_ambiguous_2a.hobby' + LANGUAGE SQL; + CREATE FUNCTION equipment_named_ambiguous_2b(hobby text) + RETURNS setof equipment_r + AS 'select * from equipment_r where equipment_r.hobby = hobby' + LANGUAGE SQL; CREATE FUNCTION user_relns() RETURNS setof name AS 'select relname diff --git a/src/test/regress/output/misc.source b/src/test/regress/output/misc.source new file mode 100644 index 2f4d482..979ed33 *** a/src/test/regress/output/misc.source --- b/src/test/regress/output/misc.source *************** SELECT name(equipment(hobby_construct(te *** 693,698 **** --- 693,743 ---- guts (1 row) + SELECT name(equipment(hobby_construct_named(text 'skywalking', text 'mer'))); + name + ------ + guts + (1 row) + + SELECT name(equipment_named(hobby_construct_named(text 'skywalking', text 'mer'))); + name + ------ + guts + (1 row) + + SELECT name(equipment_named_ambiguous_1a(hobby_construct_named(text 'skywalking', text 'mer'))); + name + ------ + guts + (1 row) + + SELECT name(equipment_named_ambiguous_1b(hobby_construct_named(text 'skywalking', text 'mer'))); + name + ------ + guts + (1 row) + + SELECT name(equipment_named_ambiguous_1c(hobby_construct_named(text 'skywalking', text 'mer'))); + name + ------ + guts + (1 row) + + SELECT name(equipment_named_ambiguous_2a(text 'skywalking')); + name + ------ + guts + (1 row) + + SELECT name(equipment_named_ambiguous_2b(text 'skywalking')); + name + --------------- + advil + peet's coffee + hightops + guts + (4 rows) + SELECT hobbies_by_name('basketball'); hobbies_by_name -----------------
-- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers