Attached is a WIP patch that allows SQL-language functions to reference their parameters by name.
It uses p_post_columnref_hook, so potentially ambiguous references prefer the column... that seems to make the most sense, both because it avoids a backwards incompatibility, and it conforms with SQL's usual notion of assuming you mean the "nearest" name. It allows the parameter name to be qualified with the function name, for when you really mean you want the parameter. This being my first foray into the PostgreSQL source, I expect the code is horribly wrong in a variety of ways. Other than that, the regression tests I've been using are a slight modification of existing queries; I imagine they should look measurably different. And finally, conspicuously absent are the documentation changes that will obviously need to accompany a real patch. (This builds & passes `make check` on current HEAD, a4425e3) Thanks! Matthew -- matt...@trebex.net
diff --git a/src/backend/executor/functions.c b/src/backend/executor/functions.c new file mode 100644 index ce3b77b..be71fbb *** a/src/backend/executor/functions.c --- b/src/backend/executor/functions.c *************** typedef SQLFunctionCache *SQLFunctionCac *** 116,122 **** --- 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 *** 124,129 **** --- 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 *** 163,168 **** --- 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 *** 201,206 **** --- 206,241 ---- pinfo->argtypes = argOidVect; } + if (nargs > 0) + { + Datum proargnames; + Datum proargmodes; + int argnum; + int n_arg_names; + bool isNull; + + proargnames = SysCacheGetAttr(PROCNAMEARGSNSP, procedureTuple, + Anum_pg_proc_proargnames, + &isNull); + if (isNull) + proargmodes = 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 *** 210,223 **** 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) */ --- 245,354 ---- 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 *** 226,231 **** --- 357,372 ---- { 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 *** 238,244 **** 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 --- 379,385 ---- 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..3207870 *** 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,42 ---- 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 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 0930a6a..958db67 *** a/src/test/regress/input/misc.source --- b/src/test/regress/input/misc.source *************** SELECT user_relns() AS user_relns *** 217,222 **** --- 217,226 ---- 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 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..e7ef281 *** 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,32 ---- 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 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 c225d0f..e1312e6 *** a/src/test/regress/output/misc.source --- b/src/test/regress/output/misc.source *************** SELECT name(equipment(hobby_construct(te *** 677,682 **** --- 677,694 ---- 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 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