I just remembered to make time to advance this from WIP to proposed
patch this week... and then worked out I'm rudely dropping it into the
last commitfest at the last minute. :/


Anyway, my interpretation of the previous discussion is a general
consensus that permitting ambiguous parameter/column references is
somewhat unsavoury, but better than the alternatives:

http://archives.postgresql.org/pgsql-hackers/2011-04/msg00433.php
http://archives.postgresql.org/pgsql-hackers/2011-04/msg00744.php
(and surrounds)


The attached patch is essentially unchanged from my WIP version; it's
updated to current master (d0dcb31), and fixes a trivial copy/paste
error. It passes `make check`.

Also attached is a rather light doc patch, which I've separated because
I'm hesitant about which approach to take. The attached version just
changes the existing mention of naming parameters in:

http://www.postgresql.org/docs/9.1/static/xfunc-sql.html#XFUNC-NAMED-PARAMETERS

It presumably ought to be clearer about the name resolution
priorities... in a quick look, I failed to locate the corresponding
discussion of column name references to link to (beyond a terse sentence
in 4.2.1).

The alternative would be to adjust most of the examples in section 35.4,
using parameter names as the preferred option, and thus make $n "the
other way".

I'm happy to do that, but I figure it'd be a bit presumptuous to present
such a patch without some discussion; it's effectively rewriting the
project's opinion of how to reference function parameters.



With regard to the discussion about aliasing the function name when used
as a qualifier
(http://archives.postgresql.org/pgsql-hackers/2011-04/msg00871.php), my
only suggestion would be that using $0 (as in, '$0.paramname') appears
safe -- surely any spec change causing it issues would equally affect
the existing $1 etc. '$.paramname' seems to look better, but presumably
runs into trouble by looking like an operator.

That whole discussion seems above my pay grade, however.


Original WIP post:

http://archives.postgresql.org/pgsql-hackers/2011-03/msg01479.php

This is an open TODO:

http://wiki.postgresql.org/wiki/Todo#SQL-Language_Functions



I've just added the above post to the CF app; I'll update it to point to
this one once it appears.



Thanks!

Matthew


-- 
matt...@trebex.net
diff --git a/src/backend/executor/functions.c b/src/backend/executor/functions.c
new file mode 100644
index 5642687..74f3e7d
*** a/src/backend/executor/functions.c
--- b/src/backend/executor/functions.c
*************** typedef SQLFunctionCache *SQLFunctionCac
*** 115,121 ****
--- 115,123 ----
   */
  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 ****
--- 125,132 ----
  
  /* 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 ****
--- 166,172 ----
  	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 ****
--- 205,240 ----
  		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)
+ 			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..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 7cd26cb..e7c3dc9
*** a/src/test/regress/input/misc.source
--- b/src/test/regress/input/misc.source
*************** SELECT user_relns() AS user_relns
*** 223,228 ****
--- 223,232 ----
  
  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 2f4d482..84ea86b
*** a/src/test/regress/output/misc.source
--- b/src/test/regress/output/misc.source
*************** SELECT name(equipment(hobby_construct(te
*** 693,698 ****
--- 693,710 ----
   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 
  -----------------
diff --git a/doc/src/sgml/xfunc.sgml b/doc/src/sgml/xfunc.sgml
new file mode 100644
index 7064312..cc6f3a1
*** 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,555 ----
  <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>tf2.debit</>.
       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">.
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to