proof concept: do statement parametrization

Started by Pavel Stehuleover 15 years ago17 messages
#1Pavel Stehule
pavel.stehule@gmail.com
1 attachment(s)

Hello

I enhanced DO statement syntax to allowing a parameters. Syntax is
relative simple:

do ([varname] vartype := value, ...) $$ ... $$

It allows to pass a content of psql variables to inline code block to
allows more easy scripting

\set schema 'public'

do(text := :'schema') $$
declare r record;
begin
for r in
select * from information_schema.tables where table_schema = $1
loop
raise notice '>>> table %', r.table_name;
end loop;
end $$;
NOTICE: >>> table t
NOTICE: >>> table t1
DO

ToDo:

* doesn't allows SubLinks :(

pavel@postgres:5432=# do(text := (SELECT :'schema')) $$ declare r
record; begin for r in select * from information_schema.tables where
table_schema = $1 loop raise notice '>>> table %', r.table_name; end
loop; end $$;
ERROR: XX000: unrecognized node type: 315
LOCATION: ExecInitExpr, execQual.c:4868

ideas, notes, comments??

Regards

Pavel Stehule

Attachments:

do_parametrization.difftext/x-patch; charset=US-ASCII; name=do_parametrization.diffDownload
*** ./src/backend/commands/functioncmds.c.orig	2010-02-26 03:00:39.000000000 +0100
--- ./src/backend/commands/functioncmds.c	2010-07-04 07:50:16.175265641 +0200
***************
*** 47,53 ****
--- 47,55 ----
  #include "catalog/pg_type_fn.h"
  #include "commands/defrem.h"
  #include "commands/proclang.h"
+ #include "executor/executor.h"
  #include "miscadmin.h"
+ #include "optimizer/planmain.h"
  #include "optimizer/var.h"
  #include "parser/parse_coerce.h"
  #include "parser/parse_expr.h"
***************
*** 55,60 ****
--- 57,63 ----
  #include "parser/parse_type.h"
  #include "utils/acl.h"
  #include "utils/builtins.h"
+ #include "utils/datum.h"
  #include "utils/fmgroids.h"
  #include "utils/guc.h"
  #include "utils/lsyscache.h"
***************
*** 1928,1934 ****
   *		Execute inline procedural-language code
   */
  void
! ExecuteDoStmt(DoStmt *stmt)
  {
  	InlineCodeBlock *codeblock = makeNode(InlineCodeBlock);
  	ListCell   *arg;
--- 1931,1937 ----
   *		Execute inline procedural-language code
   */
  void
! ExecuteDoStmt(DoStmt *stmt, const char *queryString)
  {
  	InlineCodeBlock *codeblock = makeNode(InlineCodeBlock);
  	ListCell   *arg;
***************
*** 1939,1944 ****
--- 1942,1948 ----
  	Oid			laninline;
  	HeapTuple	languageTuple;
  	Form_pg_language languageStruct;
+ 	ParseState *pstate;
  
  	/* Process options we got from gram.y */
  	foreach(arg, stmt->args)
***************
*** 1973,1978 ****
--- 1977,2088 ----
  				(errcode(ERRCODE_SYNTAX_ERROR),
  				 errmsg("no inline code specified")));
  
+ 	/* Transform parameters - when are used */
+ 	if (stmt->params != NIL)
+ 	{
+ 		ListCell   *param;
+ 		EState	   *estate;
+ 		int	nparams;
+ 		int		i = 0;
+ 
+ 		nparams = list_length(stmt->params);
+ 		
+ 		codeblock->nparams = nparams;
+ 		codeblock->dvalues = (Datum *) palloc(nparams * sizeof(Datum));
+ 		codeblock->nulls = (bool *) palloc(nparams * sizeof(bool));
+ 		codeblock->names = (char **) palloc(nparams * sizeof(char *));
+ 		codeblock->typoids = (Oid *) palloc(nparams * sizeof(Oid));
+ 
+ 		/* prepare pstate for parse analysis of param exprs */
+ 		pstate = make_parsestate(NULL);
+ 		pstate->p_sourcetext = queryString;
+ 		
+ 		foreach(param, stmt->params)
+ 		{
+ 			FunctionParameter *p = (FunctionParameter *) lfirst(param);
+ 			Oid	toid;
+ 			Type		typtup;
+ 			TypeName	*t = p->argType;
+ 			MemoryContext	   oldcontext;
+ 			ExprState *exprstate;
+ 			Node *expr;
+ 			Datum		const_val;
+ 			bool		const_is_null;
+ 			int16		resultTypLen;
+ 			bool		resultTypByVal;
+ 
+ 			codeblock->names[i] = p->name;
+ 
+ 			if (t->setof)
+ 				ereport(ERROR,
+ 					(errcode(ERRCODE_INVALID_FUNCTION_DEFINITION),
+ 					 errmsg("inline procedural code cannot accept set arguments")));
+ 
+ 			typtup = LookupTypeName(NULL, t, NULL);
+ 			if (typtup)
+ 			{
+ 				if (!((Form_pg_type) GETSTRUCT(typtup))->typisdefined)
+ 					ereport(NOTICE,
+ 							(errcode(ERRCODE_WRONG_OBJECT_TYPE),
+ 							 errmsg("argument type %s is only a shell",
+ 									TypeNameToString(t))));
+ 				
+ 				toid = typeTypeId(typtup);
+ 				ReleaseSysCache(typtup);
+ 				codeblock->typoids[i] = toid;
+ 			}
+ 			else
+ 			{
+ 				ereport(ERROR,
+ 						(errcode(ERRCODE_UNDEFINED_OBJECT),
+ 						 errmsg("type %s does not exist",
+ 								TypeNameToString(t))));
+ 				toid = InvalidOid;	/* keep compiler quiet */
+ 			}
+ 			
+ 			estate = CreateExecutorState();
+ 			
+ 			expr = transformExpr(pstate, p->defexpr);
+ 			expr  = coerce_to_specific_type(pstate, expr,
+ 									    toid, "DEFAULT");
+ 			
+ 			oldcontext = MemoryContextSwitchTo(estate->es_query_cxt);
+ 			
+ 			fix_opfuncids(expr);
+ 			
+ 			/* Prepare expr for execution */
+ 			exprstate = ExecPrepareExpr((Expr *) expr, estate);
+ 			
+ 			/* And evaluaye it */
+ 			const_val = ExecEvalExprSwitchContext(exprstate,
+ 									    GetPerTupleExprContext(estate),
+ 									    &const_is_null, NULL);
+ 			/* Get info needed about result datatype */
+ 			get_typlenbyval(toid, &resultTypLen, &resultTypByVal);
+ 									    
+ 			/* Get back outer memory context */
+ 			MemoryContextSwitchTo(oldcontext);
+ 			
+ 			if (!const_is_null)
+ 				const_val = datumCopy(const_val, resultTypByVal, resultTypLen);
+ 			
+ 			codeblock->nulls[i] = const_is_null;
+ 			codeblock->dvalues[i] = const_val;
+ 			
+ 			FreeExecutorState(estate);
+ 			i += 1;
+ 		}
+ 	}
+ 	else
+ 	{
+ 		codeblock->nparams = 0;
+ 		codeblock->dvalues = NULL;
+ 		codeblock->nulls = NULL;
+ 		codeblock->names = NULL;
+ 		codeblock->typoids = NULL;
+ 	}
+ 
+ 
  	/* if LANGUAGE option wasn't specified, use the default */
  	if (language_item)
  		language = strVal(language_item->arg);
*** ./src/backend/nodes/copyfuncs.c.orig	2010-02-26 03:00:43.000000000 +0100
--- ./src/backend/nodes/copyfuncs.c	2010-06-27 13:45:22.340830586 +0200
***************
*** 2680,2685 ****
--- 2680,2686 ----
  	DoStmt	   *newnode = makeNode(DoStmt);
  
  	COPY_NODE_FIELD(args);
+ 	COPY_NODE_FIELD(params);
  
  	return newnode;
  }
*** ./src/backend/nodes/equalfuncs.c.orig	2010-02-26 03:00:43.000000000 +0100
--- ./src/backend/nodes/equalfuncs.c	2010-06-27 13:46:01.856829383 +0200
***************
*** 1242,1247 ****
--- 1242,1248 ----
  _equalDoStmt(DoStmt *a, DoStmt *b)
  {
  	COMPARE_NODE_FIELD(args);
+ 	COMPARE_NODE_FIELD(params);
  
  	return true;
  }
*** ./src/backend/parser/gram.y.orig	2010-06-13 19:43:12.000000000 +0200
--- ./src/backend/parser/gram.y	2010-07-04 07:55:13.124266470 +0200
***************
*** 437,442 ****
--- 437,444 ----
  				opt_frame_clause frame_extent frame_bound
  %type <str>		opt_existing_window_name
  
+ %type <node>	do_arg
+ %type <list>	do_args
  
  /*
   * Non-keyword token types.  These are hard-wired into the "flex" lexer.
***************
*** 5445,5454 ****
   *
   *****************************************************************************/
  
! DoStmt: DO dostmt_opt_list
  				{
  					DoStmt *n = makeNode(DoStmt);
  					n->args = $2;
  					$$ = (Node *)n;
  				}
  		;
--- 5447,5465 ----
   *
   *****************************************************************************/
  
! DoStmt: 
! 			DO '(' do_args ')' dostmt_opt_list
! 				{
! 					DoStmt *n = makeNode(DoStmt);
! 					n->args = $5;
! 					n->params = $3;
! 					$$ = (Node *)n;
! 				}
! 			| DO dostmt_opt_list
  				{
  					DoStmt *n = makeNode(DoStmt);
  					n->args = $2;
+ 					n->params = NIL;
  					$$ = (Node *)n;
  				}
  		;
***************
*** 5469,5474 ****
--- 5480,5519 ----
  				}
  		;
  
+ do_args:
+ 			do_arg
+ 				{
+ 					$$ = list_make1($1);
+ 				}
+ 			| do_args ',' do_arg
+ 				{
+ 					$$ = lappend($1, $3);
+ 				}
+ 			;
+ 
+ do_arg:
+ 			func_type COLON_EQUALS a_expr
+ 				{
+ 					FunctionParameter *n = makeNode(FunctionParameter);
+ 					n->name = NULL;
+ 					n->argType = $1;
+ 					n->mode = FUNC_PARAM_IN;
+ 					n->defexpr = $3;
+ 					$$ = (Node *) n;
+ 				}
+ 			| param_name func_type COLON_EQUALS a_expr
+ 				{
+ 					FunctionParameter *n = makeNode(FunctionParameter);
+ 					n->name = $1;
+ 					n->argType = $2;
+ 					n->mode = FUNC_PARAM_IN;
+ 					n->defexpr = $4;
+ 					$$ = (Node *) n;
+ 				}
+ 		;
+ 
+ 
+ 
  /*****************************************************************************
   *
   *		CREATE CAST / DROP CAST
*** ./src/backend/tcop/utility.c.orig	2010-02-26 03:01:04.000000000 +0100
--- ./src/backend/tcop/utility.c	2010-06-27 16:20:12.075291309 +0200
***************
*** 930,936 ****
  			break;
  
  		case T_DoStmt:
! 			ExecuteDoStmt((DoStmt *) parsetree);
  			break;
  
  		case T_CreatedbStmt:
--- 930,936 ----
  			break;
  
  		case T_DoStmt:
! 			ExecuteDoStmt((DoStmt *) parsetree, queryString);
  			break;
  
  		case T_CreatedbStmt:
*** ./src/include/commands/defrem.h.orig	2010-02-26 03:01:24.000000000 +0100
--- ./src/include/commands/defrem.h	2010-06-27 16:21:19.119051968 +0200
***************
*** 66,72 ****
  extern void DropCastById(Oid castOid);
  extern void AlterFunctionNamespace(List *name, List *argtypes, bool isagg,
  					   const char *newschema);
! extern void ExecuteDoStmt(DoStmt *stmt);
  
  /* commands/operatorcmds.c */
  extern void DefineOperator(List *names, List *parameters);
--- 66,72 ----
  extern void DropCastById(Oid castOid);
  extern void AlterFunctionNamespace(List *name, List *argtypes, bool isagg,
  					   const char *newschema);
! extern void ExecuteDoStmt(DoStmt *stmt, const char *queryString);
  
  /* commands/operatorcmds.c */
  extern void DefineOperator(List *names, List *parameters);
*** ./src/include/nodes/parsenodes.h.orig	2010-02-26 03:01:25.000000000 +0100
--- ./src/include/nodes/parsenodes.h	2010-07-03 20:56:28.828265897 +0200
***************
*** 1991,1996 ****
--- 1991,1997 ----
  {
  	NodeTag		type;
  	List	   *args;			/* List of DefElem nodes */
+ 	List		*params;		/* List of Function parameters */
  } DoStmt;
  
  typedef struct InlineCodeBlock
***************
*** 1999,2004 ****
--- 2000,2010 ----
  	char	   *source_text;	/* source text of anonymous code block */
  	Oid			langOid;		/* OID of selected language */
  	bool		langIsTrusted;	/* trusted property of the language */
+ 	int		nparams;
+ 	Datum		   *dvalues;		/* Values of parameters if they are */
+ 	bool		   *nulls;		/* nulls of parameters if they are */
+ 	char		   **names;		/* used names for parameters if they are */
+ 	Oid		   *typoids;		/* array of parameter types if they are */
  } InlineCodeBlock;
  
  /* ----------------------
*** ./src/pl/plpgsql/src/pl_comp.c.orig	2010-02-26 03:01:34.000000000 +0100
--- ./src/pl/plpgsql/src/pl_comp.c	2010-07-04 07:58:19.441266296 +0200
***************
*** 727,733 ****
   * ----------
   */
  PLpgSQL_function *
! plpgsql_compile_inline(char *proc_source)
  {
  	char	   *func_name = "inline_code_block";
  	PLpgSQL_function *function;
--- 727,733 ----
   * ----------
   */
  PLpgSQL_function *
! plpgsql_compile_inline(InlineCodeBlock *codeblock)
  {
  	char	   *func_name = "inline_code_block";
  	PLpgSQL_function *function;
***************
*** 737,742 ****
--- 737,744 ----
  	int			parse_rc;
  	MemoryContext func_cxt;
  	int			i;
+ 	char		*proc_source = codeblock->source_text;
+ 	int		*in_arg_varnos = NULL;
  
  	/*
  	 * Setup the scanner input and error info.	We assume that this function
***************
*** 812,818 ****
  								 plpgsql_build_datatype(BOOLOID, -1),
  								 true);
  	function->found_varno = var->dno;
! 
  	/*
  	 * Now parse the function's text
  	 */
--- 814,868 ----
  								 plpgsql_build_datatype(BOOLOID, -1),
  								 true);
  	function->found_varno = var->dno;
! 	
! 	/*
! 	 * Complete the function's info
! 	 */
! 	function->fn_nargs = codeblock->nparams;
! 	in_arg_varnos = (int *) palloc(codeblock->nparams * sizeof(int));
! 	
! 	/* 
! 	 * Create variables for inline outer parameters 
! 	 */
! 	for(i = 0; i < codeblock->nparams; i++)
! 	{
! 		char		buf[32];
! 		PLpgSQL_type *argtype;
! 		PLpgSQL_variable *argvariable;
! 		int		argitemtype;
! 
! 		/* Create $n name for variable */
! 		snprintf(buf, sizeof(buf), "$%d", i + 1);
! 		
! 		/* Create datatype info */
! 		argtype = plpgsql_build_datatype(codeblock->typoids[i], -1);
! 		
! 		/* Disallow pseudotype argument */
! 		if (argtype->ttype != PLPGSQL_TTYPE_SCALAR &&
! 			argtype->ttype != PLPGSQL_TTYPE_ROW)
! 			ereport(ERROR,
! 					(errcode(ERRCODE_FEATURE_NOT_SUPPORTED),
! 					 errmsg("PL/pgSQL inline code cannot accept type %s",
! 							format_type_be(codeblock->typoids[i]))));
! 		
! 		/* Build variable and add to datum list */
! 		argvariable = plpgsql_build_variable(buf, 0,
! 									argtype, false);
! 		if (argvariable->dtype == PLPGSQL_DTYPE_ROW)
! 			argitemtype = PLPGSQL_NSTYPE_VAR;
! 		else
! 			argitemtype = PLPGSQL_NSTYPE_ROW;
! 		
! 		in_arg_varnos[i] = argvariable->dno;
! 		
! 		/* Add to namespace */
! 		plpgsql_ns_additem(argitemtype, argvariable->dno, buf);
! 		
! 		if (codeblock->names[i] != NULL)
! 			plpgsql_ns_additem(argitemtype, argvariable->dno,
! 									    codeblock->names[i]);
! 	}
! 	
  	/*
  	 * Now parse the function's text
  	 */
***************
*** 830,839 ****
  	if (function->fn_rettype == VOIDOID)
  		add_dummy_return(function);
  
! 	/*
! 	 * Complete the function's info
! 	 */
! 	function->fn_nargs = 0;
  	function->ndatums = plpgsql_nDatums;
  	function->datums = palloc(sizeof(PLpgSQL_datum *) * plpgsql_nDatums);
  	for (i = 0; i < plpgsql_nDatums; i++)
--- 880,888 ----
  	if (function->fn_rettype == VOIDOID)
  		add_dummy_return(function);
  
! 	for (i = 0; i < function->fn_nargs; i++)
! 		function->fn_argvarnos[i] = in_arg_varnos[i];
! 
  	function->ndatums = plpgsql_nDatums;
  	function->datums = palloc(sizeof(PLpgSQL_datum *) * plpgsql_nDatums);
  	for (i = 0; i < plpgsql_nDatums; i++)
***************
*** 849,854 ****
--- 898,904 ----
  
  	MemoryContextSwitchTo(compile_tmp_cxt);
  	compile_tmp_cxt = NULL;
+ 
  	return function;
  }
  
*** ./src/pl/plpgsql/src/pl_handler.c.orig	2010-02-26 03:01:35.000000000 +0100
--- ./src/pl/plpgsql/src/pl_handler.c	2010-07-04 07:27:03.842263509 +0200
***************
*** 160,165 ****
--- 160,166 ----
  	FmgrInfo	flinfo;
  	Datum		retval;
  	int			rc;
+ 	int	i;
  
  	Assert(IsA(codeblock, InlineCodeBlock));
  
***************
*** 170,183 ****
  		elog(ERROR, "SPI_connect failed: %s", SPI_result_code_string(rc));
  
  	/* Compile the anonymous code block */
! 	func = plpgsql_compile_inline(codeblock->source_text);
  
  	/*
  	 * Set up a fake fcinfo with just enough info to satisfy
  	 * plpgsql_exec_function().  In particular note that this sets things up
  	 * with no arguments passed.
  	 */
! 	MemSet(&fake_fcinfo, 0, sizeof(fake_fcinfo));
  	MemSet(&flinfo, 0, sizeof(flinfo));
  	fake_fcinfo.flinfo = &flinfo;
  	flinfo.fn_oid = InvalidOid;
--- 171,191 ----
  		elog(ERROR, "SPI_connect failed: %s", SPI_result_code_string(rc));
  
  	/* Compile the anonymous code block */
! 	func = plpgsql_compile_inline(codeblock);
  
  	/*
  	 * Set up a fake fcinfo with just enough info to satisfy
  	 * plpgsql_exec_function().  In particular note that this sets things up
  	 * with no arguments passed.
  	 */
! 	InitFunctionCallInfoData(fake_fcinfo, &flinfo, codeblock->nparams, NULL, NULL);
! 	
! 	for (i = 0; i < codeblock->nparams; i++)
! 	{
! 		fake_fcinfo.arg[i] = codeblock->dvalues[i];
! 		fake_fcinfo.argnull[i] = codeblock->nulls[i];
! 	}
! 	 
  	MemSet(&flinfo, 0, sizeof(flinfo));
  	fake_fcinfo.flinfo = &flinfo;
  	flinfo.fn_oid = InvalidOid;
*** ./src/pl/plpgsql/src/plpgsql.h.orig	2010-02-26 03:01:35.000000000 +0100
--- ./src/pl/plpgsql/src/plpgsql.h	2010-07-03 20:49:40.992266524 +0200
***************
*** 832,838 ****
   */
  extern PLpgSQL_function *plpgsql_compile(FunctionCallInfo fcinfo,
  				bool forValidator);
! extern PLpgSQL_function *plpgsql_compile_inline(char *proc_source);
  extern void plpgsql_parser_setup(struct ParseState *pstate,
  					 PLpgSQL_expr *expr);
  extern bool plpgsql_parse_word(char *word1, const char *yytxt,
--- 832,838 ----
   */
  extern PLpgSQL_function *plpgsql_compile(FunctionCallInfo fcinfo,
  				bool forValidator);
! extern PLpgSQL_function *plpgsql_compile_inline(InlineCodeBlock *codeblock);
  extern void plpgsql_parser_setup(struct ParseState *pstate,
  					 PLpgSQL_expr *expr);
  extern bool plpgsql_parse_word(char *word1, const char *yytxt,
*** ./src/test/regress/expected/plpgsql.out.orig	2010-06-25 18:40:13.000000000 +0200
--- ./src/test/regress/expected/plpgsql.out	2010-07-04 08:21:55.000000000 +0200
***************
*** 4121,4123 ****
--- 4121,4141 ----
  (1 row)
  
  drop function unreserved_test();
+ -- Inline code parametrization
+ do (int := 11, int := 22) $$
+ begin
+   raise notice '%', $1 + $2;
+ end $$;
+ NOTICE:  33
+ do (a int := 11, b int := 22) $$
+ begin
+   raise notice '%', a + b;
+ end $$;
+ NOTICE:  33
+ \set myvar1 'Hello'
+ \set myvar2 'World'
+ do (msg1 text := :'myvar1', msg2 text := :'myvar2') $$
+ begin
+   raise notice '% %', msg1, msg2;
+ end $$;
+ NOTICE:  Hello World
*** ./src/test/regress/sql/plpgsql.sql.orig	2010-06-25 18:40:13.000000000 +0200
--- ./src/test/regress/sql/plpgsql.sql	2010-07-04 08:20:42.244266106 +0200
***************
*** 3268,3270 ****
--- 3268,3289 ----
  select unreserved_test();
  
  drop function unreserved_test();
+ 
+ -- Inline code parametrization
+ do (int := 11, int := 22) $$
+ begin
+   raise notice '%', $1 + $2;
+ end $$;
+ 
+ do (a int := 11, b int := 22) $$
+ begin
+   raise notice '%', a + b;
+ end $$;
+ 
+ \set myvar1 'Hello'
+ \set myvar2 'World'
+ do (msg1 text := :'myvar1', msg2 text := :'myvar2') $$
+ begin
+   raise notice '% %', msg1, msg2;
+ end $$;
+ 
#2Florian Pflug
fgp@phlo.org
In reply to: Pavel Stehule (#1)
Re: proof concept: do statement parametrization

On Jul4, 2010, at 08:41 , Pavel Stehule wrote:

I enhanced DO statement syntax to allowing a parameters. Syntax is
relative simple:

do ([varname] vartype := value, ...) $$ ... $$

I think it'd be more useful to put the values at the very end of the statement, not somewhere in the middle. For positional parameters I envision

do (vartype, ...) $$ ... $$ using value, ...

and for named parameters it'd be

do (varname vartype) $$ ... $$ using varname := value, ...

I won't make a difference for your use-case, but it'd make it easier to call the same DO block with different parameters, like in the following shell snippet.

COMMANDS="DO (arg int) $$ ... $$"
(for a in arg1, arg2, arg3, arg4; do
echo "$COMMANDS USING $a;"
done) | psql

best regards,
Florian Pflug

#3Pavel Stehule
pavel.stehule@gmail.com
In reply to: Florian Pflug (#2)
Re: proof concept: do statement parametrization

2010/7/4 Florian Pflug <fgp@phlo.org>:

On Jul4, 2010, at 08:41 , Pavel Stehule wrote:

I enhanced DO statement syntax to allowing a parameters. Syntax is
relative simple:

do ([varname] vartype := value, ...) $$ ... $$

I think it'd be more useful to put the values at the very end of the statement, not somewhere in the middle. For positional parameters I envision

do (vartype, ...) $$ ... $$ using value, ...

and for named parameters it'd be

do (varname vartype) $$ ... $$ using varname := value, ...

I won't make a difference for your use-case, but it'd make it easier to call the same DO block with different parameters, like in the following shell  snippet.

COMMANDS="DO (arg int) $$ ... $$"
(for a in arg1, arg2, arg3, arg4; do
 echo "$COMMANDS USING $a;"
done) | psql

Your syntax is longer and less readable (my personal view). With
proposed syntax it is ensured so every parameter has a value. Next -
my syntax is reflecting fact, so these are not true parameters - it's
+/- similar to default values of function parameters. You cannot to
write do (a int := $1) $$ ... $$ - because utils statements hasn't
have variables.

I understand to your motivation - but you can use a printf command and
do it same work

CMD='do(a int := %s) $$ begin raise notice ''%%'',a; end; $$'
for a in $1 $2 $3 $4
do
if [ -n "$a" ]
then
echo `printf "$CMD" $a` | psql postgres
fi
done;

or better and safer - use a psql variables (it is preferred solution)

################################
for a in $1 $2 $3 $4
do
if [ -n "$a" ]
then
psql postgres --quiet --variable a=$a <<EOT

do (a int := :a) \$\$
begin
raise notice '%', a;
end; \$\$

EOT

fi
done
###############################

psql variables can be escaped more secure - so it is prefered

for a in `cat /etc/passwd | cut -d: -f1`
do
psql postgres --quiet --variable usrname=$a <<EOT
do (usrname varchar := :'usrname') \$\$
begin
raise notice '%', usrname;
end; \$\$
EOT
done

Regards

Pavel Stehule

Show quoted text

best regards,
Florian Pflug

#4Florian Pflug
fgp@phlo.org
In reply to: Pavel Stehule (#3)
Re: proof concept: do statement parametrization

On Jul4, 2010, at 11:59 , Pavel Stehule wrote:

2010/7/4 Florian Pflug <fgp@phlo.org>:

On Jul4, 2010, at 08:41 , Pavel Stehule wrote:

I enhanced DO statement syntax to allowing a parameters. Syntax is
relative simple:

do ([varname] vartype := value, ...) $$ ... $$

I think it'd be more useful to put the values at the very end of the statement, not somewhere in the middle. For positional parameters I envision

do (vartype, ...) $$ ... $$ using value, ...

and for named parameters it'd be

do (varname vartype) $$ ... $$ using varname := value, ...

Your syntax is longer and less readable (my personal view). With
proposed syntax it is ensured so every parameter has a value. Next -
my syntax is reflecting fact, so these are not true parameters - it's
+/- similar to default values of function parameters.

Yeah, with your syntax omitting a value is syntactically invalid, while with mine it'd parse OK and fail later on. But I fail to see the drawback of that. I do agree that my suggestion is slightly more verbose, but it think thats compensated by the increase in usefulness.

I understand to your motivation - but you can use a printf command and
do it same work.

Sure. But by the very same argument, printf makes DO-block parameters redundant as a whole.

or better and safer - use a psql variables (it is preferred solution)

I don't really buy that argument. By using a psql variable, you simply move the quoting & escaping business from SQL to the shell where psql is called. True, you avoid SQL injectiont, but in turn you make yourself vulnerable to shell injection.

best regards,
Florian Pflug

#5Pavel Stehule
pavel.stehule@gmail.com
In reply to: Florian Pflug (#4)
Re: proof concept: do statement parametrization

2010/7/4 Florian Pflug <fgp@phlo.org>:

On Jul4, 2010, at 11:59 , Pavel Stehule wrote:

2010/7/4 Florian Pflug <fgp@phlo.org>:

On Jul4, 2010, at 08:41 , Pavel Stehule wrote:

I enhanced DO statement syntax to allowing a parameters. Syntax is
relative simple:

do ([varname] vartype := value, ...) $$ ... $$

I think it'd be more useful to put the values at the very end of the statement, not somewhere in the middle. For positional parameters I envision

do (vartype, ...) $$ ... $$ using value, ...

and for named parameters it'd be

do (varname vartype) $$ ... $$ using varname := value, ...

Your syntax  is longer and less readable (my personal view). With
proposed syntax it is ensured so every parameter has a value. Next -
my syntax is reflecting fact, so these are not true parameters - it's
+/- similar to default values of function parameters.

Yeah, with your syntax omitting a value is syntactically invalid, while with mine it'd parse OK and fail later on. But I fail to see the drawback of that. I do agree that my suggestion is slightly more verbose, but it think thats compensated by the increase in usefulness.

I understand to your motivation - but you can use a printf command and
do it same work.

Sure. But by the very same argument, printf makes DO-block parameters redundant as a whole.

printf isn't nice, agree - it is just workaround for some special case
- when you don't store code in variable, then you have not any
problems.

or better and safer - use a psql variables (it is preferred solution)

I don't really buy that argument. By using a psql variable, you simply move the quoting & escaping business from SQL to the shell where psql is called. True, you avoid SQL injectiont, but in turn you make yourself vulnerable to shell injection.

can you show some example of shell injection? For me, this way via
psql variables is the best. There are clean interface between outer
and inner space. And I can call simply just psql scripts - without
external bash.

best regards
Pavel

p.s. theoretically do statement can support both syntax, maybe mix of
all. It's only about 20 lines more in parser. But code will be little
bit more complex and I am not sure if it is necessary. I dislike the
space between variable definition and values - and you have to put
param list on the statement end.

Show quoted text

best regards,
Florian Pflug

#6Tom Lane
tgl@sss.pgh.pa.us
In reply to: Pavel Stehule (#3)
Re: proof concept: do statement parametrization

Pavel Stehule <pavel.stehule@gmail.com> writes:

my syntax is reflecting fact, so these are not true parameters - it's
+/- similar to default values of function parameters.

FWIW, that doesn't seem like a positive to me.

You cannot to
write do (a int := $1) $$ ... $$ - because utils statements hasn't
have variables.

Yet. I don't particularly want to relax that either, but the syntax of
this feature shouldn't assume it'll be true forever.

I think it's better to not confuse these things with default parameters,
so Florian's idea looks better to me.

BTW, we intentionally didn't put any provision for parameters into DO
originally. What's changed to alter that decision?

regards, tom lane

#7Andrew Dunstan
andrew@dunslane.net
In reply to: Tom Lane (#6)
Re: proof concept: do statement parametrization

On Sun, July 4, 2010 9:58 am, Tom Lane wrote:

BTW, we intentionally didn't put any provision for parameters into DO
originally. What's changed to alter that decision?

Nothing that I know of, I think there is just a little impatience here. I
think the consensus was that we needed to get some experience of DO in the
field before looking at a parameter mechanism. I still think that's the
correct position.

cheers

andrew

#8Pavel Stehule
pavel.stehule@gmail.com
In reply to: Tom Lane (#6)
Re: proof concept: do statement parametrization

2010/7/4 Tom Lane <tgl@sss.pgh.pa.us>:

Pavel Stehule <pavel.stehule@gmail.com> writes:

my syntax is reflecting fact, so these are not true parameters - it's
+/- similar to default values of function parameters.

FWIW, that doesn't seem like a positive to me.

You cannot to
write do (a int := $1) $$ ... $$ - because utils statements hasn't
have variables.

Yet.  I don't particularly want to relax that either, but the syntax of
this feature shouldn't assume it'll be true forever.

I think it's better to not confuse these things with default parameters,
so Florian's idea looks better to me.

BTW, we intentionally didn't put any provision for parameters into DO
originally.  What's changed to alter that decision?

                       regards, tom lane

It just concept - nothing more. And my instinct speak so inline code
block without external parametrization is useless.

Regards

Pavel Stehule

#9Pavel Stehule
pavel.stehule@gmail.com
In reply to: Tom Lane (#6)
Re: proof concept: do statement parametrization

2010/7/4 Tom Lane <tgl@sss.pgh.pa.us>:

Pavel Stehule <pavel.stehule@gmail.com> writes:

my syntax is reflecting fact, so these are not true parameters - it's
+/- similar to default values of function parameters.

FWIW, that doesn't seem like a positive to me.

You cannot to
write do (a int := $1) $$ ... $$ - because utils statements hasn't
have variables.

Yet.  I don't particularly want to relax that either, but the syntax of
this feature shouldn't assume it'll be true forever.

I think it's better to not confuse these things with default parameters,
so Florian's idea looks better to me.

Maybe I am didn't explain well my idea. The most all is modificated
named notation enhanced about type info. It isn't default parameter
definition - so I use ":=" and not use "=". And it has same advantage
like named notation has. Using a keyword "USING" isn't perfectly clean
for me - I have a problem with position of parameters - but if other
people feel it different, I'll not have a problem.

do(a int := 20, b int := 20) $$ ... $$;
do (a int, b int) $$ .... $$ USING 10,20;

generally both syntaxes are used now.

This patch is just concept - I spoke it, I would to show attractive
behave, and Florian showed possible wery nice colaboration shell with
psql. I don't want to insult somebody.

Regards
Pavel Stehule

Show quoted text

BTW, we intentionally didn't put any provision for parameters into DO
originally.  What's changed to alter that decision?

                       regards, tom lane

#10Andrew Dunstan
andrew@dunslane.net
In reply to: Pavel Stehule (#8)
Re: proof concept: do statement parametrization

Pavel Stehule wrote:

BTW, we intentionally didn't put any provision for parameters into DO
originally. What's changed to alter that decision?

It just concept - nothing more. And my instinct speak so inline code
block without external parametrization is useless.

You have said this before, IIRC, but frankly your instinct is just
wrong. It is no more useless than are parameter-less functions, and I
use those frequently. I used a DO block for some useful testing just the
other day.

This whole proposal strikes me as premature. What we need is some
experience from the field in using DO before we can sensibly decide how
it should be extended. And we won't get that until 9.0 has been released
and used for a while.

cheers

andrew

#11Pavel Stehule
pavel.stehule@gmail.com
In reply to: Andrew Dunstan (#10)
Re: proof concept: do statement parametrization

2010/7/4 Andrew Dunstan <andrew@dunslane.net>:

Pavel Stehule wrote:

BTW, we intentionally didn't put any provision for parameters into DO
originally.  What's changed to alter that decision?

It just concept - nothing more. And my instinct speak so inline code
block without external parametrization is useless.

You have said this before, IIRC, but frankly your instinct is just wrong. It
is no more useless than are parameter-less functions, and I use those
frequently. I used a DO block for some useful testing just the other day.

This whole proposal strikes me as premature. What we need is some experience
from the field in using DO before we can sensibly decide how it should be
extended. And we won't get that until 9.0 has been released and used for a
while.

just we have different opinion

Regards

Pavel

Show quoted text

cheers

andrew

#12Tom Lane
tgl@sss.pgh.pa.us
In reply to: Andrew Dunstan (#10)
Re: proof concept: do statement parametrization

Andrew Dunstan <andrew@dunslane.net> writes:

This whole proposal strikes me as premature. What we need is some
experience from the field in using DO before we can sensibly decide how
it should be extended. And we won't get that until 9.0 has been released
and used for a while.

+1.

What strikes me about this proposal is that there isn't any way to pass
parameter strings without worrying about how to escape them; which means
that the actual functionality gain over 9.0 is at best rather limited.

Now you could get to that if we had support for utility statements
accepting parameter symbols, ie you could execute
DO ... USING $1, $2
with out-of-line parameter values passed using the PQexecParams protocol.
So maybe that's an orthogonal feature that should be done as a separate
patch, but without it I'm not sure there's really much point.

IIRC one of the stumbling blocks for parameters in utility statements
is that usually there's no good context for inferring their data types.
If we were to extend DO in the particular way Pavel suggests, then
there would be context for that case, but I'm not sure what we do about
the general case. We'd want to think about that before installing a
special-purpose rule that only works for DO.

regards, tom lane

#13Andres Freund
andres@anarazel.de
In reply to: Andrew Dunstan (#10)
Re: proof concept: do statement parametrization

On Sun, Jul 04, 2010 at 11:38:47AM -0400, Andrew Dunstan wrote:

Pavel Stehule wrote:

BTW, we intentionally didn't put any provision for parameters into DO
originally. What's changed to alter that decision?

It just concept - nothing more. And my instinct speak so inline code
block without external parametrization is useless.

You have said this before, IIRC, but frankly your instinct is just
wrong. It is no more useless than are parameter-less functions, and
I use those frequently. I used a DO block for some useful testing
just the other day.

In my opinion its even *more* useful than parameterless
functions. In many cases you will use DO to write upgrade scripts or
ad-hoc code.
In both cases its not really much of diference whether you write the
parameter inside the function or outside (as a parameter to it) and
escaping is not a critical part anyway.

So maybe I am missing the point of this discussion?

Andres

#14Pavel Stehule
pavel.stehule@gmail.com
In reply to: Tom Lane (#12)
Re: proof concept: do statement parametrization

2010/7/4 Tom Lane <tgl@sss.pgh.pa.us>:

Andrew Dunstan <andrew@dunslane.net> writes:

This whole proposal strikes me as premature. What we need is some
experience from the field in using DO before we can sensibly decide how
it should be extended. And we won't get that until 9.0 has been released
and used for a while.

+1.

What strikes me about this proposal is that there isn't any way to pass
parameter strings without worrying about how to escape them; which means
that the actual functionality gain over 9.0 is at best rather limited.

Now you could get to that if we had support for utility statements
accepting parameter symbols, ie you could execute
       DO ... USING $1, $2
with out-of-line parameter values passed using the PQexecParams protocol.
So maybe that's an orthogonal feature that should be done as a separate
patch, but without it I'm not sure there's really much point.

If I remember well, you wrote so this way isn't directly possible. You
have to know a targer datatype - so you have to use syntax DO(target
type list) ... USING ... and there have to be mechanisms to put these
values to PL. Maybe you think to use only varchar variables and then
access to values via array (from PL)?

little bit different question - but I hope related to topic. I
thinking about CALL statement and "true procedures". There are three
request - transaction control, multi record sets, and using IN, OUT
parameters (compatibility issue and conformance with standard). Now I
don't know - CALL statement have to be util statement or classic plan
statement? I inclined to think so util statement can be better. But I
would to use a IN and OUT variables too - so some support for
PQexecParams protocol can be nice

CREATE OR REPLACE PROCEDURE foo(IN a int, IN b int, OUT c int)
...

and using from psql

CALL foo(10,10, :result);
\echo :result

Pavel

Show quoted text

IIRC one of the stumbling blocks for parameters in utility statements
is that usually there's no good context for inferring their data types.
If we were to extend DO in the particular way Pavel suggests, then
there would be context for that case, but I'm not sure what we do about
the general case.  We'd want to think about that before installing a
special-purpose rule that only works for DO.

                       regards, tom lane

#15Pavel Stehule
pavel.stehule@gmail.com
In reply to: Andres Freund (#13)
Re: proof concept: do statement parametrization

2010/7/4 Andres Freund <andres@anarazel.de>:

On Sun, Jul 04, 2010 at 11:38:47AM -0400, Andrew Dunstan wrote:

Pavel Stehule wrote:

BTW, we intentionally didn't put any provision for parameters into DO
originally.  What's changed to alter that decision?

It just concept - nothing more. And my instinct speak so inline code
block without external parametrization is useless.

You have said this before, IIRC, but frankly your instinct is just
wrong. It is no more useless than are parameter-less functions, and
I use those frequently. I used a DO block for some useful testing
just the other day.

In my opinion its even *more* useful than parameterless
functions. In many cases you will use DO to write upgrade scripts or
ad-hoc code.
In both cases its not really much of diference whether you write the
parameter inside the function or outside (as a parameter to it) and
escaping is not a critical part anyway.

So maybe I am missing the point of this discussion?

when the parameter are not outside, then they are not accessable from
psql. psql's variable expansion isn't working inside code literal. So
you have not any way to put some external parameters - for example -
when I would to prepare scripts for administration of databases for
some user - cleaning schema, preparing schema, etc, then I have to
write username directly to script. I cannot use a possibility of psql
to specify variables.

Regards

Pavel

Show quoted text

Andres

--
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

#16Florian Pflug
fgp@phlo.org
In reply to: Pavel Stehule (#5)
Re: proof concept: do statement parametrization

On Jul4, 2010, at 13:57 , Pavel Stehule wrote:

I don't really buy that argument. By using a psql variable, you simply move the quoting & escaping business from SQL to the shell where psql is called. True, you avoid SQL injectiont, but in turn you make yourself vulnerable to shell injection.

can you show some example of shell injection? For me, this way via
psql variables is the best. There are clean interface between outer
and inner space. And I can call simply just psql scripts - without
external bash.

Well, on the one hand you have (with your syntax)
echo "DO (a int := $VALUE) $$ ... $$" | psql
which allows sql injection if $VALUE isn't sanitized or quoted & escaped properly.

On the other hand you have
echo "DO (a int := :value) $$ ... $$$ | psql --variable value=$VALUE
which allows at least injection of additional arguments to psql if $VALUE contains spaces. You might try to avoid that by encoding value=$VALUE in double quotes, but I doubt that it's 100% safe even then.

The point is that interpolating the value into the command is always risky, independent from whether it's a shell command or an sql command.

best regards,
Florian Pflug

#17Pavel Stehule
pavel.stehule@gmail.com
In reply to: Florian Pflug (#16)
Re: proof concept: do statement parametrization

2010/7/5 Florian Pflug <fgp@phlo.org>:

On Jul4, 2010, at 13:57 , Pavel Stehule wrote:

I don't really buy that argument. By using a psql variable, you simply move the quoting & escaping business from SQL to the shell where psql is called. True, you avoid SQL injectiont, but in turn you make yourself vulnerable to shell injection.

can you show some example of shell injection? For me, this way via
psql variables is the best. There are clean interface between outer
and inner space. And I can call simply just psql scripts - without
external bash.

Well, on the one hand you have (with your syntax)
echo "DO (a int := $VALUE) $$ ... $$" | psql
which allows sql injection if $VALUE isn't sanitized or quoted & escaped properly.

sure - but it is same for you syntax, isn't it? This is classical
dynamic SQL - and more used in from untyped language.

On the other hand you have
echo "DO (a int := :value) $$ ... $$$ | psql --variable value=$VALUE
which allows at least injection of additional arguments to psql if $VALUE contains spaces. You might try to avoid that by encoding value=$VALUE in double quotes, but I doubt that it's 100% safe even then.

[pavel@nemesis ~]$ cat y.sh
a='some variable with " ajjaja" jjaja'
b='other variable with "jaja'
c="third 'variable"
psql postgres --variable a="$a" --variable b="$b" --variable c="$c" <<EOT
\echo 'a = ' :'a'
\echo 'b = ' :'b'
\echo 'c = ' :'c'
EOT
[pavel@nemesis ~]$ sh y.sh
a = 'some variable with " ajjaja" jjaja'
b = 'other variable with "jaja'
c = 'third ''variable'

it is safe - and it is only one really secure way. My design calculate with it

you can do

DO(a int := :'variable') ... and variable is well escaped and value is
casted to int. I am really very happy from :'xxx' feature.

regards

Pavel

Show quoted text

The point is that interpolating the value into the command is always risky, independent from whether it's a shell command or an sql command.

best regards,
Florian Pflug