V0.1 patch for TODO Item: SQL-language reference parameters by name.
Hello all,
Hereby an alpha version regarding the:
TODO Item: SQL-language reference parameters by name.
I am sending this patch to check if I am on the right track.
So please take a look at this if possible.
What does this patch do?
As discussed in thread:
http://archives.postgresql.org/pgsql-hackers/2007-10/msg01490.php,
this patch adds an additional parameter (char **argnames) to
pg_parse_and_rewrite and
pg_analyze_and_rewrite and ParseState.
When transformColumnRef is about to report an error for a non existing
column,a final match is
performed to see if the non existing column is a parameter name. (argnames)
If true, then a new node is created by transformParamRef
NOTE:
- This patch is created using MSVC++ !
- Nothing is done yet for polymorphic arguments.
My test where:
create table tbl1(id serial,field1 integer,field2 varchar);
insert into tbl1 (field1,field2) values(11,'zzzz');
insert into tbl1 (field1,field2) values(22,'yyyy');
create or replace function func1(par1 integer,par2 integer,par3 varchar)
returns setof record as
$$
select
par1::text,
par2,
par1+par2,
par2+par1,
par1+field1,
(field1+par2)::varchar,
par3,
field2 || ' ' || par3
from
tbl1;
$$ language sql;
select func1(2,4,'aaaa');
select * from func1(5,16,'bbbb') as (a text ,b int ,c int, e int, f int,g
varchar,h varchar,i text);
results:
"(2,4,6,6,13,15,aaaa,"zzzz aaaa")"
"(2,4,6,6,24,26,aaaa,"yyyy aaaa")"
And
"5";16;21;21;16;"27";"bbbb";"zzzz bbbb"
"5";16;21;21;27;"38";"bbbb";"yyyy bbbb"
Regards,
Gevik
Attachments:
func-name-args-v0.1.patchapplication/octet-stream; name=func-name-args-v0.1.patchDownload
*** C:\pgdev\pgsql\src\backend\catalog\pg_proc.c.orig 2007-09-03 02:39:14.000000000 +0200
--- C:\pgdev\pgsql\src\backend\catalog\pg_proc.c 2007-11-02 12:43:52.551746600 +0100
***************
*** 533,538 ****
--- 533,543 ----
ErrorContextCallback sqlerrcontext;
bool haspolyarg;
int i;
+ int numargs;
+ Oid *argtypes;
+ char **argnames;
+ char *argmodes;
+
tuple = SearchSysCache(PROCOID,
ObjectIdGetDatum(funcoid),
***************
*** 597,605 ****
*/
if (!haspolyarg)
{
querytree_list = pg_parse_and_rewrite(prosrc,
proc->proargtypes.values,
! proc->pronargs);
(void) check_sql_fn_retval(funcoid, proc->prorettype,
querytree_list, NULL);
}
--- 602,614 ----
*/
if (!haspolyarg)
{
+ numargs = get_func_arg_info(tuple,
+ &argtypes, &argnames, &argmodes);
+
querytree_list = pg_parse_and_rewrite(prosrc,
proc->proargtypes.values,
! proc->pronargs,
! argnames);
(void) check_sql_fn_retval(funcoid, proc->prorettype,
querytree_list, NULL);
}
*** C:\pgdev\pgsql\src\backend\commands\copy.c.orig 2007-09-12 22:49:27.000000000 +0200
--- C:\pgdev\pgsql\src\backend\commands\copy.c 2007-11-02 12:51:17.566244800 +0100
***************
*** 1006,1013 ****
* function and is executed repeatedly. (See also the same hack in
* DECLARE CURSOR and PREPARE.) XXX FIXME someday.
*/
rewritten = pg_analyze_and_rewrite((Node *) copyObject(stmt->query),
! queryString, NULL, 0);
/* We don't expect more or less than one result query */
if (list_length(rewritten) != 1)
--- 1006,1014 ----
* function and is executed repeatedly. (See also the same hack in
* DECLARE CURSOR and PREPARE.) XXX FIXME someday.
*/
+ //TODO: Check here
rewritten = pg_analyze_and_rewrite((Node *) copyObject(stmt->query),
! queryString, NULL, 0,NULL);
/* We don't expect more or less than one result query */
if (list_length(rewritten) != 1)
*** C:\pgdev\pgsql\src\backend\commands\explain.c.orig 2007-08-15 23:39:50.000000000 +0200
--- C:\pgdev\pgsql\src\backend\commands\explain.c 2007-11-02 12:51:17.616303800 +0100
***************
*** 99,106 ****
* function and is executed repeatedly. (See also the same hack in
* DECLARE CURSOR and PREPARE.) XXX FIXME someday.
*/
rewritten = pg_analyze_and_rewrite((Node *) copyObject(stmt->query),
! queryString, param_types, num_params);
/* prepare for projection of tuples */
tstate = begin_tup_output_tupdesc(dest, ExplainResultDesc(stmt));
--- 99,107 ----
* function and is executed repeatedly. (See also the same hack in
* DECLARE CURSOR and PREPARE.) XXX FIXME someday.
*/
+ //TODO: Check here
rewritten = pg_analyze_and_rewrite((Node *) copyObject(stmt->query),
! queryString, param_types, num_params,NULL);
/* prepare for projection of tuples */
tstate = begin_tup_output_tupdesc(dest, ExplainResultDesc(stmt));
*** C:\pgdev\pgsql\src\backend\commands\view.c.orig 2007-08-27 05:36:08.000000000 +0200
--- C:\pgdev\pgsql\src\backend\commands\view.c 2007-11-02 12:51:17.676374600 +0100
***************
*** 362,369 ****
* Since parse analysis scribbles on its input, copy the raw parse tree;
* this ensures we don't corrupt a prepared statement, for example.
*/
viewParse = parse_analyze((Node *) copyObject(stmt->query),
! queryString, NULL, 0);
/*
* The grammar should ensure that the result is a single SELECT Query.
--- 362,370 ----
* Since parse analysis scribbles on its input, copy the raw parse tree;
* this ensures we don't corrupt a prepared statement, for example.
*/
+ //TODO: Check here
viewParse = parse_analyze((Node *) copyObject(stmt->query),
! queryString, NULL, 0,NULL);
/*
* The grammar should ensure that the result is a single SELECT Query.
*** C:\pgdev\pgsql\src\backend\executor\functions.c.orig 2007-06-17 20:57:29.000000000 +0200
--- C:\pgdev\pgsql\src\backend\executor\functions.c 2007-11-02 13:03:51.244537000 +0100
***************
*** 163,168 ****
--- 163,172 ----
List *queryTree_list;
Datum tmp;
bool isNull;
+ int numargs;
+ Oid *argtypes;
+ char **argnames;
+ char *argmodes;
fcache = (SQLFunctionCachePtr) palloc0(sizeof(SQLFunctionCache));
***************
*** 248,254 ****
/*
* Parse and rewrite the queries in the function text.
*/
! queryTree_list = pg_parse_and_rewrite(fcache->src, argOidVect, nargs);
/*
* Check that the function returns the type it claims to. Although
--- 252,261 ----
/*
* Parse and rewrite the queries in the function text.
*/
!
! numargs = get_func_arg_info(procedureTuple,
! &argtypes, &argnames, &argmodes);
! queryTree_list = pg_parse_and_rewrite(fcache->src, argOidVect, nargs,argnames);
/*
* Check that the function returns the type it claims to. Although
*** C:\pgdev\pgsql\src\backend\executor\spi.c.orig 2007-10-25 15:48:57.000000000 +0200
--- C:\pgdev\pgsql\src\backend\executor\spi.c 2007-11-02 13:03:51.354666800 +0100
***************
*** 1409,1416 ****
CachedPlan *cplan;
/* Need a copyObject here to keep parser from modifying raw tree */
stmt_list = pg_analyze_and_rewrite(copyObject(parsetree),
! src, argtypes, nargs);
stmt_list = pg_plan_queries(stmt_list, cursor_options, NULL, false);
plansource = (CachedPlanSource *) palloc0(sizeof(CachedPlanSource));
--- 1409,1417 ----
CachedPlan *cplan;
/* Need a copyObject here to keep parser from modifying raw tree */
+ //TODO: Check Here
stmt_list = pg_analyze_and_rewrite(copyObject(parsetree),
! src, argtypes, nargs,NULL);
stmt_list = pg_plan_queries(stmt_list, cursor_options, NULL, false);
plansource = (CachedPlanSource *) palloc0(sizeof(CachedPlanSource));
*** C:\pgdev\pgsql\src\backend\optimizer\util\clauses.c.orig 2007-10-11 23:27:49.000000000 +0200
--- C:\pgdev\pgsql\src\backend\optimizer\util\clauses.c 2007-11-02 13:05:18.517397600 +0100
***************
*** 18,24 ****
*/
#include "postgres.h"
!
#include "access/heapam.h"
#include "catalog/pg_aggregate.h"
#include "catalog/pg_language.h"
--- 18,24 ----
*/
#include "postgres.h"
! #include "funcapi.h"
#include "access/heapam.h"
#include "catalog/pg_aggregate.h"
#include "catalog/pg_language.h"
***************
*** 2916,2921 ****
--- 2916,2924 ----
int *usecounts;
ListCell *arg;
int i;
+ int numargs;
+ char **argnames;
+ char *argmodes;
/*
* Forget it if the function is not SQL-language or has other showstopper
***************
*** 2987,2994 ****
if (list_length(raw_parsetree_list) != 1)
goto fail;
querytree = parse_analyze(linitial(raw_parsetree_list), src,
! argtypes, funcform->pronargs);
/*
* The single command must be a simple "SELECT expression".
--- 2990,3000 ----
if (list_length(raw_parsetree_list) != 1)
goto fail;
+ numargs = get_func_arg_info(func_tuple,
+ &argtypes, &argnames, &argmodes);
+
querytree = parse_analyze(linitial(raw_parsetree_list), src,
! argtypes, funcform->pronargs,argnames);
/*
* The single command must be a simple "SELECT expression".
*** C:\pgdev\pgsql\src\backend\parser\analyze.c.orig 2007-10-25 15:48:57.000000000 +0200
--- C:\pgdev\pgsql\src\backend\parser\analyze.c 2007-11-02 12:42:00.669881600 +0100
***************
*** 83,89 ****
*/
Query *
parse_analyze(Node *parseTree, const char *sourceText,
! Oid *paramTypes, int numParams)
{
ParseState *pstate = make_parsestate(NULL);
Query *query;
--- 83,89 ----
*/
Query *
parse_analyze(Node *parseTree, const char *sourceText,
! Oid *paramTypes, int numParams,char **argnames)
{
ParseState *pstate = make_parsestate(NULL);
Query *query;
***************
*** 91,96 ****
--- 91,97 ----
pstate->p_sourcetext = sourceText;
pstate->p_paramtypes = paramTypes;
pstate->p_numparams = numParams;
+ pstate->p_argnames = argnames;
pstate->p_variableparams = false;
query = transformStmt(pstate, parseTree);
*** C:\pgdev\pgsql\src\backend\parser\parse_expr.c.orig 2007-06-24 00:12:51.000000000 +0200
--- C:\pgdev\pgsql\src\backend\parser\parse_expr.c 2007-11-02 15:25:46.130260000 +0100
***************
*** 349,354 ****
--- 349,355 ----
int numnames = list_length(cref->fields);
Node *node;
int levels_up;
+ int i;
/*----------
* The allowed syntaxes are:
***************
*** 410,423 ****
--- 411,445 ----
*/
if (refnameRangeTblEntry(pstate, NULL, name,
&levels_up) != NULL)
+ {
node = transformWholeRowRef(pstate, NULL, name,
cref->location);
+ }
+ else if(pstate->p_argnames != NULL)
+ {
+ /* try a final check to match the columnref to
+ * function argument names
+ */
+ for(i = 0; i < pstate->p_numparams; i++)
+ {
+ if(strcmp(pstate->p_argnames[i],name) == 0)
+ {
+ ParamRef pref;
+ pref.type = T_ParamRef;
+ pref.number = i+1;
+ node = transformParamRef(pstate,&pref);
+ break;
+ }
+ }
+ }
else
+ {
ereport(ERROR,
(errcode(ERRCODE_UNDEFINED_COLUMN),
errmsg("column \"%s\" does not exist",
name),
parser_errposition(pstate, cref->location)));
+ }
}
break;
}
*** C:\pgdev\pgsql\src\backend\tcop\postgres.c.orig 2007-08-03 01:39:44.000000000 +0200
--- C:\pgdev\pgsql\src\backend\tcop\postgres.c 2007-11-02 13:01:35.304316600 +0100
***************
*** 500,506 ****
List *
pg_parse_and_rewrite(const char *query_string, /* string to execute */
Oid *paramTypes, /* parameter types */
! int numParams) /* number of parameters */
{
List *raw_parsetree_list;
List *querytree_list;
--- 500,507 ----
List *
pg_parse_and_rewrite(const char *query_string, /* string to execute */
Oid *paramTypes, /* parameter types */
! int numParams,
! char **argnames) /* number of parameters */
{
List *raw_parsetree_list;
List *querytree_list;
***************
*** 519,529 ****
{
Node *parsetree = (Node *) lfirst(list_item);
querytree_list = list_concat(querytree_list,
pg_analyze_and_rewrite(parsetree,
query_string,
paramTypes,
! numParams));
}
return querytree_list;
--- 520,532 ----
{
Node *parsetree = (Node *) lfirst(list_item);
+ //TODO: Check Here
querytree_list = list_concat(querytree_list,
pg_analyze_and_rewrite(parsetree,
query_string,
paramTypes,
! numParams,
! argnames));
}
return querytree_list;
***************
*** 582,588 ****
*/
List *
pg_analyze_and_rewrite(Node *parsetree, const char *query_string,
! Oid *paramTypes, int numParams)
{
Query *query;
List *querytree_list;
--- 585,591 ----
*/
List *
pg_analyze_and_rewrite(Node *parsetree, const char *query_string,
! Oid *paramTypes, int numParams,char **argnames)
{
Query *query;
List *querytree_list;
***************
*** 593,599 ****
if (log_parser_stats)
ResetUsage();
! query = parse_analyze(parsetree, query_string, paramTypes, numParams);
if (log_parser_stats)
ShowUsage("PARSE ANALYSIS STATISTICS");
--- 596,602 ----
if (log_parser_stats)
ResetUsage();
! query = parse_analyze(parsetree, query_string, paramTypes, numParams,argnames);
if (log_parser_stats)
ShowUsage("PARSE ANALYSIS STATISTICS");
***************
*** 896,903 ****
*/
oldcontext = MemoryContextSwitchTo(MessageContext);
querytree_list = pg_analyze_and_rewrite(parsetree, query_string,
! NULL, 0);
plantree_list = pg_plan_queries(querytree_list, 0, NULL, true);
--- 899,907 ----
*/
oldcontext = MemoryContextSwitchTo(MessageContext);
+ //TODO: Check here
querytree_list = pg_analyze_and_rewrite(parsetree, query_string,
! NULL, 0,NULL);
plantree_list = pg_plan_queries(querytree_list, 0, NULL, true);
*** C:\pgdev\pgsql\src\backend\utils\cache\plancache.c.orig 2007-10-11 20:05:27.000000000 +0200
--- C:\pgdev\pgsql\src\backend\utils\cache\plancache.c 2007-11-02 13:01:35.274281200 +0100
***************
*** 474,483 ****
* parse_analyze_varparams(), assuming that the caller never wants the
* parameter types to change from the original values.
*/
slist = pg_analyze_and_rewrite(copyObject(plansource->raw_parse_tree),
plansource->query_string,
plansource->param_types,
! plansource->num_params);
if (plansource->fully_planned)
{
--- 474,485 ----
* parse_analyze_varparams(), assuming that the caller never wants the
* parameter types to change from the original values.
*/
+ //TODO: Check here
slist = pg_analyze_and_rewrite(copyObject(plansource->raw_parse_tree),
plansource->query_string,
plansource->param_types,
! plansource->num_params,
! NULL);
if (plansource->fully_planned)
{
*** C:\pgdev\pgsql\src\include\parser\analyze.h.orig 2007-06-24 00:12:52.000000000 +0200
--- C:\pgdev\pgsql\src\include\parser\analyze.h 2007-11-02 12:42:00.419586600 +0100
***************
*** 18,24 ****
extern Query *parse_analyze(Node *parseTree, const char *sourceText,
! Oid *paramTypes, int numParams);
extern Query *parse_analyze_varparams(Node *parseTree, const char *sourceText,
Oid **paramTypes, int *numParams);
--- 18,24 ----
extern Query *parse_analyze(Node *parseTree, const char *sourceText,
! Oid *paramTypes, int numParams,char **argnames);
extern Query *parse_analyze_varparams(Node *parseTree, const char *sourceText,
Oid **paramTypes, int *numParams);
*** C:\pgdev\pgsql\src\include\parser\parse_node.h.orig 2007-06-24 00:12:52.000000000 +0200
--- C:\pgdev\pgsql\src\include\parser\parse_node.h 2007-11-02 12:42:00.519704600 +0100
***************
*** 69,74 ****
--- 69,75 ----
List *p_relnamespace; /* current namespace for relations */
List *p_varnamespace; /* current namespace for columns */
Oid *p_paramtypes; /* OIDs of types for $n parameter symbols */
+ const char **p_argnames; /* names of function arguments */
int p_numparams; /* allocated size of p_paramtypes[] */
int p_next_resno; /* next targetlist resno to assign */
List *p_locking_clause; /* raw FOR UPDATE/FOR SHARE info */
*** C:\pgdev\pgsql\src\include\tcop\tcopprot.h.orig 2007-07-25 14:22:54.000000000 +0200
--- C:\pgdev\pgsql\src\include\tcop\tcopprot.h 2007-11-02 12:42:00.299445000 +0100
***************
*** 45,54 ****
extern LogStmtLevel log_statement;
extern List *pg_parse_and_rewrite(const char *query_string,
! Oid *paramTypes, int numParams);
extern List *pg_parse_query(const char *query_string);
extern List *pg_analyze_and_rewrite(Node *parsetree, const char *query_string,
! Oid *paramTypes, int numParams);
extern PlannedStmt *pg_plan_query(Query *querytree, int cursorOptions,
ParamListInfo boundParams);
extern List *pg_plan_queries(List *querytrees, int cursorOptions,
--- 45,54 ----
extern LogStmtLevel log_statement;
extern List *pg_parse_and_rewrite(const char *query_string,
! Oid *paramTypes, int numParams,char **argnames);
extern List *pg_parse_query(const char *query_string);
extern List *pg_analyze_and_rewrite(Node *parsetree, const char *query_string,
! Oid *paramTypes, int numParams,char **argnames);
extern PlannedStmt *pg_plan_query(Query *querytree, int cursorOptions,
ParamListInfo boundParams);
extern List *pg_plan_queries(List *querytrees, int cursorOptions,
Gevik Babakhani wrote:
Hello all,
Hereby an alpha version regarding the:
TODO Item: SQL-language reference parameters by name.
I am sending this patch to check if I am on the right track.
So please take a look at this if possible.
Step 1: don't use c++ style comments like this:
+ //TODO: Check here
C89 is basically our standard. gcc -std=c89 will check that it complies.
cheers
andrew
Hello all,
Hereby an alpha version regarding the:
TODO Item: SQL-language reference parameters by name.
what about name's collision? Maybe is better use some prefix, like $
or :. Without it we only propagate one problem from plpgsql to others
languages.
It can be more wide used:
* named params in prepared statements
* named params in SPI
* ..
Regards
Pavel Stehule
Noted. Thank you.
-----Original Message-----
From: pgsql-patches-owner@postgresql.org
[mailto:pgsql-patches-owner@postgresql.org] On Behalf Of Andrew Dunstan
Sent: Friday, November 02, 2007 4:19 PM
To: Gevik Babakhani
Cc: pgsql-patches@postgresql.org
Subject: Re: [PATCHES] V0.1 patch for TODO Item: SQL-language reference
parameters by name.
Gevik Babakhani wrote:
Hello all,
Hereby an alpha version regarding the:
TODO Item: SQL-language reference parameters by name.
I am sending this patch to check if I am on the right track.
So please take a look at this if possible.
Step 1: don't use c++ style comments like this:
+ //TODO: Check here
C89 is basically our standard. gcc -std=c89 will check that it complies.
cheers
andrew
---------------------------(end of broadcast)---------------------------
TIP 6: explain analyze is your friend
Hi,
what about name's collision? Maybe is better use some prefix,
like $ or :. Without it we only propagate one problem from
plpgsql to others languages.
Please explain.
Perhaps I am wrong, but plpgsql handles arsgument names before it
passes the query to be executed. Please see:
plpgsql/pl_comp.c/do_compile(...)/line: 393
Regards,
Gevik.
"Gevik Babakhani" <pgdev@xs4all.nl> writes:
I am sending this patch to check if I am on the right track.
So please take a look at this if possible.
You seem not to have understood my recommendation to use a callback
function. This patch might work nicely for SQL functions but there
will be no good way to use it for plpgsql, or probably any other
PL function language. If we're going to change the parser API
then I'd like to have a more general solution.
regards, tom lane
"Gevik Babakhani" <pgdev@xs4all.nl> writes:
what about name's collision? Maybe is better use some prefix,
like $ or :. Without it we only propagate one problem from
plpgsql to others languages.
Please explain.
Perhaps I am wrong, but plpgsql handles arsgument names before it
passes the query to be executed.
Which is actually the Wrong Thing to do: really the parameters should be
seen as being in a name scope that's outside that of the query (and thus
ambiguous names should be resolved first as column names of the query).
The proposed patch does this in the right order and so I think that
Pavel's concern is without foundation.
One point here is that it would be good to be able to qualify the
argument names with the function name, for example
create function myfunc(x int) ...
select ... from t where t.x = myfunc.x
If t has a column named x then this will be the only way that the
function parameter x can be referenced within that query. We are
partway to that point with plpgsql but haven't bitten the bullet
of changing the lookup order.
Note that this consideration is another reason for having a callback
function that's responsible for trying to resolve unresolved names.
I certainly wouldn't like to have a notion of "function name" wired
into the parser API, and if we did do that it still wouldn't be
sufficient for plpgsql which can have multiple block-label namespaces
accessible at once.
regards, tom lane
On 02/11/2007, Gevik Babakhani <pgdev@xs4all.nl> wrote:
Hi,
what about name's collision? Maybe is better use some prefix,
like $ or :. Without it we only propagate one problem from
plpgsql to others languages.Please explain.
Perhaps I am wrong, but plpgsql handles arsgument names before it
passes the query to be executed. Please see:plpgsql/pl_comp.c/do_compile(...)/line: 393
Regards,
Gevik.
it's one from mystic bugs:
create table t(a integer, b integer);
insert into t values(10,20);
create function foo(a integer)
returns integer
as $$
select a from t
where a <> b and a = 10;
$$ languge sql;
select foo(20);
output? expected 10, but you will get NULL!
Regards
Pavel Stehule
so some prefixes can help
create function foo(a integer)
returns integer
as $$
select a from t
where :a <> b and a = 10;
$$ languge sql;
Oracle use symbol ':'
I don't know what others databases has.
Regards
Pavel Stehule
Hello,
You seem not to have understood my recommendation to use a
callback function. This patch might work nicely for SQL
functions but there will be no good way to use it for
plpgsql, or probably any other PL function language. If
we're going to change the parser API then I'd like to have a
more general solution.
Perhaps I did not look well enough, but is there any callback mechanism like
the
error_context_stack etc... in the parser?
( If not, I guess one has to be created :) )
Thank you.
Gevik.