WIP: plpgsql - foreach in

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

Hello

attached patch contains a implementation of iteration over a array:

Regards

Pavel Stehule

Attachments:

foreach.difftext/x-patch; charset=US-ASCII; name=foreach.diffDownload
*** ./doc/src/sgml/plpgsql.sgml.orig	2010-12-09 08:20:08.000000000 +0100
--- ./doc/src/sgml/plpgsql.sgml	2010-12-23 21:05:51.459678695 +0100
***************
*** 2238,2243 ****
--- 2238,2268 ----
      </para>
     </sect2>
  
+    <sect2 id="plpgsql-array-iterating">
+     <title>Looping Through Array</title>
+ <synopsis>
+ <optional> &lt;&lt;<replaceable>label</replaceable>&gt;&gt; </optional>
+ FOREACH <replaceable>target</replaceable> <optional> SCALE <replaceable>number</replaceable> </optional> IN <replaceable>expression</replaceable> LOOP
+     <replaceable>statements</replaceable>
+ END LOOP <optional> <replaceable>label</replaceable> </optional>;
+ </synopsis>
+ 
+ <programlisting>
+ CREATE FUNCTION sum(VARIADIC int[]) RETURNS int8 AS $$
+ DECLARE
+   s int8; x int;
+ BEGIN
+   FOREACH x IN $1
+   LOOP
+     s := s + x;
+   END LOOP;
+   RETURN s;
+ END;
+ $$ LANGUAGE plpgsql;
+ </programlisting>
+ 
+    </sect2>
+ 
     <sect2 id="plpgsql-error-trapping">
      <title>Trapping Errors</title>
  
*** ./src/pl/plpgsql/src/gram.y.orig	2010-12-17 09:46:55.000000000 +0100
--- ./src/pl/plpgsql/src/gram.y	2010-12-23 21:24:56.604966977 +0100
***************
*** 21,26 ****
--- 21,27 ----
  #include "parser/parse_type.h"
  #include "parser/scanner.h"
  #include "parser/scansup.h"
+ #include "utils/array.h"
  
  
  /* Location tracking support --- simpler than bison's default */
***************
*** 134,139 ****
--- 135,141 ----
  			PLpgSQL_datum   *scalar;
  			PLpgSQL_rec     *rec;
  			PLpgSQL_row     *row;
+ 			int	slice;
  		}						forvariable;
  		struct
  		{
***************
*** 178,184 ****
  %type <ival>	assign_var
  %type <var>		cursor_variable
  %type <datum>	decl_cursor_arg
! %type <forvariable>	for_variable
  %type <stmt>	for_control
  
  %type <str>		any_identifier opt_block_label opt_label
--- 180,186 ----
  %type <ival>	assign_var
  %type <var>		cursor_variable
  %type <datum>	decl_cursor_arg
! %type <forvariable>	for_variable foreach_control
  %type <stmt>	for_control
  
  %type <str>		any_identifier opt_block_label opt_label
***************
*** 190,196 ****
  %type <stmt>	stmt_return stmt_raise stmt_execsql
  %type <stmt>	stmt_dynexecute stmt_for stmt_perform stmt_getdiag
  %type <stmt>	stmt_open stmt_fetch stmt_move stmt_close stmt_null
! %type <stmt>	stmt_case
  
  %type <list>	proc_exceptions
  %type <exception_block> exception_sect
--- 192,198 ----
  %type <stmt>	stmt_return stmt_raise stmt_execsql
  %type <stmt>	stmt_dynexecute stmt_for stmt_perform stmt_getdiag
  %type <stmt>	stmt_open stmt_fetch stmt_move stmt_close stmt_null
! %type <stmt>	stmt_case stmt_foreach_a 
  
  %type <list>	proc_exceptions
  %type <exception_block> exception_sect
***************
*** 264,269 ****
--- 266,272 ----
  %token <keyword>	K_FETCH
  %token <keyword>	K_FIRST
  %token <keyword>	K_FOR
+ %token <keyword>	K_FOREACH
  %token <keyword>	K_FORWARD
  %token <keyword>	K_FROM
  %token <keyword>	K_GET
***************
*** 298,303 ****
--- 301,307 ----
  %token <keyword>	K_ROWTYPE
  %token <keyword>	K_ROW_COUNT
  %token <keyword>	K_SCROLL
+ %token <keyword>	K_SLICE
  %token <keyword>	K_SQLSTATE
  %token <keyword>	K_STRICT
  %token <keyword>	K_THEN
***************
*** 739,744 ****
--- 743,750 ----
  						{ $$ = $1; }
  				| stmt_for
  						{ $$ = $1; }
+ 				| stmt_foreach_a
+ 						{ $$ = $1; }
  				| stmt_exit
  						{ $$ = $1; }
  				| stmt_return
***************
*** 1386,1391 ****
--- 1392,1455 ----
  					}
  				;
  
+ stmt_foreach_a		: opt_block_label K_FOREACH foreach_control K_IN  expr_until_loop loop_body
+ 					{
+ 						PLpgSQL_stmt_foreach_a *new = palloc0(sizeof(PLpgSQL_stmt_foreach_a));
+ 						new->cmd_type = PLPGSQL_STMT_FOREACH_A;
+ 						new->lineno = plpgsql_location_to_lineno(@2);
+ 						new->label = $1;
+ 						new->expr = $5;
+ 						new->slice = $3.slice;
+ 						new->body = $6.stmts;
+ 
+ 						if ($3.rec)
+ 						{
+ 							new->rec = $3.rec;
+ 							check_assignable((PLpgSQL_datum *) new->rec, @3);
+ 						}
+ 						else if ($3.row)
+ 						{
+ 							new->row = $3.row;
+ 							check_assignable((PLpgSQL_datum *) new->row, @3);
+ 						}
+ 						else if ($3.scalar)
+ 						{
+ 							Assert($3.scalar->dtype == PLPGSQL_DTYPE_VAR);
+ 							new->var = (PLpgSQL_var *) $3.scalar;
+ 							check_assignable($3.scalar, @3);
+ 
+ 						}
+ 						else
+ 						{
+ 							ereport(ERROR,
+ 									(errcode(ERRCODE_SYNTAX_ERROR),
+ 									 errmsg("loop variable of loop over arrat must be a record, row variable, scalar variable or list of scalar variables"),
+ 											 parser_errposition(@3)));
+ 						}
+ 
+ 						check_labels($1, $6.end_label, $6.end_label_location);
+ 						$$ = (PLpgSQL_stmt *) new;
+ 					}
+ 				;
+ 
+ foreach_control		: for_variable
+ 					{
+ 						$$ = $1;
+ 						$$.slice = 0;
+ 					}
+ 				| for_variable K_SLICE ICONST
+ 					{
+ 						$$ = $1;
+ 						$$.slice = $3;
+ 						if ($3 < 0 || $3 >= MAXDIM)
+ 							ereport(ERROR,
+ 									(errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
+ 									 errmsg("number of slicing array dimensions (%d) exceeds the maximum allowed (%d)",
+ 												$3, MAXDIM),
+ 											 parser_errposition(@3)));
+ 					}
+ 				;
+ 
  stmt_exit		: exit_type opt_label opt_exitcond
  					{
  						PLpgSQL_stmt_exit *new;
***************
*** 2063,2068 ****
--- 2127,2133 ----
  				| K_ROW_COUNT
  				| K_ROWTYPE
  				| K_SCROLL
+ 				| K_SLICE
  				| K_SQLSTATE
  				| K_TYPE
  				| K_USE_COLUMN
*** ./src/pl/plpgsql/src/pl_exec.c.orig	2010-12-16 10:25:37.000000000 +0100
--- ./src/pl/plpgsql/src/pl_exec.c	2010-12-23 21:23:43.965428167 +0100
***************
*** 107,112 ****
--- 107,114 ----
  			   PLpgSQL_stmt_fors *stmt);
  static int exec_stmt_forc(PLpgSQL_execstate *estate,
  			   PLpgSQL_stmt_forc *stmt);
+ static int exec_stmt_foreach_a(PLpgSQL_execstate *estate,
+ 				    PLpgSQL_stmt_foreach_a *stmt);
  static int exec_stmt_open(PLpgSQL_execstate *estate,
  			   PLpgSQL_stmt_open *stmt);
  static int exec_stmt_fetch(PLpgSQL_execstate *estate,
***************
*** 1312,1317 ****
--- 1314,1323 ----
  			rc = exec_stmt_forc(estate, (PLpgSQL_stmt_forc *) stmt);
  			break;
  
+ 		case PLPGSQL_STMT_FOREACH_A:
+ 			rc = exec_stmt_foreach_a(estate, (PLpgSQL_stmt_foreach_a *) stmt);
+ 			break;
+ 
  		case PLPGSQL_STMT_EXIT:
  			rc = exec_stmt_exit(estate, (PLpgSQL_stmt_exit *) stmt);
  			break;
***************
*** 2028,2033 ****
--- 2034,2267 ----
  
  
  /* ----------
+  * exec_stmt_foreach_a			Implements loop over array
+  *
+  * ----------
+  */
+ static int 
+ exec_stmt_foreach_a(PLpgSQL_execstate *estate, PLpgSQL_stmt_foreach_a *stmt)
+ {
+ 	Datum		value;
+ 	bool		isnull;
+ 	Oid			valtype;
+ 	int	numelems = 0;
+ 	Oid 	array_typelem;
+ 	int	idx;
+ 	ArrayType	*arr;
+ 	char *ptr;
+ 	bits8	*arraynullsptr;
+ 	int16	elmlen;
+ 	bool	elmbyval;
+ 	char	elmalign;
+ 	PLpgSQL_datum *ctrl_var;
+ 	bool		found = false;
+ 	int			rc = PLPGSQL_RC_OK;
+ 	int		nitems = 1;
+ 
+ 	/* get a result of array_expr */
+ 	value = exec_eval_expr(estate, stmt->expr, &isnull, &valtype);
+ 	if (isnull)
+ 		ereport(ERROR,
+ 				(errcode(ERRCODE_NULL_VALUE_NOT_ALLOWED),
+ 				 errmsg("NULL value isn't allowed as parameter of FOREACH-IN")));
+ 
+ 	/* check a result of expression - must be a array */
+ 	array_typelem = get_element_type(valtype);
+ 
+ 	if (!OidIsValid(array_typelem))
+ 		ereport(ERROR,
+ 				(errcode(ERRCODE_DATATYPE_MISMATCH),
+ 				 errmsg("result of expression isn't array"),
+ 				 errdetail("result of expression is %s", 
+ 									format_type_be(valtype))));
+  
+ 	/* copy a result and takes some infos */
+ 	arr = DatumGetArrayTypePCopy(value);
+ 	numelems = ArrayGetNItems(ARR_NDIM(arr), ARR_DIMS(arr));
+ 	ptr = ARR_DATA_PTR(arr);
+ 	arraynullsptr = ARR_NULLBITMAP(arr);
+ 	get_typlenbyvalalign(ARR_ELEMTYPE(arr),
+ 						&elmlen,
+ 						&elmbyval,
+ 						&elmalign);
+ 
+ 	/* clean a stack */
+ 	exec_eval_cleanup(estate);
+ 
+ 	if (stmt->slice > 0)
+ 	{
+ 		if (stmt->rec != NULL || stmt->row != NULL)
+ 			ereport(ERROR,
+ 					(errcode(ERRCODE_DATATYPE_MISMATCH),
+ 					 errmsg("target variable \"%s\" isn't a of array type",
+ 							    stmt->row ? stmt->row->refname : stmt->rec->refname),
+ 					 errhint("Assigned value will be a value of array type.")));
+ 
+ 		if (stmt->slice > ARR_NDIM(arr))
+ 			ereport(ERROR,
+ 					(errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
+ 					 errmsg("slice level %d is higher than array dimension",
+ 								    stmt->slice)));
+ 	}
+ 
+ 	/* get a target variable */
+ 	if (stmt->var != NULL)
+ 	{
+ 		int	typoid = stmt->var->datatype->typoid;
+ 		int	elmoid = get_element_type(typoid);
+ 
+ 		if (stmt->slice > 0)
+ 		{
+ 			/* target variable have to be a array type */
+ 			if (elmoid == InvalidOid)
+ 				ereport(ERROR,
+ 						(errcode(ERRCODE_DATATYPE_MISMATCH),
+ 						 errmsg("target variable \"%s\" for sliced array should be a array",
+ 										stmt->var->refname)));
+ 			nitems = ArrayGetNItems(stmt->slice, ARR_DIMS(arr) + ARR_NDIM(arr) - stmt->slice);
+ 		}
+ 		else
+ 		{
+ 			/* target variable should be a scalar */
+ 			if (elmoid != InvalidOid)
+ 				ereport(ERROR,
+ 						(errcode(ERRCODE_DATATYPE_MISMATCH),
+ 						 errmsg("target variable \"%s\" is a array", 
+ 										stmt->var->refname)));
+ 		}
+ 		ctrl_var = estate->datums[stmt->var->dno];
+ 	}
+ 	else if (stmt->row != NULL)
+ 	{
+ 		ctrl_var = estate->datums[stmt->row->dno];
+ 	}
+ 	else 
+ 	{
+ 		ctrl_var = estate->datums[stmt->rec->dno];
+ 	}
+ 
+ 	/*
+ 	 * Now do the loop
+ 	 */
+ 	idx = 0;
+ 	while (idx < numelems)
+ 	{
+ 		int		j;
+ 		ArrayBuildState *astate = NULL;
+ 
+ 		found = true;				/* looped at least once */
+ 
+ 		for (j = 0; j < nitems; j++)
+ 		{
+ 			if (arraynullsptr != NULL && !(arraynullsptr[idx / 8] & (1 << (idx % 8))))
+ 			{
+ 				isnull = true;
+ 				value = (Datum) 0;
+ 			}
+ 			else
+ 			{
+ 				isnull = false;
+ 				value = fetch_att(ptr, elmbyval, elmlen);
+ 
+ 				ptr = att_addlength_pointer(ptr, elmlen, ptr);
+ 				ptr = (char *) att_align_nominal(ptr, elmalign);
+ 			}
+ 
+ 			if (stmt->slice > 0)
+ 				astate = accumArrayResult(astate, value, isnull, 
+ 									array_typelem, 
+ 									CurrentMemoryContext);
+ 			idx++;
+ 		}
+ 
+ 		/* 
+ 		 * store a item to variable - we expecting so almost all
+ 		 * iteration will be over scalar values, so it is reason
+ 		 * why we don't create a fake tuple over scalar and we 
+ 		 * don't use a exec_move_row for scalars. This is about 
+ 		 * four times faster.
+ 		 */
+ 		if (astate != NULL)
+ 		{
+ 			Datum sliced_arr;
+ 			bool isnull = false;
+ 
+ 			sliced_arr = makeMdArrayResult(astate, stmt->slice, 
+ 									    ARR_DIMS(arr) + ARR_NDIM(arr) - stmt->slice, 
+ 									    ARR_LBOUND(arr) ? ARR_LBOUND(arr) + ARR_NDIM(arr) - stmt->slice : NULL,
+ 										    CurrentMemoryContext, true);
+ 			exec_assign_value(estate, ctrl_var, sliced_arr, valtype, &isnull);
+ 		}
+ 		else
+ 			exec_assign_value(estate, ctrl_var,
+ 						    value, array_typelem, &isnull);
+ 
+ 		/*
+ 		 * Execute the statements
+ 		 */
+ 		rc = exec_stmts(estate, stmt->body);
+ 
+ 		if (rc == PLPGSQL_RC_RETURN)
+ 			break;				/* break out of the loop */
+ 		else if (rc == PLPGSQL_RC_EXIT)
+ 		{
+ 			if (estate->exitlabel == NULL)
+ 				/* unlabelled exit, finish the current loop */
+ 				rc = PLPGSQL_RC_OK;
+ 			else if (stmt->label != NULL &&
+ 					 strcmp(stmt->label, estate->exitlabel) == 0)
+ 			{
+ 				/* labelled exit, matches the current stmt's label */
+ 				estate->exitlabel = NULL;
+ 				rc = PLPGSQL_RC_OK;
+ 			}
+ 
+ 			/*
+ 			 * otherwise, this is a labelled exit that does not match the
+ 			 * current statement's label, if any: return RC_EXIT so that the
+ 			 * EXIT continues to propagate up the stack.
+ 			 */
+ 			break;
+ 		}
+ 		else if (rc == PLPGSQL_RC_CONTINUE)
+ 		{
+ 			if (estate->exitlabel == NULL)
+ 				/* unlabelled continue, so re-run the current loop */
+ 				rc = PLPGSQL_RC_OK;
+ 			else if (stmt->label != NULL &&
+ 					 strcmp(stmt->label, estate->exitlabel) == 0)
+ 			{
+ 				/* label matches named continue, so re-run loop */
+ 				estate->exitlabel = NULL;
+ 				rc = PLPGSQL_RC_OK;
+ 			}
+ 			else
+ 			{
+ 				/*
+ 				 * otherwise, this is a named continue that does not match the
+ 				 * current statement's label, if any: return RC_CONTINUE so
+ 				 * that the CONTINUE will propagate up the stack.
+ 				 */
+ 				break;
+ 			}
+ 		}
+ 	}
+ 
+ 	pfree(arr);
+ 
+ 	/*
+ 	 * Set the FOUND variable to indicate the result of executing the loop
+ 	 * (namely, whether we looped one or more times). This must be set here so
+ 	 * that it does not interfere with the value of the FOUND variable inside
+ 	 * the loop processing itself.
+ 	 */
+ 	exec_set_found(estate, found);
+ 
+ 	return rc;
+ }
+ 
+ 
+ /* ----------
   * exec_stmt_exit			Implements EXIT and CONTINUE
   *
   * This begins the process of exiting / restarting a loop.
*** ./src/pl/plpgsql/src/pl_funcs.c.orig	2010-12-16 10:06:08.000000000 +0100
--- ./src/pl/plpgsql/src/pl_funcs.c	2010-12-23 10:26:17.588404829 +0100
***************
*** 230,235 ****
--- 230,237 ----
  			return _("FOR over SELECT rows");
  		case PLPGSQL_STMT_FORC:
  			return _("FOR over cursor");
+ 		case PLPGSQL_STMT_FOREACH_A:
+ 			return _("FOREACH over array");
  		case PLPGSQL_STMT_EXIT:
  			return "EXIT";
  		case PLPGSQL_STMT_RETURN:
***************
*** 293,298 ****
--- 295,301 ----
  static void dump_close(PLpgSQL_stmt_close *stmt);
  static void dump_perform(PLpgSQL_stmt_perform *stmt);
  static void dump_expr(PLpgSQL_expr *expr);
+ static void dump_foreach_a(PLpgSQL_stmt_foreach_a *stmt);
  
  static void
  dump_ind(void)
***************
*** 375,380 ****
--- 378,386 ----
  		case PLPGSQL_STMT_PERFORM:
  			dump_perform((PLpgSQL_stmt_perform *) stmt);
  			break;
+ 		case PLPGSQL_STMT_FOREACH_A:
+ 			dump_foreach_a((PLpgSQL_stmt_foreach_a *) stmt);
+ 			break;
  		default:
  			elog(ERROR, "unrecognized cmd_type: %d", stmt->cmd_type);
  			break;
***************
*** 595,600 ****
--- 601,624 ----
  }
  
  static void
+ dump_foreach_a(PLpgSQL_stmt_foreach_a *stmt)
+ {
+ 	dump_ind();
+ 	printf("FOREACHA %s", (stmt->rec != NULL) ? stmt->rec->refname : 
+ 						    (stmt->row != NULL) ? stmt->row->refname : stmt->var->refname);
+ 	if (stmt->slice != 0)
+ 		printf("SLICE %d ", stmt->slice);
+ 	printf("IN ");
+ 	dump_expr(stmt->expr);
+ 	printf("\n");
+ 
+ 	dump_stmts(stmt->body);
+ 
+ 	dump_ind();
+ 	printf("    ENDOFOREACHA");
+ }
+ 
+ static void
  dump_open(PLpgSQL_stmt_open *stmt)
  {
  	dump_ind();
*** ./src/pl/plpgsql/src/plpgsql.h.orig	2010-12-16 09:14:42.000000000 +0100
--- ./src/pl/plpgsql/src/plpgsql.h	2010-12-23 10:19:34.716264314 +0100
***************
*** 87,92 ****
--- 87,93 ----
  	PLPGSQL_STMT_CASE,
  	PLPGSQL_STMT_LOOP,
  	PLPGSQL_STMT_WHILE,
+ 	PLPGSQL_STMT_FOREACH_A,
  	PLPGSQL_STMT_FORI,
  	PLPGSQL_STMT_FORS,
  	PLPGSQL_STMT_FORC,
***************
*** 427,432 ****
--- 428,447 ----
  
  
  typedef struct
+ {								/* FOREACH item in array loop */
+ 	int			cmd_type;
+ 	int			lineno;
+ 	char	   *label;
+ 	PLpgSQL_var *var;
+ 	PLpgSQL_rec *rec;
+ 	PLpgSQL_row *row;
+ 	PLpgSQL_expr	*expr;
+ 	int		slice;
+ 	List	   *body;			/* List of statements */
+ } PLpgSQL_stmt_foreach_a;
+ 
+ 
+ typedef struct
  {								/* FOR statement with integer loopvar	*/
  	int			cmd_type;
  	int			lineno;
*** ./src/pl/plpgsql/src/pl_scanner.c.orig	2010-12-16 09:11:11.000000000 +0100
--- ./src/pl/plpgsql/src/pl_scanner.c	2010-12-23 10:01:21.647731521 +0100
***************
*** 77,82 ****
--- 77,83 ----
  	PG_KEYWORD("exit", K_EXIT, RESERVED_KEYWORD)
  	PG_KEYWORD("fetch", K_FETCH, RESERVED_KEYWORD)
  	PG_KEYWORD("for", K_FOR, RESERVED_KEYWORD)
+ 	PG_KEYWORD("foreach", K_FOREACH, RESERVED_KEYWORD)
  	PG_KEYWORD("from", K_FROM, RESERVED_KEYWORD)
  	PG_KEYWORD("get", K_GET, RESERVED_KEYWORD)
  	PG_KEYWORD("if", K_IF, RESERVED_KEYWORD)
***************
*** 133,138 ****
--- 134,140 ----
  	PG_KEYWORD("row_count", K_ROW_COUNT, UNRESERVED_KEYWORD)
  	PG_KEYWORD("rowtype", K_ROWTYPE, UNRESERVED_KEYWORD)
  	PG_KEYWORD("scroll", K_SCROLL, UNRESERVED_KEYWORD)
+ 	PG_KEYWORD("slice", K_SLICE, UNRESERVED_KEYWORD)
  	PG_KEYWORD("sqlstate", K_SQLSTATE, UNRESERVED_KEYWORD)
  	PG_KEYWORD("type", K_TYPE, UNRESERVED_KEYWORD)
  	PG_KEYWORD("use_column", K_USE_COLUMN, UNRESERVED_KEYWORD)
*** ./src/test/regress/expected/plpgsql.out.orig	2010-12-09 08:20:10.000000000 +0100
--- ./src/test/regress/expected/plpgsql.out	2010-12-23 21:18:30.000000000 +0100
***************
*** 4240,4242 ****
--- 4240,4434 ----
  (1 row)
  
  drop function unreserved_test();
+ -- Checking a FOREACH statement
+ create function foreach_test(anyarray)
+ returns void as $$
+ declare x int;
+ begin
+   foreach x in $1
+   loop
+     raise notice '%', x;
+   end loop;
+   end;
+ $$ language plpgsql;
+ select foreach_test(ARRAY[1,2,3,4]);
+ NOTICE:  1
+ NOTICE:  2
+ NOTICE:  3
+ NOTICE:  4
+  foreach_test 
+ --------------
+  
+ (1 row)
+ 
+ select foreach_test(ARRAY[[1,2],[3,4]]);
+ NOTICE:  1
+ NOTICE:  2
+ NOTICE:  3
+ NOTICE:  4
+  foreach_test 
+ --------------
+  
+ (1 row)
+ 
+ create or replace function foreach_test(anyarray)
+ returns void as $$
+ declare x int;
+ begin
+   foreach x slice 1 in $1
+   loop
+     raise notice '%', x;
+   end loop;
+   end;
+ $$ language plpgsql;
+ -- should fail
+ select foreach_test(ARRAY[1,2,3,4]);
+ ERROR:  target variable "x" for sliced array should be a array
+ CONTEXT:  PL/pgSQL function "foreach_test" line 4 at FOREACH over array
+ select foreach_test(ARRAY[[1,2],[3,4]]);
+ ERROR:  target variable "x" for sliced array should be a array
+ CONTEXT:  PL/pgSQL function "foreach_test" line 4 at FOREACH over array
+ create or replace function foreach_test(anyarray)
+ returns void as $$
+ declare x int[];
+ begin
+   foreach x slice 1 in $1
+   loop
+     raise notice '%', x;
+   end loop;
+   end;
+ $$ language plpgsql;
+ select foreach_test(ARRAY[1,2,3,4]);
+ NOTICE:  {1,2,3,4}
+  foreach_test 
+ --------------
+  
+ (1 row)
+ 
+ select foreach_test(ARRAY[[1,2],[3,4]]);
+ NOTICE:  {1,2}
+ NOTICE:  {3,4}
+  foreach_test 
+ --------------
+  
+ (1 row)
+ 
+ -- higher level of slicing
+ create or replace function foreach_test(anyarray)
+ returns void as $$
+ declare x int[];
+ begin
+   foreach x slice 2 in $1
+   loop
+     raise notice '%', x;
+   end loop;
+   end;
+ $$ language plpgsql;
+ -- should fail
+ select foreach_test(ARRAY[1,2,3,4]);
+ ERROR:  slice level 2 is higher than array dimension
+ CONTEXT:  PL/pgSQL function "foreach_test" line 4 at FOREACH over array
+ -- ok
+ select foreach_test(ARRAY[[1,2],[3,4]]);
+ NOTICE:  {{1,2},{3,4}}
+  foreach_test 
+ --------------
+  
+ (1 row)
+ 
+ select foreach_test(ARRAY[[[1,2]],[[3,4]]]);
+ NOTICE:  {{1,2}}
+ NOTICE:  {{3,4}}
+  foreach_test 
+ --------------
+  
+ (1 row)
+ 
+ create type xy_tuple AS (x int, y int);
+ -- iteration over array of records
+ create or replace function foreach_test(anyarray)
+ returns void as $$
+ declare r record;
+ begin
+   foreach r in $1
+   loop
+     raise notice '%', r;
+   end loop;
+   end;
+ $$ language plpgsql;
+ select foreach_test(ARRAY[(10,20),(40,69),(35,78)]::xy_tuple[]);
+ NOTICE:  (10,20)
+ NOTICE:  (40,69)
+ NOTICE:  (35,78)
+  foreach_test 
+ --------------
+  
+ (1 row)
+ 
+ select foreach_test(ARRAY[[(10,20),(40,69)],[(35,78),(88,76)]]::xy_tuple[]);
+ NOTICE:  (10,20)
+ NOTICE:  (40,69)
+ NOTICE:  (35,78)
+ NOTICE:  (88,76)
+  foreach_test 
+ --------------
+  
+ (1 row)
+ 
+ create or replace function foreach_test(anyarray)
+ returns void as $$
+ declare x int; y int;
+ begin
+   foreach x, y in $1
+   loop
+     raise notice 'x = %, y = %', x, y;
+   end loop;
+   end;
+ $$ language plpgsql;
+ select foreach_test(ARRAY[(10,20),(40,69),(35,78)]::xy_tuple[]);
+ NOTICE:  x = 10, y = 20
+ NOTICE:  x = 40, y = 69
+ NOTICE:  x = 35, y = 78
+  foreach_test 
+ --------------
+  
+ (1 row)
+ 
+ select foreach_test(ARRAY[[(10,20),(40,69)],[(35,78),(88,76)]]::xy_tuple[]);
+ NOTICE:  x = 10, y = 20
+ NOTICE:  x = 40, y = 69
+ NOTICE:  x = 35, y = 78
+ NOTICE:  x = 88, y = 76
+  foreach_test 
+ --------------
+  
+ (1 row)
+ 
+ -- slicing over array of composite types
+ create or replace function foreach_test(anyarray)
+ returns void as $$
+ declare x xy_tuple[];
+ begin
+   foreach x slice 1 in $1
+   loop
+     raise notice '%', x;
+   end loop;
+   end;
+ $$ language plpgsql;
+ select foreach_test(ARRAY[(10,20),(40,69),(35,78)]::xy_tuple[]);
+ NOTICE:  {"(10,20)","(40,69)","(35,78)"}
+  foreach_test 
+ --------------
+  
+ (1 row)
+ 
+ select foreach_test(ARRAY[[(10,20),(40,69)],[(35,78),(88,76)]]::xy_tuple[]);
+ NOTICE:  {"(10,20)","(40,69)"}
+ NOTICE:  {"(35,78)","(88,76)"}
+  foreach_test 
+ --------------
+  
+ (1 row)
+ 
+ drop function foreach_test(anyarray);
+ drop type xy_tuple;
*** ./src/test/regress/sql/plpgsql.sql.orig	2010-12-09 08:20:10.000000000 +0100
--- ./src/test/regress/sql/plpgsql.sql	2010-12-23 21:17:17.852697013 +0100
***************
*** 3375,3377 ****
--- 3375,3488 ----
  select unreserved_test();
  
  drop function unreserved_test();
+ 
+ -- Checking a FOREACH statement
+ create function foreach_test(anyarray)
+ returns void as $$
+ declare x int;
+ begin
+   foreach x in $1
+   loop
+     raise notice '%', x;
+   end loop;
+   end;
+ $$ language plpgsql;
+ 
+ select foreach_test(ARRAY[1,2,3,4]);
+ select foreach_test(ARRAY[[1,2],[3,4]]);
+ 
+ create or replace function foreach_test(anyarray)
+ returns void as $$
+ declare x int;
+ begin
+   foreach x slice 1 in $1
+   loop
+     raise notice '%', x;
+   end loop;
+   end;
+ $$ language plpgsql;
+ 
+ -- should fail
+ select foreach_test(ARRAY[1,2,3,4]);
+ select foreach_test(ARRAY[[1,2],[3,4]]);
+ 
+ create or replace function foreach_test(anyarray)
+ returns void as $$
+ declare x int[];
+ begin
+   foreach x slice 1 in $1
+   loop
+     raise notice '%', x;
+   end loop;
+   end;
+ $$ language plpgsql;
+ 
+ select foreach_test(ARRAY[1,2,3,4]);
+ select foreach_test(ARRAY[[1,2],[3,4]]);
+ 
+ -- higher level of slicing
+ create or replace function foreach_test(anyarray)
+ returns void as $$
+ declare x int[];
+ begin
+   foreach x slice 2 in $1
+   loop
+     raise notice '%', x;
+   end loop;
+   end;
+ $$ language plpgsql;
+ 
+ -- should fail
+ select foreach_test(ARRAY[1,2,3,4]);
+ -- ok
+ select foreach_test(ARRAY[[1,2],[3,4]]);
+ select foreach_test(ARRAY[[[1,2]],[[3,4]]]);
+ 
+ create type xy_tuple AS (x int, y int);
+ 
+ -- iteration over array of records
+ create or replace function foreach_test(anyarray)
+ returns void as $$
+ declare r record;
+ begin
+   foreach r in $1
+   loop
+     raise notice '%', r;
+   end loop;
+   end;
+ $$ language plpgsql;
+ 
+ select foreach_test(ARRAY[(10,20),(40,69),(35,78)]::xy_tuple[]);
+ select foreach_test(ARRAY[[(10,20),(40,69)],[(35,78),(88,76)]]::xy_tuple[]);
+ 
+ create or replace function foreach_test(anyarray)
+ returns void as $$
+ declare x int; y int;
+ begin
+   foreach x, y in $1
+   loop
+     raise notice 'x = %, y = %', x, y;
+   end loop;
+   end;
+ $$ language plpgsql;
+ 
+ select foreach_test(ARRAY[(10,20),(40,69),(35,78)]::xy_tuple[]);
+ select foreach_test(ARRAY[[(10,20),(40,69)],[(35,78),(88,76)]]::xy_tuple[]);
+ 
+ -- slicing over array of composite types
+ create or replace function foreach_test(anyarray)
+ returns void as $$
+ declare x xy_tuple[];
+ begin
+   foreach x slice 1 in $1
+   loop
+     raise notice '%', x;
+   end loop;
+   end;
+ $$ language plpgsql;
+ 
+ select foreach_test(ARRAY[(10,20),(40,69),(35,78)]::xy_tuple[]);
+ select foreach_test(ARRAY[[(10,20),(40,69)],[(35,78),(88,76)]]::xy_tuple[]);
+ 
+ drop function foreach_test(anyarray);
+ drop type xy_tuple;
#2Stephen Frost
sfrost@snowman.net
In reply to: Pavel Stehule (#1)
2 attachment(s)
REVIEW: WIP: plpgsql - foreach in

Greetings,

* Pavel Stehule (pavel.stehule@gmail.com) wrote:

attached patch contains a implementation of iteration over a array:

I've gone through this patch and, in general, it looks pretty reasonable
to me. There's a number of places where I think additional comments
would be good and maybe some variable name improvments. Also, my
changes should be reviewed to make sure they make sense.

Attached is a patch against master which includes my changes, and a
patch against Pavel's patch, so he can more easily see my changes and
include them if he'd like.

I'm going to mark this returned to author with feedback.

commit 30295015739930e68c33b29da4f7ef535bc293ea
Author: Stephen Frost <sfrost@snowman.net>
Date: Wed Jan 19 17:58:24 2011 -0500

Clean up foreach-in-array PL/PgSQL code/comments

Minor clean-up of the PL/PgSQL foreach-in-array patch, includes
some white-space cleanup, grammar fixes, additional errhint where
it makes sense, etc.

Also added a number of 'XXX' comments asking for clarification
and additional comments on what's happening in the code.

commit f1a02fe3a8fa84217dae32d5ba74e9764c77431c
Author: Stephen Frost <sfrost@snowman.net>
Date: Wed Jan 19 15:11:53 2011 -0500

PL/PgSQL - Add interate-over-array support

This patch adds support for iterating over an array in PL/PgSQL.

Patch Author: Pavel Stehule

Thanks,

Stephen

Attachments:

test_plpgsql_array.patchtext/x-diff; charset=us-asciiDownload
*** a/doc/src/sgml/plpgsql.sgml
--- b/doc/src/sgml/plpgsql.sgml
***************
*** 2238,2243 **** END LOOP <optional> <replaceable>label</replaceable> </optional>;
--- 2238,2268 ----
      </para>
     </sect2>
  
+    <sect2 id="plpgsql-array-iterating">
+     <title>Looping Through Array</title>
+ <synopsis>
+ <optional> &lt;&lt;<replaceable>label</replaceable>&gt;&gt; </optional>
+ FOREACH <replaceable>target</replaceable> <optional> SCALE <replaceable>number</replaceable> </optional> IN <replaceable>expression</replaceable> LOOP
+     <replaceable>statements</replaceable>
+ END LOOP <optional> <replaceable>label</replaceable> </optional>;
+ </synopsis>
+ 
+ <programlisting>
+ CREATE FUNCTION sum(VARIADIC int[]) RETURNS int8 AS $$
+ DECLARE
+   s int8; x int;
+ BEGIN
+   FOREACH x IN $1
+   LOOP
+     s := s + x;
+   END LOOP;
+   RETURN s;
+ END;
+ $$ LANGUAGE plpgsql;
+ </programlisting>
+ 
+    </sect2>
+ 
     <sect2 id="plpgsql-error-trapping">
      <title>Trapping Errors</title>
  
*** a/src/pl/plpgsql/src/gram.y
--- b/src/pl/plpgsql/src/gram.y
***************
*** 21,26 ****
--- 21,27 ----
  #include "parser/parse_type.h"
  #include "parser/scanner.h"
  #include "parser/scansup.h"
+ #include "utils/array.h"
  
  
  /* Location tracking support --- simpler than bison's default */
***************
*** 134,139 **** static	List			*read_raise_options(void);
--- 135,141 ----
  			PLpgSQL_datum   *scalar;
  			PLpgSQL_rec     *rec;
  			PLpgSQL_row     *row;
+ 			int	slice;
  		}						forvariable;
  		struct
  		{
***************
*** 178,184 **** static	List			*read_raise_options(void);
  %type <ival>	assign_var
  %type <var>		cursor_variable
  %type <datum>	decl_cursor_arg
! %type <forvariable>	for_variable
  %type <stmt>	for_control
  
  %type <str>		any_identifier opt_block_label opt_label
--- 180,186 ----
  %type <ival>	assign_var
  %type <var>		cursor_variable
  %type <datum>	decl_cursor_arg
! %type <forvariable>	for_variable foreach_control
  %type <stmt>	for_control
  
  %type <str>		any_identifier opt_block_label opt_label
***************
*** 190,196 **** static	List			*read_raise_options(void);
  %type <stmt>	stmt_return stmt_raise stmt_execsql
  %type <stmt>	stmt_dynexecute stmt_for stmt_perform stmt_getdiag
  %type <stmt>	stmt_open stmt_fetch stmt_move stmt_close stmt_null
! %type <stmt>	stmt_case
  
  %type <list>	proc_exceptions
  %type <exception_block> exception_sect
--- 192,198 ----
  %type <stmt>	stmt_return stmt_raise stmt_execsql
  %type <stmt>	stmt_dynexecute stmt_for stmt_perform stmt_getdiag
  %type <stmt>	stmt_open stmt_fetch stmt_move stmt_close stmt_null
! %type <stmt>	stmt_case stmt_foreach_a
  
  %type <list>	proc_exceptions
  %type <exception_block> exception_sect
***************
*** 264,269 **** static	List			*read_raise_options(void);
--- 266,272 ----
  %token <keyword>	K_FETCH
  %token <keyword>	K_FIRST
  %token <keyword>	K_FOR
+ %token <keyword>	K_FOREACH
  %token <keyword>	K_FORWARD
  %token <keyword>	K_FROM
  %token <keyword>	K_GET
***************
*** 298,303 **** static	List			*read_raise_options(void);
--- 301,307 ----
  %token <keyword>	K_ROWTYPE
  %token <keyword>	K_ROW_COUNT
  %token <keyword>	K_SCROLL
+ %token <keyword>	K_SLICE
  %token <keyword>	K_SQLSTATE
  %token <keyword>	K_STRICT
  %token <keyword>	K_THEN
***************
*** 739,744 **** proc_stmt		: pl_block ';'
--- 743,750 ----
  						{ $$ = $1; }
  				| stmt_for
  						{ $$ = $1; }
+ 				| stmt_foreach_a
+ 						{ $$ = $1; }
  				| stmt_exit
  						{ $$ = $1; }
  				| stmt_return
***************
*** 1386,1391 **** for_variable	: T_DATUM
--- 1392,1455 ----
  					}
  				;
  
+ stmt_foreach_a		: opt_block_label K_FOREACH foreach_control K_IN expr_until_loop loop_body
+ 					{
+ 						PLpgSQL_stmt_foreach_a *new = palloc0(sizeof(PLpgSQL_stmt_foreach_a));
+ 						new->cmd_type = PLPGSQL_STMT_FOREACH_A;
+ 						new->lineno = plpgsql_location_to_lineno(@2);
+ 						new->label = $1;
+ 						new->expr = $5;
+ 						new->slice = $3.slice;
+ 						new->body = $6.stmts;
+ 
+ 						if ($3.rec)
+ 						{
+ 							new->rec = $3.rec;
+ 							check_assignable((PLpgSQL_datum *) new->rec, @3);
+ 						}
+ 						else if ($3.row)
+ 						{
+ 							new->row = $3.row;
+ 							check_assignable((PLpgSQL_datum *) new->row, @3);
+ 						}
+ 						else if ($3.scalar)
+ 						{
+ 							Assert($3.scalar->dtype == PLPGSQL_DTYPE_VAR);
+ 							new->var = (PLpgSQL_var *) $3.scalar;
+ 							check_assignable($3.scalar, @3);
+ 
+ 						}
+ 						else
+ 						{
+ 							ereport(ERROR,
+ 									(errcode(ERRCODE_SYNTAX_ERROR),
+ 									 errmsg("loop variable of loop over array must be a record, row variable, scalar variable or list of scalar variables"),
+ 											 parser_errposition(@3)));
+ 						}
+ 
+ 						check_labels($1, $6.end_label, $6.end_label_location);
+ 						$$ = (PLpgSQL_stmt *) new;
+ 					}
+ 				;
+ 
+ foreach_control		: for_variable
+ 					{
+ 						$$ = $1;
+ 						$$.slice = 0;
+ 					}
+ 				| for_variable K_SLICE ICONST
+ 					{
+ 						$$ = $1;
+ 						$$.slice = $3;
+ 						if ($3 < 0 || $3 >= MAXDIM)
+ 							ereport(ERROR,
+ 									(errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
+ 									 errmsg("number of slicing array dimensions (%d) exceeds the maximum allowed (%d)",
+ 												$3, MAXDIM),
+ 											 parser_errposition(@3)));
+ 					}
+ 				;
+ 
  stmt_exit		: exit_type opt_label opt_exitcond
  					{
  						PLpgSQL_stmt_exit *new;
***************
*** 2063,2068 **** unreserved_keyword	:
--- 2127,2133 ----
  				| K_ROW_COUNT
  				| K_ROWTYPE
  				| K_SCROLL
+ 				| K_SLICE
  				| K_SQLSTATE
  				| K_TYPE
  				| K_USE_COLUMN
*** a/src/pl/plpgsql/src/pl_exec.c
--- b/src/pl/plpgsql/src/pl_exec.c
***************
*** 107,112 **** static int exec_stmt_fors(PLpgSQL_execstate *estate,
--- 107,114 ----
  			   PLpgSQL_stmt_fors *stmt);
  static int exec_stmt_forc(PLpgSQL_execstate *estate,
  			   PLpgSQL_stmt_forc *stmt);
+ static int exec_stmt_foreach_a(PLpgSQL_execstate *estate,
+ 				    PLpgSQL_stmt_foreach_a *stmt);
  static int exec_stmt_open(PLpgSQL_execstate *estate,
  			   PLpgSQL_stmt_open *stmt);
  static int exec_stmt_fetch(PLpgSQL_execstate *estate,
***************
*** 1312,1317 **** exec_stmt(PLpgSQL_execstate *estate, PLpgSQL_stmt *stmt)
--- 1314,1323 ----
  			rc = exec_stmt_forc(estate, (PLpgSQL_stmt_forc *) stmt);
  			break;
  
+ 		case PLPGSQL_STMT_FOREACH_A:
+ 			rc = exec_stmt_foreach_a(estate, (PLpgSQL_stmt_foreach_a *) stmt);
+ 			break;
+ 
  		case PLPGSQL_STMT_EXIT:
  			rc = exec_stmt_exit(estate, (PLpgSQL_stmt_exit *) stmt);
  			break;
***************
*** 2028,2033 **** exec_stmt_forc(PLpgSQL_execstate *estate, PLpgSQL_stmt_forc *stmt)
--- 2034,2290 ----
  
  
  /* ----------
+  * exec_stmt_foreach_a			Implements loop over array
+  * ----------
+  */
+ static int
+ exec_stmt_foreach_a(PLpgSQL_execstate *estate, PLpgSQL_stmt_foreach_a *stmt)
+ {
+ 	Datum		value;
+ 	bool		isnull;
+ 	Oid			valtype;
+ 	int			numelems = 0;
+ 	Oid			array_typelem;
+ 	int			idx;
+ 	ArrayType  *arr;
+ 	char	   *ptr;
+ 	bits8	   *arraynullsptr;
+ 	int16		elmlen;
+ 	bool		elmbyval;
+ 	char		elmalign;
+ 	PLpgSQL_datum *ctrl_var;
+ 	bool		found = false;
+ 	int			rc = PLPGSQL_RC_OK;
+ 	int			nitems = 1;
+ 
+ 	/* get the value of the array expression using array_expr */
+ 	value = exec_eval_expr(estate, stmt->expr, &isnull, &valtype);
+ 	if (isnull)
+ 		ereport(ERROR,
+ 				(errcode(ERRCODE_NULL_VALUE_NOT_ALLOWED),
+ 				 errmsg("NULL value isn't allowed as parameter of FOREACH-IN")));
+ 
+ 	/* check the type of the expression - must be an array */
+ 	array_typelem = get_element_type(valtype);
+ 
+ 	if (!OidIsValid(array_typelem))
+ 		ereport(ERROR,
+ 				(errcode(ERRCODE_DATATYPE_MISMATCH),
+ 				 errmsg("result of expression isn't an array"),
+ 				 errdetail("result of expression is %s",
+ 									format_type_be(valtype))));
+ 
+ 	/* make a copy of the result */
+ 	arr = DatumGetArrayTypePCopy(value);
+ 
+ 	/* Calculate the number of elements we're going to loop through */
+ 	numelems = ArrayGetNItems(ARR_NDIM(arr), ARR_DIMS(arr));
+ 
+ 	/* Gather information about the array */
+ 	ptr = ARR_DATA_PTR(arr);
+ 	arraynullsptr = ARR_NULLBITMAP(arr);
+ 	get_typlenbyvalalign(ARR_ELEMTYPE(arr),
+ 						&elmlen,
+ 						&elmbyval,
+ 						&elmalign);
+ 
+ 	/* Clean up any leftover temporary memory */
+ 	exec_eval_cleanup(estate);
+ 
+ 	/*
+ 	 * If the target needs to be an array, check that it actually is,
+ 	 * and that it has a valid dimension for the array we're looping
+ 	 * through.
+ 	 *
+ 	 * XXX: Is this redundant?  Could it be merged with the below
+ 	 * set of conditionals?
+ 	 */
+ 	if (stmt->slice > 0)
+ 	{
+ 		if (stmt->rec != NULL || stmt->row != NULL)
+ 			ereport(ERROR,
+ 					(errcode(ERRCODE_DATATYPE_MISMATCH),
+ 					 errmsg("target variable \"%s\" isn't an array type",
+ 							    stmt->row ? stmt->row->refname : stmt->rec->refname),
+ 					 errhint("Value assigned will be of an array type.")));
+ 
+ 		if (stmt->slice > ARR_NDIM(arr))
+ 			ereport(ERROR,
+ 					(errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
+ 					 errmsg("slice level %d is higher than array dimension",
+ 								    stmt->slice)));
+ 	}
+ 
+ 	/* Set up the target variable */
+ 	if (stmt->var != NULL)
+ 	{
+ 		int	typoid = stmt->var->datatype->typoid;
+ 		int	elmoid = get_element_type(typoid);
+ 
+ 		if (stmt->slice > 0)
+ 		{
+ 			/* target variable has to be an array */
+ 			if (elmoid == InvalidOid)
+ 				ereport(ERROR,
+ 						(errcode(ERRCODE_DATATYPE_MISMATCH),
+ 						 errmsg("target variable \"%s\" for sliced array should be an array type",
+ 										stmt->var->refname)));
+ 
+ 			/* Determine the number of items in the slice */
+ 			nitems = ArrayGetNItems(stmt->slice, ARR_DIMS(arr) + ARR_NDIM(arr) - stmt->slice);
+ 		}
+ 		else
+ 		{
+ 			/* target variable should be a scalar */
+ 			if (elmoid != InvalidOid)
+ 				ereport(ERROR,
+ 						(errcode(ERRCODE_DATATYPE_MISMATCH),
+ 						 errmsg("target variable \"%s\" is an array",
+ 										stmt->var->refname),
+ 						 errhint("Value assigned will be of a scalar type")));
+ 		}
+ 		ctrl_var = estate->datums[stmt->var->dno];
+ 	}
+ 	else if (stmt->row != NULL)
+ 	{
+ 		ctrl_var = estate->datums[stmt->row->dno];
+ 	}
+ 	else
+ 	{
+ 		ctrl_var = estate->datums[stmt->rec->dno];
+ 	}
+ 
+ 	/*
+ 	 * Now do the loop
+ 	 *
+ 	 * XXX: These variable names are.. unfortunate.  Could we come up
+ 	 * with soe which are more descriptive?
+ 	 */
+ 	idx = 0;
+ 	while (idx < numelems)
+ 	{
+ 		int		j;
+ 		ArrayBuildState *astate = NULL;
+ 
+ 		found = true;				/* looped at least once */
+ 
+ 		/* Loop through the items in this slice to build up the
+ 		 * array slice to be used through the exec statements ... ?
+ 		 * XXX: This really needs some better commenting.. */
+ 		for (j = 0; j < nitems; j++)
+ 		{
+ 			/* XXX: Do we have a better way to check arraynullsptr?
+ 			 * Maybe a macro of some kind ..? */
+ 			if (arraynullsptr != NULL && !(arraynullsptr[idx / 8] & (1 << (idx % 8))))
+ 			{
+ 				isnull = true;
+ 				value = (Datum) 0;
+ 			}
+ 			else
+ 			{
+ 				isnull = false;
+ 				value = fetch_att(ptr, elmbyval, elmlen);
+ 
+ 				ptr = att_addlength_pointer(ptr, elmlen, ptr);
+ 				ptr = (char *) att_align_nominal(ptr, elmalign);
+ 			}
+ 
+ 			if (stmt->slice > 0)
+ 				astate = accumArrayResult(astate, value, isnull,
+ 									array_typelem,
+ 									CurrentMemoryContext);
+ 
+ 			/* XXX: Why are we updating this here..? */
+ 			idx++;
+ 		}
+ 
+ 		/*
+ 		 * store an item in to the variable - we are expecting almost all
+ 		 * iterations will be over scalar values, so we don't create
+ 		 * a fake tuple over scalar and we don't use exec_move_row for
+ 		 * scalars. This is about four times faster.
+ 		 */
+ 		if (astate != NULL)
+ 		{
+ 			Datum sliced_arr;
+ 			bool isnull = false;
+ 
+ 			sliced_arr = makeMdArrayResult(astate, stmt->slice,
+ 									    ARR_DIMS(arr) + ARR_NDIM(arr) - stmt->slice,
+ 									    ARR_LBOUND(arr) ? ARR_LBOUND(arr) + ARR_NDIM(arr) - stmt->slice : NULL,
+ 										    CurrentMemoryContext, true);
+ 			exec_assign_value(estate, ctrl_var, sliced_arr, valtype, &isnull);
+ 		}
+ 		else
+ 			exec_assign_value(estate, ctrl_var,
+ 						    value, array_typelem, &isnull);
+ 
+ 		/*
+ 		 * Execute the statements
+ 		 */
+ 		rc = exec_stmts(estate, stmt->body);
+ 
+ 		if (rc == PLPGSQL_RC_RETURN)
+ 			break;				/* break out of the loop */
+ 		else if (rc == PLPGSQL_RC_EXIT)
+ 		{
+ 			if (estate->exitlabel == NULL)
+ 				/* unlabelled exit, finish the current loop */
+ 				rc = PLPGSQL_RC_OK;
+ 			else if (stmt->label != NULL &&
+ 					 strcmp(stmt->label, estate->exitlabel) == 0)
+ 			{
+ 				/* labelled exit, matches the current stmt's label */
+ 				estate->exitlabel = NULL;
+ 				rc = PLPGSQL_RC_OK;
+ 			}
+ 
+ 			/*
+ 			 * otherwise, this is a labelled exit that does not match the
+ 			 * current statement's label, if any: return RC_EXIT so that the
+ 			 * EXIT continues to propagate up the stack.
+ 			 */
+ 			break;
+ 		}
+ 		else if (rc == PLPGSQL_RC_CONTINUE)
+ 		{
+ 			if (estate->exitlabel == NULL)
+ 				/* unlabelled continue, so re-run the current loop */
+ 				rc = PLPGSQL_RC_OK;
+ 			else if (stmt->label != NULL &&
+ 					 strcmp(stmt->label, estate->exitlabel) == 0)
+ 			{
+ 				/* label matches named continue, so re-run loop */
+ 				estate->exitlabel = NULL;
+ 				rc = PLPGSQL_RC_OK;
+ 			}
+ 			else
+ 			{
+ 				/*
+ 				 * otherwise, this is a named continue that does not match the
+ 				 * current statement's label, if any: return RC_CONTINUE so
+ 				 * that the CONTINUE will propagate up the stack.
+ 				 */
+ 				break;
+ 			}
+ 		}
+ 	}
+ 
+ 	pfree(arr);
+ 
+ 	/*
+ 	 * Set the FOUND variable to indicate the result of executing the loop
+ 	 * (namely, whether we looped one or more times). This must be set here so
+ 	 * that it does not interfere with the value of the FOUND variable inside
+ 	 * the loop processing itself.
+ 	 */
+ 	exec_set_found(estate, found);
+ 
+ 	return rc;
+ }
+ 
+ 
+ /* ----------
   * exec_stmt_exit			Implements EXIT and CONTINUE
   *
   * This begins the process of exiting / restarting a loop.
*** a/src/pl/plpgsql/src/pl_funcs.c
--- b/src/pl/plpgsql/src/pl_funcs.c
***************
*** 230,235 **** plpgsql_stmt_typename(PLpgSQL_stmt *stmt)
--- 230,237 ----
  			return _("FOR over SELECT rows");
  		case PLPGSQL_STMT_FORC:
  			return _("FOR over cursor");
+ 		case PLPGSQL_STMT_FOREACH_A:
+ 			return _("FOREACH over array");
  		case PLPGSQL_STMT_EXIT:
  			return "EXIT";
  		case PLPGSQL_STMT_RETURN:
***************
*** 293,298 **** static void dump_cursor_direction(PLpgSQL_stmt_fetch *stmt);
--- 295,301 ----
  static void dump_close(PLpgSQL_stmt_close *stmt);
  static void dump_perform(PLpgSQL_stmt_perform *stmt);
  static void dump_expr(PLpgSQL_expr *expr);
+ static void dump_foreach_a(PLpgSQL_stmt_foreach_a *stmt);
  
  
  static void
***************
*** 376,381 **** dump_stmt(PLpgSQL_stmt *stmt)
--- 379,387 ----
  		case PLPGSQL_STMT_PERFORM:
  			dump_perform((PLpgSQL_stmt_perform *) stmt);
  			break;
+ 		case PLPGSQL_STMT_FOREACH_A:
+ 			dump_foreach_a((PLpgSQL_stmt_foreach_a *) stmt);
+ 			break;
  		default:
  			elog(ERROR, "unrecognized cmd_type: %d", stmt->cmd_type);
  			break;
***************
*** 596,601 **** dump_forc(PLpgSQL_stmt_forc *stmt)
--- 602,625 ----
  }
  
  static void
+ dump_foreach_a(PLpgSQL_stmt_foreach_a *stmt)
+ {
+ 	dump_ind();
+ 	printf("FOREACHA %s", (stmt->rec != NULL) ? stmt->rec->refname :
+ 						    (stmt->row != NULL) ? stmt->row->refname : stmt->var->refname);
+ 	if (stmt->slice != 0)
+ 		printf("SLICE %d ", stmt->slice);
+ 	printf("IN ");
+ 	dump_expr(stmt->expr);
+ 	printf("\n");
+ 
+ 	dump_stmts(stmt->body);
+ 
+ 	dump_ind();
+ 	printf("    ENDOFOREACHA");
+ }
+ 
+ static void
  dump_open(PLpgSQL_stmt_open *stmt)
  {
  	dump_ind();
*** a/src/pl/plpgsql/src/pl_scanner.c
--- b/src/pl/plpgsql/src/pl_scanner.c
***************
*** 77,82 **** static const ScanKeyword reserved_keywords[] = {
--- 77,83 ----
  	PG_KEYWORD("exit", K_EXIT, RESERVED_KEYWORD)
  	PG_KEYWORD("fetch", K_FETCH, RESERVED_KEYWORD)
  	PG_KEYWORD("for", K_FOR, RESERVED_KEYWORD)
+ 	PG_KEYWORD("foreach", K_FOREACH, RESERVED_KEYWORD)
  	PG_KEYWORD("from", K_FROM, RESERVED_KEYWORD)
  	PG_KEYWORD("get", K_GET, RESERVED_KEYWORD)
  	PG_KEYWORD("if", K_IF, RESERVED_KEYWORD)
***************
*** 133,138 **** static const ScanKeyword unreserved_keywords[] = {
--- 134,140 ----
  	PG_KEYWORD("row_count", K_ROW_COUNT, UNRESERVED_KEYWORD)
  	PG_KEYWORD("rowtype", K_ROWTYPE, UNRESERVED_KEYWORD)
  	PG_KEYWORD("scroll", K_SCROLL, UNRESERVED_KEYWORD)
+ 	PG_KEYWORD("slice", K_SLICE, UNRESERVED_KEYWORD)
  	PG_KEYWORD("sqlstate", K_SQLSTATE, UNRESERVED_KEYWORD)
  	PG_KEYWORD("type", K_TYPE, UNRESERVED_KEYWORD)
  	PG_KEYWORD("use_column", K_USE_COLUMN, UNRESERVED_KEYWORD)
*** a/src/pl/plpgsql/src/plpgsql.h
--- b/src/pl/plpgsql/src/plpgsql.h
***************
*** 87,92 **** enum PLpgSQL_stmt_types
--- 87,93 ----
  	PLPGSQL_STMT_CASE,
  	PLPGSQL_STMT_LOOP,
  	PLPGSQL_STMT_WHILE,
+ 	PLPGSQL_STMT_FOREACH_A,
  	PLPGSQL_STMT_FORI,
  	PLPGSQL_STMT_FORS,
  	PLPGSQL_STMT_FORC,
***************
*** 427,432 **** typedef struct
--- 428,447 ----
  
  
  typedef struct
+ {								/* FOREACH item in array loop */
+ 	int			cmd_type;
+ 	int			lineno;
+ 	char	   *label;
+ 	PLpgSQL_var *var;
+ 	PLpgSQL_rec *rec;
+ 	PLpgSQL_row *row;
+ 	PLpgSQL_expr	*expr;
+ 	int			slice;
+ 	List	   *body;			/* List of statements */
+ } PLpgSQL_stmt_foreach_a;
+ 
+ 
+ typedef struct
  {								/* FOR statement with integer loopvar	*/
  	int			cmd_type;
  	int			lineno;
*** a/src/test/regress/expected/plpgsql.out
--- b/src/test/regress/expected/plpgsql.out
***************
*** 4240,4242 **** select unreserved_test();
--- 4240,4434 ----
  (1 row)
  
  drop function unreserved_test();
+ -- Checking a FOREACH statement
+ create function foreach_test(anyarray)
+ returns void as $$
+ declare x int;
+ begin
+   foreach x in $1
+   loop
+     raise notice '%', x;
+   end loop;
+   end;
+ $$ language plpgsql;
+ select foreach_test(ARRAY[1,2,3,4]);
+ NOTICE:  1
+ NOTICE:  2
+ NOTICE:  3
+ NOTICE:  4
+  foreach_test 
+ --------------
+  
+ (1 row)
+ 
+ select foreach_test(ARRAY[[1,2],[3,4]]);
+ NOTICE:  1
+ NOTICE:  2
+ NOTICE:  3
+ NOTICE:  4
+  foreach_test 
+ --------------
+  
+ (1 row)
+ 
+ create or replace function foreach_test(anyarray)
+ returns void as $$
+ declare x int;
+ begin
+   foreach x slice 1 in $1
+   loop
+     raise notice '%', x;
+   end loop;
+   end;
+ $$ language plpgsql;
+ -- should fail
+ select foreach_test(ARRAY[1,2,3,4]);
+ ERROR:  target variable "x" for sliced array should be an array type
+ CONTEXT:  PL/pgSQL function "foreach_test" line 4 at FOREACH over array
+ select foreach_test(ARRAY[[1,2],[3,4]]);
+ ERROR:  target variable "x" for sliced array should be an array type
+ CONTEXT:  PL/pgSQL function "foreach_test" line 4 at FOREACH over array
+ create or replace function foreach_test(anyarray)
+ returns void as $$
+ declare x int[];
+ begin
+   foreach x slice 1 in $1
+   loop
+     raise notice '%', x;
+   end loop;
+   end;
+ $$ language plpgsql;
+ select foreach_test(ARRAY[1,2,3,4]);
+ NOTICE:  {1,2,3,4}
+  foreach_test 
+ --------------
+  
+ (1 row)
+ 
+ select foreach_test(ARRAY[[1,2],[3,4]]);
+ NOTICE:  {1,2}
+ NOTICE:  {3,4}
+  foreach_test 
+ --------------
+  
+ (1 row)
+ 
+ -- higher level of slicing
+ create or replace function foreach_test(anyarray)
+ returns void as $$
+ declare x int[];
+ begin
+   foreach x slice 2 in $1
+   loop
+     raise notice '%', x;
+   end loop;
+   end;
+ $$ language plpgsql;
+ -- should fail
+ select foreach_test(ARRAY[1,2,3,4]);
+ ERROR:  slice level 2 is higher than array dimension
+ CONTEXT:  PL/pgSQL function "foreach_test" line 4 at FOREACH over array
+ -- ok
+ select foreach_test(ARRAY[[1,2],[3,4]]);
+ NOTICE:  {{1,2},{3,4}}
+  foreach_test 
+ --------------
+  
+ (1 row)
+ 
+ select foreach_test(ARRAY[[[1,2]],[[3,4]]]);
+ NOTICE:  {{1,2}}
+ NOTICE:  {{3,4}}
+  foreach_test 
+ --------------
+  
+ (1 row)
+ 
+ create type xy_tuple AS (x int, y int);
+ -- iteration over array of records
+ create or replace function foreach_test(anyarray)
+ returns void as $$
+ declare r record;
+ begin
+   foreach r in $1
+   loop
+     raise notice '%', r;
+   end loop;
+   end;
+ $$ language plpgsql;
+ select foreach_test(ARRAY[(10,20),(40,69),(35,78)]::xy_tuple[]);
+ NOTICE:  (10,20)
+ NOTICE:  (40,69)
+ NOTICE:  (35,78)
+  foreach_test 
+ --------------
+  
+ (1 row)
+ 
+ select foreach_test(ARRAY[[(10,20),(40,69)],[(35,78),(88,76)]]::xy_tuple[]);
+ NOTICE:  (10,20)
+ NOTICE:  (40,69)
+ NOTICE:  (35,78)
+ NOTICE:  (88,76)
+  foreach_test 
+ --------------
+  
+ (1 row)
+ 
+ create or replace function foreach_test(anyarray)
+ returns void as $$
+ declare x int; y int;
+ begin
+   foreach x, y in $1
+   loop
+     raise notice 'x = %, y = %', x, y;
+   end loop;
+   end;
+ $$ language plpgsql;
+ select foreach_test(ARRAY[(10,20),(40,69),(35,78)]::xy_tuple[]);
+ NOTICE:  x = 10, y = 20
+ NOTICE:  x = 40, y = 69
+ NOTICE:  x = 35, y = 78
+  foreach_test 
+ --------------
+  
+ (1 row)
+ 
+ select foreach_test(ARRAY[[(10,20),(40,69)],[(35,78),(88,76)]]::xy_tuple[]);
+ NOTICE:  x = 10, y = 20
+ NOTICE:  x = 40, y = 69
+ NOTICE:  x = 35, y = 78
+ NOTICE:  x = 88, y = 76
+  foreach_test 
+ --------------
+  
+ (1 row)
+ 
+ -- slicing over array of composite types
+ create or replace function foreach_test(anyarray)
+ returns void as $$
+ declare x xy_tuple[];
+ begin
+   foreach x slice 1 in $1
+   loop
+     raise notice '%', x;
+   end loop;
+   end;
+ $$ language plpgsql;
+ select foreach_test(ARRAY[(10,20),(40,69),(35,78)]::xy_tuple[]);
+ NOTICE:  {"(10,20)","(40,69)","(35,78)"}
+  foreach_test 
+ --------------
+  
+ (1 row)
+ 
+ select foreach_test(ARRAY[[(10,20),(40,69)],[(35,78),(88,76)]]::xy_tuple[]);
+ NOTICE:  {"(10,20)","(40,69)"}
+ NOTICE:  {"(35,78)","(88,76)"}
+  foreach_test 
+ --------------
+  
+ (1 row)
+ 
+ drop function foreach_test(anyarray);
+ drop type xy_tuple;
*** a/src/test/regress/sql/plpgsql.sql
--- b/src/test/regress/sql/plpgsql.sql
***************
*** 3375,3377 **** $$ language plpgsql;
--- 3375,3488 ----
  select unreserved_test();
  
  drop function unreserved_test();
+ 
+ -- Checking a FOREACH statement
+ create function foreach_test(anyarray)
+ returns void as $$
+ declare x int;
+ begin
+   foreach x in $1
+   loop
+     raise notice '%', x;
+   end loop;
+   end;
+ $$ language plpgsql;
+ 
+ select foreach_test(ARRAY[1,2,3,4]);
+ select foreach_test(ARRAY[[1,2],[3,4]]);
+ 
+ create or replace function foreach_test(anyarray)
+ returns void as $$
+ declare x int;
+ begin
+   foreach x slice 1 in $1
+   loop
+     raise notice '%', x;
+   end loop;
+   end;
+ $$ language plpgsql;
+ 
+ -- should fail
+ select foreach_test(ARRAY[1,2,3,4]);
+ select foreach_test(ARRAY[[1,2],[3,4]]);
+ 
+ create or replace function foreach_test(anyarray)
+ returns void as $$
+ declare x int[];
+ begin
+   foreach x slice 1 in $1
+   loop
+     raise notice '%', x;
+   end loop;
+   end;
+ $$ language plpgsql;
+ 
+ select foreach_test(ARRAY[1,2,3,4]);
+ select foreach_test(ARRAY[[1,2],[3,4]]);
+ 
+ -- higher level of slicing
+ create or replace function foreach_test(anyarray)
+ returns void as $$
+ declare x int[];
+ begin
+   foreach x slice 2 in $1
+   loop
+     raise notice '%', x;
+   end loop;
+   end;
+ $$ language plpgsql;
+ 
+ -- should fail
+ select foreach_test(ARRAY[1,2,3,4]);
+ -- ok
+ select foreach_test(ARRAY[[1,2],[3,4]]);
+ select foreach_test(ARRAY[[[1,2]],[[3,4]]]);
+ 
+ create type xy_tuple AS (x int, y int);
+ 
+ -- iteration over array of records
+ create or replace function foreach_test(anyarray)
+ returns void as $$
+ declare r record;
+ begin
+   foreach r in $1
+   loop
+     raise notice '%', r;
+   end loop;
+   end;
+ $$ language plpgsql;
+ 
+ select foreach_test(ARRAY[(10,20),(40,69),(35,78)]::xy_tuple[]);
+ select foreach_test(ARRAY[[(10,20),(40,69)],[(35,78),(88,76)]]::xy_tuple[]);
+ 
+ create or replace function foreach_test(anyarray)
+ returns void as $$
+ declare x int; y int;
+ begin
+   foreach x, y in $1
+   loop
+     raise notice 'x = %, y = %', x, y;
+   end loop;
+   end;
+ $$ language plpgsql;
+ 
+ select foreach_test(ARRAY[(10,20),(40,69),(35,78)]::xy_tuple[]);
+ select foreach_test(ARRAY[[(10,20),(40,69)],[(35,78),(88,76)]]::xy_tuple[]);
+ 
+ -- slicing over array of composite types
+ create or replace function foreach_test(anyarray)
+ returns void as $$
+ declare x xy_tuple[];
+ begin
+   foreach x slice 1 in $1
+   loop
+     raise notice '%', x;
+   end loop;
+   end;
+ $$ language plpgsql;
+ 
+ select foreach_test(ARRAY[(10,20),(40,69),(35,78)]::xy_tuple[]);
+ select foreach_test(ARRAY[[(10,20),(40,69)],[(35,78),(88,76)]]::xy_tuple[]);
+ 
+ drop function foreach_test(anyarray);
+ drop type xy_tuple;
test_plpgsql_array_cleanup.patchtext/x-diff; charset=us-asciiDownload
*** a/src/pl/plpgsql/src/gram.y
--- b/src/pl/plpgsql/src/gram.y
***************
*** 192,198 **** static	List			*read_raise_options(void);
  %type <stmt>	stmt_return stmt_raise stmt_execsql
  %type <stmt>	stmt_dynexecute stmt_for stmt_perform stmt_getdiag
  %type <stmt>	stmt_open stmt_fetch stmt_move stmt_close stmt_null
! %type <stmt>	stmt_case stmt_foreach_a 
  
  %type <list>	proc_exceptions
  %type <exception_block> exception_sect
--- 192,198 ----
  %type <stmt>	stmt_return stmt_raise stmt_execsql
  %type <stmt>	stmt_dynexecute stmt_for stmt_perform stmt_getdiag
  %type <stmt>	stmt_open stmt_fetch stmt_move stmt_close stmt_null
! %type <stmt>	stmt_case stmt_foreach_a
  
  %type <list>	proc_exceptions
  %type <exception_block> exception_sect
***************
*** 1392,1398 **** for_variable	: T_DATUM
  					}
  				;
  
! stmt_foreach_a		: opt_block_label K_FOREACH foreach_control K_IN  expr_until_loop loop_body
  					{
  						PLpgSQL_stmt_foreach_a *new = palloc0(sizeof(PLpgSQL_stmt_foreach_a));
  						new->cmd_type = PLPGSQL_STMT_FOREACH_A;
--- 1392,1398 ----
  					}
  				;
  
! stmt_foreach_a		: opt_block_label K_FOREACH foreach_control K_IN expr_until_loop loop_body
  					{
  						PLpgSQL_stmt_foreach_a *new = palloc0(sizeof(PLpgSQL_stmt_foreach_a));
  						new->cmd_type = PLPGSQL_STMT_FOREACH_A;
***************
*** 1423,1429 **** stmt_foreach_a		: opt_block_label K_FOREACH foreach_control K_IN  expr_until_loo
  						{
  							ereport(ERROR,
  									(errcode(ERRCODE_SYNTAX_ERROR),
! 									 errmsg("loop variable of loop over arrat must be a record, row variable, scalar variable or list of scalar variables"),
  											 parser_errposition(@3)));
  						}
  
--- 1423,1429 ----
  						{
  							ereport(ERROR,
  									(errcode(ERRCODE_SYNTAX_ERROR),
! 									 errmsg("loop variable of loop over array must be a record, row variable, scalar variable or list of scalar variables"),
  											 parser_errposition(@3)));
  						}
  
*** a/src/pl/plpgsql/src/pl_exec.c
--- b/src/pl/plpgsql/src/pl_exec.c
***************
*** 2035,2083 **** exec_stmt_forc(PLpgSQL_execstate *estate, PLpgSQL_stmt_forc *stmt)
  
  /* ----------
   * exec_stmt_foreach_a			Implements loop over array
-  *
   * ----------
   */
! static int 
  exec_stmt_foreach_a(PLpgSQL_execstate *estate, PLpgSQL_stmt_foreach_a *stmt)
  {
  	Datum		value;
  	bool		isnull;
  	Oid			valtype;
! 	int	numelems = 0;
! 	Oid 	array_typelem;
! 	int	idx;
! 	ArrayType	*arr;
! 	char *ptr;
! 	bits8	*arraynullsptr;
! 	int16	elmlen;
! 	bool	elmbyval;
! 	char	elmalign;
  	PLpgSQL_datum *ctrl_var;
  	bool		found = false;
  	int			rc = PLPGSQL_RC_OK;
! 	int		nitems = 1;
  
! 	/* get a result of array_expr */
  	value = exec_eval_expr(estate, stmt->expr, &isnull, &valtype);
  	if (isnull)
  		ereport(ERROR,
  				(errcode(ERRCODE_NULL_VALUE_NOT_ALLOWED),
  				 errmsg("NULL value isn't allowed as parameter of FOREACH-IN")));
  
! 	/* check a result of expression - must be a array */
  	array_typelem = get_element_type(valtype);
  
  	if (!OidIsValid(array_typelem))
  		ereport(ERROR,
  				(errcode(ERRCODE_DATATYPE_MISMATCH),
! 				 errmsg("result of expression isn't array"),
! 				 errdetail("result of expression is %s", 
  									format_type_be(valtype))));
!  
! 	/* copy a result and takes some infos */
  	arr = DatumGetArrayTypePCopy(value);
  	numelems = ArrayGetNItems(ARR_NDIM(arr), ARR_DIMS(arr));
  	ptr = ARR_DATA_PTR(arr);
  	arraynullsptr = ARR_NULLBITMAP(arr);
  	get_typlenbyvalalign(ARR_ELEMTYPE(arr),
--- 2035,2086 ----
  
  /* ----------
   * exec_stmt_foreach_a			Implements loop over array
   * ----------
   */
! static int
  exec_stmt_foreach_a(PLpgSQL_execstate *estate, PLpgSQL_stmt_foreach_a *stmt)
  {
  	Datum		value;
  	bool		isnull;
  	Oid			valtype;
! 	int			numelems = 0;
! 	Oid			array_typelem;
! 	int			idx;
! 	ArrayType  *arr;
! 	char	   *ptr;
! 	bits8	   *arraynullsptr;
! 	int16		elmlen;
! 	bool		elmbyval;
! 	char		elmalign;
  	PLpgSQL_datum *ctrl_var;
  	bool		found = false;
  	int			rc = PLPGSQL_RC_OK;
! 	int			nitems = 1;
  
! 	/* get the value of the array expression using array_expr */
  	value = exec_eval_expr(estate, stmt->expr, &isnull, &valtype);
  	if (isnull)
  		ereport(ERROR,
  				(errcode(ERRCODE_NULL_VALUE_NOT_ALLOWED),
  				 errmsg("NULL value isn't allowed as parameter of FOREACH-IN")));
  
! 	/* check the type of the expression - must be an array */
  	array_typelem = get_element_type(valtype);
  
  	if (!OidIsValid(array_typelem))
  		ereport(ERROR,
  				(errcode(ERRCODE_DATATYPE_MISMATCH),
! 				 errmsg("result of expression isn't an array"),
! 				 errdetail("result of expression is %s",
  									format_type_be(valtype))));
! 
! 	/* make a copy of the result */
  	arr = DatumGetArrayTypePCopy(value);
+ 
+ 	/* Calculate the number of elements we're going to loop through */
  	numelems = ArrayGetNItems(ARR_NDIM(arr), ARR_DIMS(arr));
+ 
+ 	/* Gather information about the array */
  	ptr = ARR_DATA_PTR(arr);
  	arraynullsptr = ARR_NULLBITMAP(arr);
  	get_typlenbyvalalign(ARR_ELEMTYPE(arr),
***************
*** 2085,2101 **** exec_stmt_foreach_a(PLpgSQL_execstate *estate, PLpgSQL_stmt_foreach_a *stmt)
  						&elmbyval,
  						&elmalign);
  
! 	/* clean a stack */
  	exec_eval_cleanup(estate);
  
  	if (stmt->slice > 0)
  	{
  		if (stmt->rec != NULL || stmt->row != NULL)
  			ereport(ERROR,
  					(errcode(ERRCODE_DATATYPE_MISMATCH),
! 					 errmsg("target variable \"%s\" isn't a of array type",
  							    stmt->row ? stmt->row->refname : stmt->rec->refname),
! 					 errhint("Assigned value will be a value of array type.")));
  
  		if (stmt->slice > ARR_NDIM(arr))
  			ereport(ERROR,
--- 2088,2112 ----
  						&elmbyval,
  						&elmalign);
  
! 	/* Clean up any leftover temporary memory */
  	exec_eval_cleanup(estate);
  
+ 	/*
+ 	 * If the target needs to be an array, check that it actually is,
+ 	 * and that it has a valid dimension for the array we're looping
+ 	 * through.
+ 	 *
+ 	 * XXX: Is this redundant?  Could it be merged with the below
+ 	 * set of conditionals?
+ 	 */
  	if (stmt->slice > 0)
  	{
  		if (stmt->rec != NULL || stmt->row != NULL)
  			ereport(ERROR,
  					(errcode(ERRCODE_DATATYPE_MISMATCH),
! 					 errmsg("target variable \"%s\" isn't an array type",
  							    stmt->row ? stmt->row->refname : stmt->rec->refname),
! 					 errhint("Value assigned will be of an array type.")));
  
  		if (stmt->slice > ARR_NDIM(arr))
  			ereport(ERROR,
***************
*** 2104,2110 **** exec_stmt_foreach_a(PLpgSQL_execstate *estate, PLpgSQL_stmt_foreach_a *stmt)
  								    stmt->slice)));
  	}
  
! 	/* get a target variable */
  	if (stmt->var != NULL)
  	{
  		int	typoid = stmt->var->datatype->typoid;
--- 2115,2121 ----
  								    stmt->slice)));
  	}
  
! 	/* Set up the target variable */
  	if (stmt->var != NULL)
  	{
  		int	typoid = stmt->var->datatype->typoid;
***************
*** 2112,2123 **** exec_stmt_foreach_a(PLpgSQL_execstate *estate, PLpgSQL_stmt_foreach_a *stmt)
  
  		if (stmt->slice > 0)
  		{
! 			/* target variable have to be a array type */
  			if (elmoid == InvalidOid)
  				ereport(ERROR,
  						(errcode(ERRCODE_DATATYPE_MISMATCH),
! 						 errmsg("target variable \"%s\" for sliced array should be a array",
  										stmt->var->refname)));
  			nitems = ArrayGetNItems(stmt->slice, ARR_DIMS(arr) + ARR_NDIM(arr) - stmt->slice);
  		}
  		else
--- 2123,2136 ----
  
  		if (stmt->slice > 0)
  		{
! 			/* target variable has to be an array */
  			if (elmoid == InvalidOid)
  				ereport(ERROR,
  						(errcode(ERRCODE_DATATYPE_MISMATCH),
! 						 errmsg("target variable \"%s\" for sliced array should be an array type",
  										stmt->var->refname)));
+ 
+ 			/* Determine the number of items in the slice */
  			nitems = ArrayGetNItems(stmt->slice, ARR_DIMS(arr) + ARR_NDIM(arr) - stmt->slice);
  		}
  		else
***************
*** 2126,2133 **** exec_stmt_foreach_a(PLpgSQL_execstate *estate, PLpgSQL_stmt_foreach_a *stmt)
  			if (elmoid != InvalidOid)
  				ereport(ERROR,
  						(errcode(ERRCODE_DATATYPE_MISMATCH),
! 						 errmsg("target variable \"%s\" is a array", 
! 										stmt->var->refname)));
  		}
  		ctrl_var = estate->datums[stmt->var->dno];
  	}
--- 2139,2147 ----
  			if (elmoid != InvalidOid)
  				ereport(ERROR,
  						(errcode(ERRCODE_DATATYPE_MISMATCH),
! 						 errmsg("target variable \"%s\" is an array",
! 										stmt->var->refname),
! 						 errhint("Value assigned will be of a scalar type")));
  		}
  		ctrl_var = estate->datums[stmt->var->dno];
  	}
***************
*** 2135,2147 **** exec_stmt_foreach_a(PLpgSQL_execstate *estate, PLpgSQL_stmt_foreach_a *stmt)
  	{
  		ctrl_var = estate->datums[stmt->row->dno];
  	}
! 	else 
  	{
  		ctrl_var = estate->datums[stmt->rec->dno];
  	}
  
  	/*
  	 * Now do the loop
  	 */
  	idx = 0;
  	while (idx < numelems)
--- 2149,2164 ----
  	{
  		ctrl_var = estate->datums[stmt->row->dno];
  	}
! 	else
  	{
  		ctrl_var = estate->datums[stmt->rec->dno];
  	}
  
  	/*
  	 * Now do the loop
+ 	 *
+ 	 * XXX: These variable names are.. unfortunate.  Could we come up
+ 	 * with soe which are more descriptive?
  	 */
  	idx = 0;
  	while (idx < numelems)
***************
*** 2151,2158 **** exec_stmt_foreach_a(PLpgSQL_execstate *estate, PLpgSQL_stmt_foreach_a *stmt)
--- 2168,2180 ----
  
  		found = true;				/* looped at least once */
  
+ 		/* Loop through the items in this slice to build up the
+ 		 * array slice to be used through the exec statements ... ?
+ 		 * XXX: This really needs some better commenting.. */
  		for (j = 0; j < nitems; j++)
  		{
+ 			/* XXX: Do we have a better way to check arraynullsptr?
+ 			 * Maybe a macro of some kind ..? */
  			if (arraynullsptr != NULL && !(arraynullsptr[idx / 8] & (1 << (idx % 8))))
  			{
  				isnull = true;
***************
*** 2168,2193 **** exec_stmt_foreach_a(PLpgSQL_execstate *estate, PLpgSQL_stmt_foreach_a *stmt)
  			}
  
  			if (stmt->slice > 0)
! 				astate = accumArrayResult(astate, value, isnull, 
! 									array_typelem, 
  									CurrentMemoryContext);
  			idx++;
  		}
  
! 		/* 
! 		 * store a item to variable - we expecting so almost all
! 		 * iteration will be over scalar values, so it is reason
! 		 * why we don't create a fake tuple over scalar and we 
! 		 * don't use a exec_move_row for scalars. This is about 
! 		 * four times faster.
  		 */
  		if (astate != NULL)
  		{
  			Datum sliced_arr;
  			bool isnull = false;
  
! 			sliced_arr = makeMdArrayResult(astate, stmt->slice, 
! 									    ARR_DIMS(arr) + ARR_NDIM(arr) - stmt->slice, 
  									    ARR_LBOUND(arr) ? ARR_LBOUND(arr) + ARR_NDIM(arr) - stmt->slice : NULL,
  										    CurrentMemoryContext, true);
  			exec_assign_value(estate, ctrl_var, sliced_arr, valtype, &isnull);
--- 2190,2216 ----
  			}
  
  			if (stmt->slice > 0)
! 				astate = accumArrayResult(astate, value, isnull,
! 									array_typelem,
  									CurrentMemoryContext);
+ 
+ 			/* XXX: Why are we updating this here..? */
  			idx++;
  		}
  
! 		/*
! 		 * store an item in to the variable - we are expecting almost all
! 		 * iterations will be over scalar values, so we don't create
! 		 * a fake tuple over scalar and we don't use exec_move_row for
! 		 * scalars. This is about four times faster.
  		 */
  		if (astate != NULL)
  		{
  			Datum sliced_arr;
  			bool isnull = false;
  
! 			sliced_arr = makeMdArrayResult(astate, stmt->slice,
! 									    ARR_DIMS(arr) + ARR_NDIM(arr) - stmt->slice,
  									    ARR_LBOUND(arr) ? ARR_LBOUND(arr) + ARR_NDIM(arr) - stmt->slice : NULL,
  										    CurrentMemoryContext, true);
  			exec_assign_value(estate, ctrl_var, sliced_arr, valtype, &isnull);
*** a/src/pl/plpgsql/src/pl_funcs.c
--- b/src/pl/plpgsql/src/pl_funcs.c
***************
*** 605,611 **** static void
  dump_foreach_a(PLpgSQL_stmt_foreach_a *stmt)
  {
  	dump_ind();
! 	printf("FOREACHA %s", (stmt->rec != NULL) ? stmt->rec->refname : 
  						    (stmt->row != NULL) ? stmt->row->refname : stmt->var->refname);
  	if (stmt->slice != 0)
  		printf("SLICE %d ", stmt->slice);
--- 605,611 ----
  dump_foreach_a(PLpgSQL_stmt_foreach_a *stmt)
  {
  	dump_ind();
! 	printf("FOREACHA %s", (stmt->rec != NULL) ? stmt->rec->refname :
  						    (stmt->row != NULL) ? stmt->row->refname : stmt->var->refname);
  	if (stmt->slice != 0)
  		printf("SLICE %d ", stmt->slice);
*** a/src/pl/plpgsql/src/plpgsql.h
--- b/src/pl/plpgsql/src/plpgsql.h
***************
*** 436,442 **** typedef struct
  	PLpgSQL_rec *rec;
  	PLpgSQL_row *row;
  	PLpgSQL_expr	*expr;
! 	int		slice;
  	List	   *body;			/* List of statements */
  } PLpgSQL_stmt_foreach_a;
  
--- 436,442 ----
  	PLpgSQL_rec *rec;
  	PLpgSQL_row *row;
  	PLpgSQL_expr	*expr;
! 	int			slice;
  	List	   *body;			/* List of statements */
  } PLpgSQL_stmt_foreach_a;
  
*** a/src/test/regress/expected/plpgsql.out
--- b/src/test/regress/expected/plpgsql.out
***************
*** 4283,4292 **** begin
  $$ language plpgsql;
  -- should fail
  select foreach_test(ARRAY[1,2,3,4]);
! ERROR:  target variable "x" for sliced array should be a array
  CONTEXT:  PL/pgSQL function "foreach_test" line 4 at FOREACH over array
  select foreach_test(ARRAY[[1,2],[3,4]]);
! ERROR:  target variable "x" for sliced array should be a array
  CONTEXT:  PL/pgSQL function "foreach_test" line 4 at FOREACH over array
  create or replace function foreach_test(anyarray)
  returns void as $$
--- 4283,4292 ----
  $$ language plpgsql;
  -- should fail
  select foreach_test(ARRAY[1,2,3,4]);
! ERROR:  target variable "x" for sliced array should be an array type
  CONTEXT:  PL/pgSQL function "foreach_test" line 4 at FOREACH over array
  select foreach_test(ARRAY[[1,2],[3,4]]);
! ERROR:  target variable "x" for sliced array should be an array type
  CONTEXT:  PL/pgSQL function "foreach_test" line 4 at FOREACH over array
  create or replace function foreach_test(anyarray)
  returns void as $$
#3Robert Haas
robertmhaas@gmail.com
In reply to: Stephen Frost (#2)
Re: REVIEW: WIP: plpgsql - foreach in

On Wed, Jan 19, 2011 at 6:04 PM, Stephen Frost <sfrost@snowman.net> wrote:

I'm going to mark this returned to author with feedback.

That implies you don't think it should be considered further for this
CommitFest. Perhaps you mean Waiting on Author?

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

#4Stephen Frost
sfrost@snowman.net
In reply to: Robert Haas (#3)
Re: REVIEW: WIP: plpgsql - foreach in

* Robert Haas (robertmhaas@gmail.com) wrote:

On Wed, Jan 19, 2011 at 6:04 PM, Stephen Frost <sfrost@snowman.net> wrote:

I'm going to mark this returned to author with feedback.

That implies you don't think it should be considered further for this
CommitFest. Perhaps you mean Waiting on Author?

I did, actually, and that's what I actually marked it as in the CF.
Sorry for any confusion. When I went to mark it in CF, I realized my
mistake.

Thanks,

Stephen

#5Pavel Stehule
pavel.stehule@gmail.com
In reply to: Stephen Frost (#4)
Re: REVIEW: WIP: plpgsql - foreach in

2011/1/20 Stephen Frost <sfrost@snowman.net>:

* Robert Haas (robertmhaas@gmail.com) wrote:

On Wed, Jan 19, 2011 at 6:04 PM, Stephen Frost <sfrost@snowman.net> wrote:

I'm going to mark this returned to author with feedback.

That implies you don't think it should be considered further for this
CommitFest.  Perhaps you mean Waiting on Author?

I did, actually, and that's what I actually marked it as in the CF.
Sorry for any confusion.  When I went to mark it in CF, I realized my
mistake.

ok :), I'll look on it tomorrow.

regards

Pavel

Show quoted text

       Thanks,

               Stephen

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)

iEYEARECAAYFAk03ltkACgkQrzgMPqB3kihSmQCePy6+fpC7RJdki5guPRCLp5IZ
EJMAoIqgjb+IsG853/gC9T9xgFg5M5aM
=VLWh
-----END PGP SIGNATURE-----

#6Pavel Stehule
pavel.stehule@gmail.com
In reply to: Stephen Frost (#2)
1 attachment(s)
Re: REVIEW: WIP: plpgsql - foreach in

Hello

I merge your changes and little enhanced comments.

Regards

Pavel Stehule

2011/1/20 Stephen Frost <sfrost@snowman.net>:

Show quoted text

Greetings,

* Pavel Stehule (pavel.stehule@gmail.com) wrote:

attached patch contains a implementation of iteration over a array:

I've gone through this patch and, in general, it looks pretty reasonable
to me.  There's a number of places where I think additional comments
would be good and maybe some variable name improvments.  Also, my
changes should be reviewed to make sure they make sense.

Attached is a patch against master which includes my changes, and a
patch against Pavel's patch, so he can more easily see my changes and
include them if he'd like.

I'm going to mark this returned to author with feedback.

commit 30295015739930e68c33b29da4f7ef535bc293ea
Author: Stephen Frost <sfrost@snowman.net>
Date:   Wed Jan 19 17:58:24 2011 -0500

   Clean up foreach-in-array PL/PgSQL code/comments

   Minor clean-up of the PL/PgSQL foreach-in-array patch, includes
   some white-space cleanup, grammar fixes, additional errhint where
   it makes sense, etc.

   Also added a number of 'XXX' comments asking for clarification
   and additional comments on what's happening in the code.

commit f1a02fe3a8fa84217dae32d5ba74e9764c77431c
Author: Stephen Frost <sfrost@snowman.net>
Date:   Wed Jan 19 15:11:53 2011 -0500

   PL/PgSQL - Add interate-over-array support

   This patch adds support for iterating over an array in PL/PgSQL.

   Patch Author: Pavel Stehule

       Thanks,

               Stephen

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)

iEYEARECAAYFAk03bf8ACgkQrzgMPqB3kihxuwCfZYKFpEraRCIltlUeYtD9AyX0
tvoAnjuxddXhZB6w2/V9oVSD1+K7Idu9
=w38Z
-----END PGP SIGNATURE-----

Attachments:

foreach.difftext/x-patch; charset=US-ASCII; name=foreach.diffDownload
*** ./doc/src/sgml/plpgsql.sgml.orig	2011-01-16 14:18:58.000000000 +0100
--- ./doc/src/sgml/plpgsql.sgml	2011-01-17 11:31:54.086217514 +0100
***************
*** 2238,2243 ****
--- 2238,2268 ----
      </para>
     </sect2>
  
+    <sect2 id="plpgsql-array-iterating">
+     <title>Looping Through Array</title>
+ <synopsis>
+ <optional> &lt;&lt;<replaceable>label</replaceable>&gt;&gt; </optional>
+ FOREACH <replaceable>target</replaceable> <optional> SCALE <replaceable>number</replaceable> </optional> IN <replaceable>expression</replaceable> LOOP
+     <replaceable>statements</replaceable>
+ END LOOP <optional> <replaceable>label</replaceable> </optional>;
+ </synopsis>
+ 
+ <programlisting>
+ CREATE FUNCTION sum(VARIADIC int[]) RETURNS int8 AS $$
+ DECLARE
+   s int8; x int;
+ BEGIN
+   FOREACH x IN $1
+   LOOP
+     s := s + x;
+   END LOOP;
+   RETURN s;
+ END;
+ $$ LANGUAGE plpgsql;
+ </programlisting>
+ 
+    </sect2>
+ 
     <sect2 id="plpgsql-error-trapping">
      <title>Trapping Errors</title>
  
*** ./src/backend/utils/adt/arrayfuncs.c.orig	2011-01-16 14:18:58.000000000 +0100
--- ./src/backend/utils/adt/arrayfuncs.c	2011-01-21 22:59:00.315522289 +0100
***************
*** 68,74 ****
  			 Datum *values, bool *nulls, int nitems,
  			 int typlen, bool typbyval, char typalign,
  			 bool freedata);
- static bool array_get_isnull(const bits8 *nullbitmap, int offset);
  static void array_set_isnull(bits8 *nullbitmap, int offset, bool isNull);
  static Datum ArrayCast(char *value, bool byval, int len);
  static int ArrayCastAndSet(Datum src,
--- 68,73 ----
***************
*** 3836,3842 ****
   * nullbitmap: pointer to array's null bitmap (NULL if none)
   * offset: 0-based linear element number of array element
   */
! static bool
  array_get_isnull(const bits8 *nullbitmap, int offset)
  {
  	if (nullbitmap == NULL)
--- 3835,3841 ----
   * nullbitmap: pointer to array's null bitmap (NULL if none)
   * offset: 0-based linear element number of array element
   */
! bool
  array_get_isnull(const bits8 *nullbitmap, int offset)
  {
  	if (nullbitmap == NULL)
*** ./src/include/utils/array.h.orig	2011-01-16 14:18:59.000000000 +0100
--- ./src/include/utils/array.h	2011-01-21 23:03:07.294898398 +0100
***************
*** 215,220 ****
--- 215,221 ----
  extern ArrayType *array_set(ArrayType *array, int nSubscripts, int *indx,
  		  Datum dataValue, bool isNull,
  		  int arraytyplen, int elmlen, bool elmbyval, char elmalign);
+ extern bool array_get_isnull(const bits8 *nullbitmap, int offset);
  extern ArrayType *array_get_slice(ArrayType *array, int nSubscripts,
  				int *upperIndx, int *lowerIndx,
  				int arraytyplen, int elmlen, bool elmbyval, char elmalign);
*** ./src/pl/plpgsql/src/gram.y.orig	2011-01-16 14:18:59.000000000 +0100
--- ./src/pl/plpgsql/src/gram.y	2011-01-21 21:35:21.959579809 +0100
***************
*** 21,26 ****
--- 21,27 ----
  #include "parser/parse_type.h"
  #include "parser/scanner.h"
  #include "parser/scansup.h"
+ #include "utils/array.h"
  
  
  /* Location tracking support --- simpler than bison's default */
***************
*** 134,139 ****
--- 135,141 ----
  			PLpgSQL_datum   *scalar;
  			PLpgSQL_rec     *rec;
  			PLpgSQL_row     *row;
+ 			int	slice;
  		}						forvariable;
  		struct
  		{
***************
*** 178,184 ****
  %type <ival>	assign_var
  %type <var>		cursor_variable
  %type <datum>	decl_cursor_arg
! %type <forvariable>	for_variable
  %type <stmt>	for_control
  
  %type <str>		any_identifier opt_block_label opt_label
--- 180,186 ----
  %type <ival>	assign_var
  %type <var>		cursor_variable
  %type <datum>	decl_cursor_arg
! %type <forvariable>	for_variable foreach_control
  %type <stmt>	for_control
  
  %type <str>		any_identifier opt_block_label opt_label
***************
*** 190,196 ****
  %type <stmt>	stmt_return stmt_raise stmt_execsql
  %type <stmt>	stmt_dynexecute stmt_for stmt_perform stmt_getdiag
  %type <stmt>	stmt_open stmt_fetch stmt_move stmt_close stmt_null
! %type <stmt>	stmt_case
  
  %type <list>	proc_exceptions
  %type <exception_block> exception_sect
--- 192,198 ----
  %type <stmt>	stmt_return stmt_raise stmt_execsql
  %type <stmt>	stmt_dynexecute stmt_for stmt_perform stmt_getdiag
  %type <stmt>	stmt_open stmt_fetch stmt_move stmt_close stmt_null
! %type <stmt>	stmt_case stmt_foreach_a
  
  %type <list>	proc_exceptions
  %type <exception_block> exception_sect
***************
*** 264,269 ****
--- 266,272 ----
  %token <keyword>	K_FETCH
  %token <keyword>	K_FIRST
  %token <keyword>	K_FOR
+ %token <keyword>	K_FOREACH
  %token <keyword>	K_FORWARD
  %token <keyword>	K_FROM
  %token <keyword>	K_GET
***************
*** 298,303 ****
--- 301,307 ----
  %token <keyword>	K_ROWTYPE
  %token <keyword>	K_ROW_COUNT
  %token <keyword>	K_SCROLL
+ %token <keyword>	K_SLICE
  %token <keyword>	K_SQLSTATE
  %token <keyword>	K_STRICT
  %token <keyword>	K_THEN
***************
*** 739,744 ****
--- 743,750 ----
  						{ $$ = $1; }
  				| stmt_for
  						{ $$ = $1; }
+ 				| stmt_foreach_a
+ 						{ $$ = $1; }
  				| stmt_exit
  						{ $$ = $1; }
  				| stmt_return
***************
*** 1386,1391 ****
--- 1392,1455 ----
  					}
  				;
  
+ stmt_foreach_a		: opt_block_label K_FOREACH foreach_control K_IN expr_until_loop loop_body
+ 					{
+ 						PLpgSQL_stmt_foreach_a *new = palloc0(sizeof(PLpgSQL_stmt_foreach_a));
+ 						new->cmd_type = PLPGSQL_STMT_FOREACH_A;
+ 						new->lineno = plpgsql_location_to_lineno(@2);
+ 						new->label = $1;
+ 						new->expr = $5;
+ 						new->slice = $3.slice;
+ 						new->body = $6.stmts;
+ 
+ 						if ($3.rec)
+ 						{
+ 							new->rec = $3.rec;
+ 							check_assignable((PLpgSQL_datum *) new->rec, @3);
+ 						}
+ 						else if ($3.row)
+ 						{
+ 							new->row = $3.row;
+ 							check_assignable((PLpgSQL_datum *) new->row, @3);
+ 						}
+ 						else if ($3.scalar)
+ 						{
+ 							Assert($3.scalar->dtype == PLPGSQL_DTYPE_VAR);
+ 							new->var = (PLpgSQL_var *) $3.scalar;
+ 							check_assignable($3.scalar, @3);
+ 
+ 						}
+ 						else
+ 						{
+ 							ereport(ERROR,
+ 									(errcode(ERRCODE_SYNTAX_ERROR),
+ 									 errmsg("loop variable of loop over array must be a record, row variable, scalar variable or list of scalar variables"),
+ 											 parser_errposition(@3)));
+ 						}
+ 
+ 						check_labels($1, $6.end_label, $6.end_label_location);
+ 						$$ = (PLpgSQL_stmt *) new;
+ 					}
+ 				;
+ 
+ foreach_control		: for_variable
+ 					{
+ 						$$ = $1;
+ 						$$.slice = 0;
+ 					}
+ 				| for_variable K_SLICE ICONST
+ 					{
+ 						$$ = $1;
+ 						$$.slice = $3;
+ 						if ($3 < 0 || $3 >= MAXDIM)
+ 							ereport(ERROR,
+ 									(errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
+ 									 errmsg("number of slicing array dimensions (%d) exceeds the maximum allowed (%d)",
+ 												$3, MAXDIM),
+ 											 parser_errposition(@3)));
+ 					}
+ 				;
+ 
  stmt_exit		: exit_type opt_label opt_exitcond
  					{
  						PLpgSQL_stmt_exit *new;
***************
*** 2063,2068 ****
--- 2127,2133 ----
  				| K_ROW_COUNT
  				| K_ROWTYPE
  				| K_SCROLL
+ 				| K_SLICE
  				| K_SQLSTATE
  				| K_TYPE
  				| K_USE_COLUMN
*** ./src/pl/plpgsql/src/pl_exec.c.orig	2011-01-16 14:18:59.000000000 +0100
--- ./src/pl/plpgsql/src/pl_exec.c	2011-01-21 22:54:46.406350001 +0100
***************
*** 107,112 ****
--- 107,114 ----
  			   PLpgSQL_stmt_fors *stmt);
  static int exec_stmt_forc(PLpgSQL_execstate *estate,
  			   PLpgSQL_stmt_forc *stmt);
+ static int exec_stmt_foreach_a(PLpgSQL_execstate *estate,
+ 				    PLpgSQL_stmt_foreach_a *stmt);
  static int exec_stmt_open(PLpgSQL_execstate *estate,
  			   PLpgSQL_stmt_open *stmt);
  static int exec_stmt_fetch(PLpgSQL_execstate *estate,
***************
*** 1312,1317 ****
--- 1314,1323 ----
  			rc = exec_stmt_forc(estate, (PLpgSQL_stmt_forc *) stmt);
  			break;
  
+ 		case PLPGSQL_STMT_FOREACH_A:
+ 			rc = exec_stmt_foreach_a(estate, (PLpgSQL_stmt_foreach_a *) stmt);
+ 			break;
+ 
  		case PLPGSQL_STMT_EXIT:
  			rc = exec_stmt_exit(estate, (PLpgSQL_stmt_exit *) stmt);
  			break;
***************
*** 2028,2033 ****
--- 2034,2295 ----
  
  
  /* ----------
+  * exec_stmt_foreach_a			Implements loop over array
+  *
+  * ----------
+  */
+ static int
+ exec_stmt_foreach_a(PLpgSQL_execstate *estate, PLpgSQL_stmt_foreach_a *stmt)
+ {
+ 	Datum		value;
+ 	bool		isnull;
+ 	Oid		valtype;
+ 	int		nitems;
+ 	int		slice_len = 1;
+ 	Oid 		array_typelem;
+ 	int		i;
+ 	ArrayType	*arr;
+ 	char		*ptr;
+ 	bits8		*arraynullsptr;
+ 	int16		elmlen;
+ 	bool		elmbyval;
+ 	char		elmalign;
+ 	PLpgSQL_datum	*ctrl_var = NULL;		/* be compiler quite */
+ 	bool		found = false;
+ 	int		rc = PLPGSQL_RC_OK;
+ 
+ 	/* get the value of the array expression using array_expr */
+ 	value = exec_eval_expr(estate, stmt->expr, &isnull, &valtype);
+ 	if (isnull)
+ 		ereport(ERROR,
+ 				(errcode(ERRCODE_NULL_VALUE_NOT_ALLOWED),
+ 				 errmsg("NULL value isn't allowed as parameter of FOREACH-IN")));
+ 
+ 	/* check the type of the expression - must be an array */
+ 	array_typelem = get_element_type(valtype);
+ 
+ 	if (!OidIsValid(array_typelem))
+ 		ereport(ERROR,
+ 				(errcode(ERRCODE_DATATYPE_MISMATCH),
+ 				 errmsg("result of expression isn't an array"),
+ 				 errdetail("result of expression is %s",
+ 									format_type_be(valtype))));
+ 
+ 	/* make a copy of the result */
+ 	arr = DatumGetArrayTypePCopy(value);
+ 
+ 	/* Calculate the number of elements we're going to loop through */
+ 	nitems = ArrayGetNItems(ARR_NDIM(arr), ARR_DIMS(arr));
+ 
+ 	/* Gather information about the array */
+ 	ptr = ARR_DATA_PTR(arr);
+ 	arraynullsptr = ARR_NULLBITMAP(arr);
+ 	get_typlenbyvalalign(ARR_ELEMTYPE(arr),
+ 						&elmlen,
+ 						&elmbyval,
+ 						&elmalign);
+ 
+ 	/* Clean up any leftover temporary memory */
+ 	exec_eval_cleanup(estate);
+ 
+ 	/* slice dimension should be less or equal to array dimension */
+ 	if (stmt->slice > ARR_NDIM(arr))
+ 			ereport(ERROR,
+ 					(errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
+ 					 errmsg("slice level %d is higher than array dimension",
+ 								    stmt->slice)));
+ 
+  	/*
+ 	 * If the target needs to be an array, check that it actually is,
+ 	 * and that it has a valid dimension for the array we're looping
+ 	 * through.
+ 	 */
+ 	if (stmt->slice > 0)
+ 	{
+ 		if (stmt->var != NULL)
+ 		{
+ 			int	typoid = stmt->var->datatype->typoid;
+ 			int	elmoid = get_element_type(typoid);
+ 
+ 			/* target variable has to be an array */
+ 			if (elmoid == InvalidOid)
+ 				ereport(ERROR,
+ 						(errcode(ERRCODE_DATATYPE_MISMATCH),
+ 						 errmsg("target variable \"%s\" for sliced array should be an array type",
+ 									stmt->var->refname),
+ 						 errhint("Value assigned will be of an array type.")));
+ 			/* Determine the number of items in the slice */
+ 			slice_len = ArrayGetNItems(stmt->slice, ARR_DIMS(arr) + ARR_NDIM(arr) - stmt->slice);
+ 			ctrl_var = estate->datums[stmt->var->dno];
+ 		}
+ 		else
+ 			ereport(ERROR,
+ 					(errcode(ERRCODE_DATATYPE_MISMATCH),
+ 					 errmsg("target variable \"%s\" for  sliced array should be an array type",
+ 								stmt->row ? stmt->row->refname : stmt->rec->refname),
+ 					 errhint("Value assigned will be of an array type.")));
+ 	}
+ 	else
+ 	{
+ 		/* target variable has to be a scalar */
+ 		if (stmt->var != NULL)
+ 		{
+ 			int	typoid = stmt->var->datatype->typoid;
+ 			int	elmoid = get_element_type(typoid);
+ 
+ 			/* target variable should be a scalar */
+ 			if (elmoid != InvalidOid)
+ 				ereport(ERROR,
+ 						(errcode(ERRCODE_DATATYPE_MISMATCH),
+ 						 errmsg("target variable \"%s\" is  an array",
+ 									stmt->var->refname),
+ 						errhint("Value assigned will be of a scalar type")));
+ 
+ 			ctrl_var = estate->datums[stmt->var->dno];
+ 		}
+ 		else if (stmt->row != NULL)
+ 		{
+ 			ctrl_var = estate->datums[stmt->row->dno];
+ 		}
+ 		else
+ 		{
+ 			ctrl_var = estate->datums[stmt->rec->dno];
+ 		}
+ 	}
+ 
+ 	i = 0;
+ 	/*
+ 	 * process all items in an array
+ 	 */
+ 	while (i < nitems)
+ 	{
+ 		int		j;
+ 		ArrayBuildState *astate = NULL;
+ 
+ 		found = true;				/* looped at least once */
+ 
+ 		/*
+ 		 * Loop through the items in this slice to build up the
+ 		 * array slice to be used through the exec statements.
+ 		 * When slice_len is one, then arr[i] value is stored in
+ 		 * variable "value", else astate is filled.
+ 		 */
+ 		for (j = 0; j < slice_len; j++)
+ 		{
+ 			if (array_get_isnull(arraynullsptr, i))
+ 			{
+ 				isnull = true;
+ 				value = (Datum) 0;
+ 			}
+ 			else
+ 			{
+ 				isnull = false;
+ 				value = fetch_att(ptr, elmbyval, elmlen);
+ 
+ 				ptr = att_addlength_pointer(ptr, elmlen, ptr);
+ 				ptr = (char *) att_align_nominal(ptr, elmalign);
+ 			}
+ 
+ 			/* build an array when is slice higher then one */
+ 			if (stmt->slice > 0)
+ 				astate = accumArrayResult(astate, value, isnull,
+ 									array_typelem,
+ 									CurrentMemoryContext);
+ 			/*
+ 			 * One item was processed. Leave cycle or add next item
+ 			 * to astate (items in slice). Increase number of processed
+ 			 * items per any item in slice.
+ 			 */
+ 			i++;
+ 		}
+ 
+  		/*
+ 		 * store an item in to the variable - we are expecting almost all
+ 		 * iterations will be over scalar values, so we don't create
+ 		 * a fake tuple over scalar and we don't use exec_move_row for
+ 		 * scalars. This is about four times faster.
+ 		 */
+ 		if (astate != NULL)
+ 		{
+ 			Datum sliced_arr;
+ 			bool isnull = false;
+ 
+ 			sliced_arr = makeMdArrayResult(astate, stmt->slice,
+ 									    ARR_DIMS(arr) + ARR_NDIM(arr) - stmt->slice,
+ 									    ARR_LBOUND(arr) ? ARR_LBOUND(arr) + ARR_NDIM(arr) - stmt->slice : NULL,
+ 										    CurrentMemoryContext, true);
+ 			exec_assign_value(estate, ctrl_var, sliced_arr, valtype, &isnull);
+ 		}
+ 		else
+ 			exec_assign_value(estate, ctrl_var,
+ 						    value, array_typelem, &isnull);
+ 
+ 		/*
+ 		 * Execute the statements
+ 		 */
+ 		rc = exec_stmts(estate, stmt->body);
+ 
+ 		if (rc == PLPGSQL_RC_RETURN)
+ 			break;				/* break out of the loop */
+ 		else if (rc == PLPGSQL_RC_EXIT)
+ 		{
+ 			if (estate->exitlabel == NULL)
+ 				/* unlabelled exit, finish the current loop */
+ 				rc = PLPGSQL_RC_OK;
+ 			else if (stmt->label != NULL &&
+ 					 strcmp(stmt->label, estate->exitlabel) == 0)
+ 			{
+ 				/* labelled exit, matches the current stmt's label */
+ 				estate->exitlabel = NULL;
+ 				rc = PLPGSQL_RC_OK;
+ 			}
+ 
+ 			/*
+ 			 * otherwise, this is a labelled exit that does not match the
+ 			 * current statement's label, if any: return RC_EXIT so that the
+ 			 * EXIT continues to propagate up the stack.
+ 			 */
+ 			break;
+ 		}
+ 		else if (rc == PLPGSQL_RC_CONTINUE)
+ 		{
+ 			if (estate->exitlabel == NULL)
+ 				/* unlabelled continue, so re-run the current loop */
+ 				rc = PLPGSQL_RC_OK;
+ 			else if (stmt->label != NULL &&
+ 					 strcmp(stmt->label, estate->exitlabel) == 0)
+ 			{
+ 				/* label matches named continue, so re-run loop */
+ 				estate->exitlabel = NULL;
+ 				rc = PLPGSQL_RC_OK;
+ 			}
+ 			else
+ 			{
+ 				/*
+ 				 * otherwise, this is a named continue that does not match the
+ 				 * current statement's label, if any: return RC_CONTINUE so
+ 				 * that the CONTINUE will propagate up the stack.
+ 				 */
+ 				break;
+ 			}
+ 		}
+ 	}
+ 
+ 	pfree(arr);
+ 
+ 	/*
+ 	 * Set the FOUND variable to indicate the result of executing the loop
+ 	 * (namely, whether we looped one or more times). This must be set here so
+ 	 * that it does not interfere with the value of the FOUND variable inside
+ 	 * the loop processing itself.
+ 	 */
+ 	exec_set_found(estate, found);
+ 
+ 	return rc;
+ }
+ 
+ 
+ /* ----------
   * exec_stmt_exit			Implements EXIT and CONTINUE
   *
   * This begins the process of exiting / restarting a loop.
***************
*** 3179,3185 ****
  	SPI_cursor_close(portal);
  
  	return rc;
! }
  
  
  /* ----------
--- 3441,3447 ----
  	SPI_cursor_close(portal);
  
  	return rc;
! } 
  
  
  /* ----------
*** ./src/pl/plpgsql/src/pl_funcs.c.orig	2011-01-16 14:18:59.265620438 +0100
--- ./src/pl/plpgsql/src/pl_funcs.c	2011-01-21 22:55:19.266125346 +0100
***************
*** 230,235 ****
--- 230,237 ----
  			return _("FOR over SELECT rows");
  		case PLPGSQL_STMT_FORC:
  			return _("FOR over cursor");
+ 		case PLPGSQL_STMT_FOREACH_A:
+ 			return _("FOREACH over array");
  		case PLPGSQL_STMT_EXIT:
  			return "EXIT";
  		case PLPGSQL_STMT_RETURN:
***************
*** 293,298 ****
--- 295,301 ----
  static void dump_close(PLpgSQL_stmt_close *stmt);
  static void dump_perform(PLpgSQL_stmt_perform *stmt);
  static void dump_expr(PLpgSQL_expr *expr);
+ static void dump_foreach_a(PLpgSQL_stmt_foreach_a *stmt);
  
  
  static void
***************
*** 376,381 ****
--- 379,387 ----
  		case PLPGSQL_STMT_PERFORM:
  			dump_perform((PLpgSQL_stmt_perform *) stmt);
  			break;
+ 		case PLPGSQL_STMT_FOREACH_A:
+ 			dump_foreach_a((PLpgSQL_stmt_foreach_a *) stmt);
+ 			break;
  		default:
  			elog(ERROR, "unrecognized cmd_type: %d", stmt->cmd_type);
  			break;
***************
*** 596,601 ****
--- 602,625 ----
  }
  
  static void
+ dump_foreach_a(PLpgSQL_stmt_foreach_a *stmt)
+ {
+ 	dump_ind();
+ 	printf("FOREACHA %s", (stmt->rec != NULL) ? stmt->rec->refname :
+ 						    (stmt->row != NULL) ? stmt->row->refname : stmt->var->refname);
+ 	if (stmt->slice != 0)
+ 		printf("SLICE %d ", stmt->slice);
+ 	printf("IN ");
+ 	dump_expr(stmt->expr);
+ 	printf("\n");
+ 
+ 	dump_stmts(stmt->body);
+ 
+ 	dump_ind();
+ 	printf("    ENDOFOREACHA");
+ }
+ 
+ static void
  dump_open(PLpgSQL_stmt_open *stmt)
  {
  	dump_ind();
*** ./src/pl/plpgsql/src/plpgsql.h.orig	2011-01-16 14:18:59.000000000 +0100
--- ./src/pl/plpgsql/src/plpgsql.h	2011-01-21 22:56:26.879893675 +0100
***************
*** 87,92 ****
--- 87,93 ----
  	PLPGSQL_STMT_CASE,
  	PLPGSQL_STMT_LOOP,
  	PLPGSQL_STMT_WHILE,
+ 	PLPGSQL_STMT_FOREACH_A,
  	PLPGSQL_STMT_FORI,
  	PLPGSQL_STMT_FORS,
  	PLPGSQL_STMT_FORC,
***************
*** 427,432 ****
--- 428,447 ----
  
  
  typedef struct
+ {								/* FOREACH item in array loop */
+ 	int			cmd_type;
+ 	int			lineno;
+ 	char	   *label;
+ 	PLpgSQL_var *var;
+ 	PLpgSQL_rec *rec;
+ 	PLpgSQL_row *row;
+ 	PLpgSQL_expr	*expr;
+ 	int	   slice;
+ 	List	   *body;			/* List of statements */
+ } PLpgSQL_stmt_foreach_a;
+ 
+ 
+ typedef struct
  {								/* FOR statement with integer loopvar	*/
  	int			cmd_type;
  	int			lineno;
*** ./src/pl/plpgsql/src/pl_scanner.c.orig	2011-01-16 14:18:59.000000000 +0100
--- ./src/pl/plpgsql/src/pl_scanner.c	2011-01-17 11:31:54.114221247 +0100
***************
*** 77,82 ****
--- 77,83 ----
  	PG_KEYWORD("exit", K_EXIT, RESERVED_KEYWORD)
  	PG_KEYWORD("fetch", K_FETCH, RESERVED_KEYWORD)
  	PG_KEYWORD("for", K_FOR, RESERVED_KEYWORD)
+ 	PG_KEYWORD("foreach", K_FOREACH, RESERVED_KEYWORD)
  	PG_KEYWORD("from", K_FROM, RESERVED_KEYWORD)
  	PG_KEYWORD("get", K_GET, RESERVED_KEYWORD)
  	PG_KEYWORD("if", K_IF, RESERVED_KEYWORD)
***************
*** 133,138 ****
--- 134,140 ----
  	PG_KEYWORD("row_count", K_ROW_COUNT, UNRESERVED_KEYWORD)
  	PG_KEYWORD("rowtype", K_ROWTYPE, UNRESERVED_KEYWORD)
  	PG_KEYWORD("scroll", K_SCROLL, UNRESERVED_KEYWORD)
+ 	PG_KEYWORD("slice", K_SLICE, UNRESERVED_KEYWORD)
  	PG_KEYWORD("sqlstate", K_SQLSTATE, UNRESERVED_KEYWORD)
  	PG_KEYWORD("type", K_TYPE, UNRESERVED_KEYWORD)
  	PG_KEYWORD("use_column", K_USE_COLUMN, UNRESERVED_KEYWORD)
*** ./src/test/regress/expected/plpgsql.out.orig	2011-01-16 14:18:59.000000000 +0100
--- ./src/test/regress/expected/plpgsql.out	2011-01-21 23:05:24.000000000 +0100
***************
*** 4240,4242 ****
--- 4240,4436 ----
  (1 row)
  
  drop function unreserved_test();
+ -- Checking a FOREACH statement
+ create function foreach_test(anyarray)
+ returns void as $$
+ declare x int;
+ begin
+   foreach x in $1
+   loop
+     raise notice '%', x;
+   end loop;
+   end;
+ $$ language plpgsql;
+ select foreach_test(ARRAY[1,2,3,4]);
+ NOTICE:  1
+ NOTICE:  2
+ NOTICE:  3
+ NOTICE:  4
+  foreach_test 
+ --------------
+  
+ (1 row)
+ 
+ select foreach_test(ARRAY[[1,2],[3,4]]);
+ NOTICE:  1
+ NOTICE:  2
+ NOTICE:  3
+ NOTICE:  4
+  foreach_test 
+ --------------
+  
+ (1 row)
+ 
+ create or replace function foreach_test(anyarray)
+ returns void as $$
+ declare x int;
+ begin
+   foreach x slice 1 in $1
+   loop
+     raise notice '%', x;
+   end loop;
+   end;
+ $$ language plpgsql;
+ -- should fail
+ select foreach_test(ARRAY[1,2,3,4]);
+ ERROR:  target variable "x" for sliced array should be an array type
+ HINT:  Value assigned will be of an array type.
+ CONTEXT:  PL/pgSQL function "foreach_test" line 4 at FOREACH over array
+ select foreach_test(ARRAY[[1,2],[3,4]]);
+ ERROR:  target variable "x" for sliced array should be an array type
+ HINT:  Value assigned will be of an array type.
+ CONTEXT:  PL/pgSQL function "foreach_test" line 4 at FOREACH over array
+ create or replace function foreach_test(anyarray)
+ returns void as $$
+ declare x int[];
+ begin
+   foreach x slice 1 in $1
+   loop
+     raise notice '%', x;
+   end loop;
+   end;
+ $$ language plpgsql;
+ select foreach_test(ARRAY[1,2,3,4]);
+ NOTICE:  {1,2,3,4}
+  foreach_test 
+ --------------
+  
+ (1 row)
+ 
+ select foreach_test(ARRAY[[1,2],[3,4]]);
+ NOTICE:  {1,2}
+ NOTICE:  {3,4}
+  foreach_test 
+ --------------
+  
+ (1 row)
+ 
+ -- higher level of slicing
+ create or replace function foreach_test(anyarray)
+ returns void as $$
+ declare x int[];
+ begin
+   foreach x slice 2 in $1
+   loop
+     raise notice '%', x;
+   end loop;
+   end;
+ $$ language plpgsql;
+ -- should fail
+ select foreach_test(ARRAY[1,2,3,4]);
+ ERROR:  slice level 2 is higher than array dimension
+ CONTEXT:  PL/pgSQL function "foreach_test" line 4 at FOREACH over array
+ -- ok
+ select foreach_test(ARRAY[[1,2],[3,4]]);
+ NOTICE:  {{1,2},{3,4}}
+  foreach_test 
+ --------------
+  
+ (1 row)
+ 
+ select foreach_test(ARRAY[[[1,2]],[[3,4]]]);
+ NOTICE:  {{1,2}}
+ NOTICE:  {{3,4}}
+  foreach_test 
+ --------------
+  
+ (1 row)
+ 
+ create type xy_tuple AS (x int, y int);
+ -- iteration over array of records
+ create or replace function foreach_test(anyarray)
+ returns void as $$
+ declare r record;
+ begin
+   foreach r in $1
+   loop
+     raise notice '%', r;
+   end loop;
+   end;
+ $$ language plpgsql;
+ select foreach_test(ARRAY[(10,20),(40,69),(35,78)]::xy_tuple[]);
+ NOTICE:  (10,20)
+ NOTICE:  (40,69)
+ NOTICE:  (35,78)
+  foreach_test 
+ --------------
+  
+ (1 row)
+ 
+ select foreach_test(ARRAY[[(10,20),(40,69)],[(35,78),(88,76)]]::xy_tuple[]);
+ NOTICE:  (10,20)
+ NOTICE:  (40,69)
+ NOTICE:  (35,78)
+ NOTICE:  (88,76)
+  foreach_test 
+ --------------
+  
+ (1 row)
+ 
+ create or replace function foreach_test(anyarray)
+ returns void as $$
+ declare x int; y int;
+ begin
+   foreach x, y in $1
+   loop
+     raise notice 'x = %, y = %', x, y;
+   end loop;
+   end;
+ $$ language plpgsql;
+ select foreach_test(ARRAY[(10,20),(40,69),(35,78)]::xy_tuple[]);
+ NOTICE:  x = 10, y = 20
+ NOTICE:  x = 40, y = 69
+ NOTICE:  x = 35, y = 78
+  foreach_test 
+ --------------
+  
+ (1 row)
+ 
+ select foreach_test(ARRAY[[(10,20),(40,69)],[(35,78),(88,76)]]::xy_tuple[]);
+ NOTICE:  x = 10, y = 20
+ NOTICE:  x = 40, y = 69
+ NOTICE:  x = 35, y = 78
+ NOTICE:  x = 88, y = 76
+  foreach_test 
+ --------------
+  
+ (1 row)
+ 
+ -- slicing over array of composite types
+ create or replace function foreach_test(anyarray)
+ returns void as $$
+ declare x xy_tuple[];
+ begin
+   foreach x slice 1 in $1
+   loop
+     raise notice '%', x;
+   end loop;
+   end;
+ $$ language plpgsql;
+ select foreach_test(ARRAY[(10,20),(40,69),(35,78)]::xy_tuple[]);
+ NOTICE:  {"(10,20)","(40,69)","(35,78)"}
+  foreach_test 
+ --------------
+  
+ (1 row)
+ 
+ select foreach_test(ARRAY[[(10,20),(40,69)],[(35,78),(88,76)]]::xy_tuple[]);
+ NOTICE:  {"(10,20)","(40,69)"}
+ NOTICE:  {"(35,78)","(88,76)"}
+  foreach_test 
+ --------------
+  
+ (1 row)
+ 
+ drop function foreach_test(anyarray);
+ drop type xy_tuple;
*** ./src/test/regress/sql/plpgsql.sql.orig	2011-01-16 14:18:59.000000000 +0100
--- ./src/test/regress/sql/plpgsql.sql	2011-01-17 11:31:54.123222448 +0100
***************
*** 3375,3377 ****
--- 3375,3488 ----
  select unreserved_test();
  
  drop function unreserved_test();
+ 
+ -- Checking a FOREACH statement
+ create function foreach_test(anyarray)
+ returns void as $$
+ declare x int;
+ begin
+   foreach x in $1
+   loop
+     raise notice '%', x;
+   end loop;
+   end;
+ $$ language plpgsql;
+ 
+ select foreach_test(ARRAY[1,2,3,4]);
+ select foreach_test(ARRAY[[1,2],[3,4]]);
+ 
+ create or replace function foreach_test(anyarray)
+ returns void as $$
+ declare x int;
+ begin
+   foreach x slice 1 in $1
+   loop
+     raise notice '%', x;
+   end loop;
+   end;
+ $$ language plpgsql;
+ 
+ -- should fail
+ select foreach_test(ARRAY[1,2,3,4]);
+ select foreach_test(ARRAY[[1,2],[3,4]]);
+ 
+ create or replace function foreach_test(anyarray)
+ returns void as $$
+ declare x int[];
+ begin
+   foreach x slice 1 in $1
+   loop
+     raise notice '%', x;
+   end loop;
+   end;
+ $$ language plpgsql;
+ 
+ select foreach_test(ARRAY[1,2,3,4]);
+ select foreach_test(ARRAY[[1,2],[3,4]]);
+ 
+ -- higher level of slicing
+ create or replace function foreach_test(anyarray)
+ returns void as $$
+ declare x int[];
+ begin
+   foreach x slice 2 in $1
+   loop
+     raise notice '%', x;
+   end loop;
+   end;
+ $$ language plpgsql;
+ 
+ -- should fail
+ select foreach_test(ARRAY[1,2,3,4]);
+ -- ok
+ select foreach_test(ARRAY[[1,2],[3,4]]);
+ select foreach_test(ARRAY[[[1,2]],[[3,4]]]);
+ 
+ create type xy_tuple AS (x int, y int);
+ 
+ -- iteration over array of records
+ create or replace function foreach_test(anyarray)
+ returns void as $$
+ declare r record;
+ begin
+   foreach r in $1
+   loop
+     raise notice '%', r;
+   end loop;
+   end;
+ $$ language plpgsql;
+ 
+ select foreach_test(ARRAY[(10,20),(40,69),(35,78)]::xy_tuple[]);
+ select foreach_test(ARRAY[[(10,20),(40,69)],[(35,78),(88,76)]]::xy_tuple[]);
+ 
+ create or replace function foreach_test(anyarray)
+ returns void as $$
+ declare x int; y int;
+ begin
+   foreach x, y in $1
+   loop
+     raise notice 'x = %, y = %', x, y;
+   end loop;
+   end;
+ $$ language plpgsql;
+ 
+ select foreach_test(ARRAY[(10,20),(40,69),(35,78)]::xy_tuple[]);
+ select foreach_test(ARRAY[[(10,20),(40,69)],[(35,78),(88,76)]]::xy_tuple[]);
+ 
+ -- slicing over array of composite types
+ create or replace function foreach_test(anyarray)
+ returns void as $$
+ declare x xy_tuple[];
+ begin
+   foreach x slice 1 in $1
+   loop
+     raise notice '%', x;
+   end loop;
+   end;
+ $$ language plpgsql;
+ 
+ select foreach_test(ARRAY[(10,20),(40,69),(35,78)]::xy_tuple[]);
+ select foreach_test(ARRAY[[(10,20),(40,69)],[(35,78),(88,76)]]::xy_tuple[]);
+ 
+ drop function foreach_test(anyarray);
+ drop type xy_tuple;
#7Stephen Frost
sfrost@snowman.net
In reply to: Pavel Stehule (#6)
Re: REVIEW: WIP: plpgsql - foreach in

Pavel,

* Pavel Stehule (pavel.stehule@gmail.com) wrote:

I merge your changes and little enhanced comments.

Thanks. Reviewing this further-

Why are you using 'FOREACH' here instead of just making it another
variation of 'FOR'? What is 'FOUND' set to following this? I realize
that might make the code easier, but it really doesn't seem to make much
sense to me from a usability point of view.

There also appears to be some extra whitespace changes that aren't
necessary and a number of places where you don't follow the indentation
conventions (eg: variable definitions in exec_stmt_foreach_a()).

I'm still not really thrilled with how the checking for scalar vs.
array, etc, is handled. Additionally, what is this? :

else if (stmt->row != NULL)
{
ctrl_var = estate->datums[stmt->row->dno];
}
else
{
ctrl_var = estate->datums[stmt->rec->dno];
}

Other comments- I don't like using 'i' and 'j', you really should use
better variable names, especially in large loops which contain other
loops. I'd also suggest changing the outer loop to be equivilant to the
number of iterations that will be done instead of the number of items
and then to *not* update 'i' inside the inner-loop. That structure is
really just confusing, imv (I certainly didn't entirely follow what was
happening there the first time I read it). Isn't there a function you
could use to pull out the array slice you need on each iteration through
the array?

Thanks,

Stephen

#8Robert Haas
robertmhaas@gmail.com
In reply to: Stephen Frost (#7)
Re: REVIEW: WIP: plpgsql - foreach in

On Sun, Jan 23, 2011 at 9:49 PM, Stephen Frost <sfrost@snowman.net> wrote:

Pavel,

* Pavel Stehule (pavel.stehule@gmail.com) wrote:

I merge your changes and little enhanced comments.

Thanks.  Reviewing this further-

Why are you using 'FOREACH' here instead of just making it another
variation of 'FOR'?

Uh oh. You just reopened the can of worms from hell.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

#9Stephen Frost
sfrost@snowman.net
In reply to: Robert Haas (#8)
Re: REVIEW: WIP: plpgsql - foreach in

* Robert Haas (robertmhaas@gmail.com) wrote:

On Sun, Jan 23, 2011 at 9:49 PM, Stephen Frost <sfrost@snowman.net> wrote:

Why are you using 'FOREACH' here instead of just making it another
variation of 'FOR'?

Uh oh. You just reopened the can of worms from hell.

hahahaha. Apparently I missed that discussion; also wasn't linked off
the patch. :/ Guess I'll go poke through the archives... Struck me as
obviously wrong to invent something completely new for this, but..

Thanks,

Stephen

#10Stephen Frost
sfrost@snowman.net
In reply to: Robert Haas (#8)
Re: REVIEW: WIP: plpgsql - foreach in

* Robert Haas (robertmhaas@gmail.com) wrote:

Uh oh. You just reopened the can of worms from hell.

Alright.. I'm missing what happened to this suggestion of using:

FOR var in ARRAY array_expression ...

I like that a lot more than inventing a new top-level keyword, for the
same reasons that Tom mentioned: using a different initial keyword
makes it awkward to make generic statements about "all types of FOR
loop" (as I noticed while looking through the documentation changes that
should be made for this); and also some of the other comments about how
FOREACH doesn't give you any clue that this is some
array-specific-FOR-loop-thingy.

Thanks,

Stephen

#11Pavel Stehule
pavel.stehule@gmail.com
In reply to: Stephen Frost (#7)
Re: REVIEW: WIP: plpgsql - foreach in

2011/1/24 Stephen Frost <sfrost@snowman.net>:

Pavel,

* Pavel Stehule (pavel.stehule@gmail.com) wrote:

I merge your changes and little enhanced comments.

Thanks.  Reviewing this further-

Why are you using 'FOREACH' here instead of just making it another
variation of 'FOR'?  What is 'FOUND' set to following this?  I realize
that might make the code easier, but it really doesn't seem to make much
sense to me from a usability point of view.

FOR keyword - please, look on thread about my proposal FOR-IN-ARRAY

I work with FOUND variable, because I like a consistent behave with
FOR statement. When FOUND is true after cycle, you are sure, so there
was a minimally one iteration.

There also appears to be some extra whitespace changes that aren't
necessary and a number of places where you don't follow the indentation
conventions (eg: variable definitions in exec_stmt_foreach_a()).

I am really not sure about correct indentation of variables :(, if you
know a correct number of spaces, please, fix it.

I'm still not really thrilled with how the checking for scalar vs.
array, etc, is handled.  Additionally, what is this? :

       else if (stmt->row != NULL)
       {
           ctrl_var = estate->datums[stmt->row->dno];
       }
       else
       {
           ctrl_var = estate->datums[stmt->rec->dno];
       }

PLpgSQL distinct between vars, row and record values. These structures
can be different and information about variable's offset in frame can
be on different position in structure. This IF means:

1) get offset of target variable
2) get addr, where is target variable saved in current frame

one note: a scalar or array can be saved on var type, only scalar can
be used on row or record type. This is often used pattern - you can
see it more time in executor.

Other comments- I don't like using 'i' and 'j', you really should use
better variable names, especially in large loops which contain other
loops.  I'd also suggest changing the outer loop to be equivilant to the
number of iterations that will be done instead of the number of items
and then to *not* update 'i' inside the inner-loop.  That structure is
really just confusing, imv (I certainly didn't entirely follow what was
happening there the first time I read it).  Isn't there a function you
could use to pull out the array slice you need on each iteration through
the array?

I don't know a better short index identifiers than I used. But I am
not against to change.

I'll try to redesign main cycle.

Regards

Pavel Stehule

Show quoted text

       Thanks,

               Stephen

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)

iEYEARECAAYFAk086MEACgkQrzgMPqB3kigCzQCffx0iVSMjU2UbOgAOaY/MvtOp
iKsAnA5tdhKxTssdXJ+Rda4qkhNVm26g
=Yn5O
-----END PGP SIGNATURE-----

#12Pavel Stehule
pavel.stehule@gmail.com
In reply to: Pavel Stehule (#11)
1 attachment(s)
Re: REVIEW: WIP: plpgsql - foreach in

Hello

Other comments- I don't like using 'i' and 'j', you really should use
better variable names, especially in large loops which contain other
loops.  I'd also suggest changing the outer loop to be equivilant to the
number of iterations that will be done instead of the number of items
and then to *not* update 'i' inside the inner-loop.  That structure is
really just confusing, imv (I certainly didn't entirely follow what was
happening there the first time I read it).  Isn't there a function you
could use to pull out the array slice you need on each iteration through
the array?

I looked on code again. There are a few results:

I'll change identifiers 'i' and 'j' with any, that you send. It's
usual identifiers for nested loops and in this case they has really
well known semantic - it's subscript of array.

But others changes are more difficult

we have to iterate over array's items because it allow seq. access to
array's data. I need a global index for function "array_get_isnull". I
can't to use a buildin functions like array_slize_size or
array_get_slice, because there is high overhead of array_seek
function. I redesigned the initial part of main cycle, but code is
little bit longer :(, but I hope, it is more readable.

Regards

Pavel

Show quoted text

I don't know a better short index identifiers than I used. But I am
not against to change.

I'll try to redesign main cycle.

Regards

Pavel Stehule

       Thanks,

               Stephen

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)

iEYEARECAAYFAk086MEACgkQrzgMPqB3kigCzQCffx0iVSMjU2UbOgAOaY/MvtOp
iKsAnA5tdhKxTssdXJ+Rda4qkhNVm26g
=Yn5O
-----END PGP SIGNATURE-----

Attachments:

foreach.difftext/x-patch; charset=US-ASCII; name=foreach.diffDownload
*** ./gram.y.orig	2011-01-16 14:18:59.000000000 +0100
--- ./gram.y	2011-01-21 21:35:21.000000000 +0100
***************
*** 21,26 ****
--- 21,27 ----
  #include "parser/parse_type.h"
  #include "parser/scanner.h"
  #include "parser/scansup.h"
+ #include "utils/array.h"
  
  
  /* Location tracking support --- simpler than bison's default */
***************
*** 134,139 ****
--- 135,141 ----
  			PLpgSQL_datum   *scalar;
  			PLpgSQL_rec     *rec;
  			PLpgSQL_row     *row;
+ 			int	slice;
  		}						forvariable;
  		struct
  		{
***************
*** 178,184 ****
  %type <ival>	assign_var
  %type <var>		cursor_variable
  %type <datum>	decl_cursor_arg
! %type <forvariable>	for_variable
  %type <stmt>	for_control
  
  %type <str>		any_identifier opt_block_label opt_label
--- 180,186 ----
  %type <ival>	assign_var
  %type <var>		cursor_variable
  %type <datum>	decl_cursor_arg
! %type <forvariable>	for_variable foreach_control
  %type <stmt>	for_control
  
  %type <str>		any_identifier opt_block_label opt_label
***************
*** 190,196 ****
  %type <stmt>	stmt_return stmt_raise stmt_execsql
  %type <stmt>	stmt_dynexecute stmt_for stmt_perform stmt_getdiag
  %type <stmt>	stmt_open stmt_fetch stmt_move stmt_close stmt_null
! %type <stmt>	stmt_case
  
  %type <list>	proc_exceptions
  %type <exception_block> exception_sect
--- 192,198 ----
  %type <stmt>	stmt_return stmt_raise stmt_execsql
  %type <stmt>	stmt_dynexecute stmt_for stmt_perform stmt_getdiag
  %type <stmt>	stmt_open stmt_fetch stmt_move stmt_close stmt_null
! %type <stmt>	stmt_case stmt_foreach_a
  
  %type <list>	proc_exceptions
  %type <exception_block> exception_sect
***************
*** 264,269 ****
--- 266,272 ----
  %token <keyword>	K_FETCH
  %token <keyword>	K_FIRST
  %token <keyword>	K_FOR
+ %token <keyword>	K_FOREACH
  %token <keyword>	K_FORWARD
  %token <keyword>	K_FROM
  %token <keyword>	K_GET
***************
*** 298,303 ****
--- 301,307 ----
  %token <keyword>	K_ROWTYPE
  %token <keyword>	K_ROW_COUNT
  %token <keyword>	K_SCROLL
+ %token <keyword>	K_SLICE
  %token <keyword>	K_SQLSTATE
  %token <keyword>	K_STRICT
  %token <keyword>	K_THEN
***************
*** 739,744 ****
--- 743,750 ----
  						{ $$ = $1; }
  				| stmt_for
  						{ $$ = $1; }
+ 				| stmt_foreach_a
+ 						{ $$ = $1; }
  				| stmt_exit
  						{ $$ = $1; }
  				| stmt_return
***************
*** 1386,1391 ****
--- 1392,1455 ----
  					}
  				;
  
+ stmt_foreach_a		: opt_block_label K_FOREACH foreach_control K_IN expr_until_loop loop_body
+ 					{
+ 						PLpgSQL_stmt_foreach_a *new = palloc0(sizeof(PLpgSQL_stmt_foreach_a));
+ 						new->cmd_type = PLPGSQL_STMT_FOREACH_A;
+ 						new->lineno = plpgsql_location_to_lineno(@2);
+ 						new->label = $1;
+ 						new->expr = $5;
+ 						new->slice = $3.slice;
+ 						new->body = $6.stmts;
+ 
+ 						if ($3.rec)
+ 						{
+ 							new->rec = $3.rec;
+ 							check_assignable((PLpgSQL_datum *) new->rec, @3);
+ 						}
+ 						else if ($3.row)
+ 						{
+ 							new->row = $3.row;
+ 							check_assignable((PLpgSQL_datum *) new->row, @3);
+ 						}
+ 						else if ($3.scalar)
+ 						{
+ 							Assert($3.scalar->dtype == PLPGSQL_DTYPE_VAR);
+ 							new->var = (PLpgSQL_var *) $3.scalar;
+ 							check_assignable($3.scalar, @3);
+ 
+ 						}
+ 						else
+ 						{
+ 							ereport(ERROR,
+ 									(errcode(ERRCODE_SYNTAX_ERROR),
+ 									 errmsg("loop variable of loop over array must be a record, row variable, scalar variable or list of scalar variables"),
+ 											 parser_errposition(@3)));
+ 						}
+ 
+ 						check_labels($1, $6.end_label, $6.end_label_location);
+ 						$$ = (PLpgSQL_stmt *) new;
+ 					}
+ 				;
+ 
+ foreach_control		: for_variable
+ 					{
+ 						$$ = $1;
+ 						$$.slice = 0;
+ 					}
+ 				| for_variable K_SLICE ICONST
+ 					{
+ 						$$ = $1;
+ 						$$.slice = $3;
+ 						if ($3 < 0 || $3 >= MAXDIM)
+ 							ereport(ERROR,
+ 									(errcode(ERRCODE_PROGRAM_LIMIT_EXCEEDED),
+ 									 errmsg("number of slicing array dimensions (%d) exceeds the maximum allowed (%d)",
+ 												$3, MAXDIM),
+ 											 parser_errposition(@3)));
+ 					}
+ 				;
+ 
  stmt_exit		: exit_type opt_label opt_exitcond
  					{
  						PLpgSQL_stmt_exit *new;
***************
*** 2063,2068 ****
--- 2127,2133 ----
  				| K_ROW_COUNT
  				| K_ROWTYPE
  				| K_SCROLL
+ 				| K_SLICE
  				| K_SQLSTATE
  				| K_TYPE
  				| K_USE_COLUMN
*** ./pl_exec.c.orig	2011-01-16 14:18:59.000000000 +0100
--- ./pl_exec.c	2011-01-24 12:05:58.599741965 +0100
***************
*** 107,112 ****
--- 107,114 ----
  			   PLpgSQL_stmt_fors *stmt);
  static int exec_stmt_forc(PLpgSQL_execstate *estate,
  			   PLpgSQL_stmt_forc *stmt);
+ static int exec_stmt_foreach_a(PLpgSQL_execstate *estate,
+ 				    PLpgSQL_stmt_foreach_a *stmt);
  static int exec_stmt_open(PLpgSQL_execstate *estate,
  			   PLpgSQL_stmt_open *stmt);
  static int exec_stmt_fetch(PLpgSQL_execstate *estate,
***************
*** 1312,1317 ****
--- 1314,1323 ----
  			rc = exec_stmt_forc(estate, (PLpgSQL_stmt_forc *) stmt);
  			break;
  
+ 		case PLPGSQL_STMT_FOREACH_A:
+ 			rc = exec_stmt_foreach_a(estate, (PLpgSQL_stmt_foreach_a *) stmt);
+ 			break;
+ 
  		case PLPGSQL_STMT_EXIT:
  			rc = exec_stmt_exit(estate, (PLpgSQL_stmt_exit *) stmt);
  			break;
***************
*** 2028,2033 ****
--- 2034,2295 ----
  
  
  /* ----------
+  * exec_stmt_foreach_a			Implements loop over array
+  *
+  * ----------
+  */
+ static int
+ exec_stmt_foreach_a(PLpgSQL_execstate *estate, PLpgSQL_stmt_foreach_a *stmt)
+ {
+ 	Datum		value;
+ 	bool		isnull;
+ 	Oid		valtype;
+ 	int		nitems;
+ 	int		slice_len = 0;			/* be compiler quite */
+ 	Oid 		array_typelem;
+ 	int		i;
+ 	ArrayType	*arr;
+ 	char		*ptr;
+ 	bits8		*arraynullsptr;
+ 	int16		elmlen;
+ 	bool		elmbyval;
+ 	char		elmalign;
+ 	PLpgSQL_datum	*ctrl_var = NULL;		/* be compiler quite */
+ 	bool		found = false;
+ 	int		rc = PLPGSQL_RC_OK;
+ 	int		niterations;
+ 	int		offset;
+ 
+ 	/* get the value of the array expression using array_expr */
+ 	value = exec_eval_expr(estate, stmt->expr, &isnull, &valtype);
+ 	if (isnull)
+ 		ereport(ERROR,
+ 				(errcode(ERRCODE_NULL_VALUE_NOT_ALLOWED),
+ 				 errmsg("NULL value isn't allowed as parameter of FOREACH-IN")));
+ 
+ 	/* check the type of the expression - must be an array */
+ 	array_typelem = get_element_type(valtype);
+ 
+ 	if (!OidIsValid(array_typelem))
+ 		ereport(ERROR,
+ 				(errcode(ERRCODE_DATATYPE_MISMATCH),
+ 				 errmsg("result of expression isn't an array"),
+ 				 errdetail("result of expression is %s",
+ 									format_type_be(valtype))));
+ 
+ 	/* make a copy of the result */
+ 	arr = DatumGetArrayTypePCopy(value);
+ 
+ 	/* Calculate the number of elements we're going to loop through */
+ 	nitems = ArrayGetNItems(ARR_NDIM(arr), ARR_DIMS(arr));
+ 
+ 	/* Gather information about the array */
+ 	ptr = ARR_DATA_PTR(arr);
+ 	arraynullsptr = ARR_NULLBITMAP(arr);
+ 	get_typlenbyvalalign(ARR_ELEMTYPE(arr),
+ 						&elmlen,
+ 						&elmbyval,
+ 						&elmalign);
+ 
+ 	/* Clean up any leftover temporary memory */
+ 	exec_eval_cleanup(estate);
+ 
+ 	/* slice dimension should be less or equal to array dimension */
+ 	if (stmt->slice > ARR_NDIM(arr))
+ 			ereport(ERROR,
+ 					(errcode(ERRCODE_NUMERIC_VALUE_OUT_OF_RANGE),
+ 					 errmsg("slice level %d is higher than array dimension",
+ 								    stmt->slice)));
+ 
+  	/*
+ 	 * If the target needs to be an array, check that it actually is,
+ 	 * and that it has a valid dimension for the array we're looping
+ 	 * through.
+ 	 */
+ 	if (stmt->slice > 0)
+ 	{
+ 		if (stmt->var != NULL)
+ 		{
+ 			int	typoid = stmt->var->datatype->typoid;
+ 			int	elmoid = get_element_type(typoid);
+ 
+ 			/* target variable has to be an array */
+ 			if (elmoid == InvalidOid)
+ 				ereport(ERROR,
+ 						(errcode(ERRCODE_DATATYPE_MISMATCH),
+ 						 errmsg("target variable \"%s\" for sliced array should be an array type",
+ 									stmt->var->refname),
+ 						 errhint("Value assigned will be of an array type.")));
+ 			/* Determine the number of items in the slice */
+ 			slice_len = ArrayGetNItems(stmt->slice, ARR_DIMS(arr) + ARR_NDIM(arr) - stmt->slice);
+ 			ctrl_var = estate->datums[stmt->var->dno];
+ 		}
+ 		else
+ 			ereport(ERROR,
+ 					(errcode(ERRCODE_DATATYPE_MISMATCH),
+ 					 errmsg("target variable \"%s\" for  sliced array should be an array type",
+ 								stmt->row ? stmt->row->refname : stmt->rec->refname),
+ 					 errhint("Value assigned will be of an array type.")));
+ 
+ 		niterations = nitems / slice_len;
+ 	}
+ 	else
+ 	{
+ 		/* target variable has to be a scalar */
+ 		if (stmt->var != NULL)
+ 		{
+ 			int	typoid = stmt->var->datatype->typoid;
+ 			int	elmoid = get_element_type(typoid);
+ 
+ 			/* target variable should be a scalar */
+ 			if (elmoid != InvalidOid)
+ 				ereport(ERROR,
+ 						(errcode(ERRCODE_DATATYPE_MISMATCH),
+ 						 errmsg("target variable \"%s\" is  an array", 
+ 									stmt->var->refname),
+ 						errhint("Value assigned will be of a scalar type")));
+ 
+ 			ctrl_var = estate->datums[stmt->var->dno];
+ 		}
+ 		else if (stmt->row != NULL)
+ 		{
+ 			ctrl_var = estate->datums[stmt->row->dno];
+ 		}
+ 		else
+ 		{
+ 			ctrl_var = estate->datums[stmt->rec->dno];
+ 		}
+ 
+ 		niterations = nitems;
+ 	}
+ 
+ 	/* 
+ 	 * main loop - iteration over array's items or array's slices 
+ 	 */
+ 	offset = 0;
+ 	for (i = 0; i < niterations; i++)
+ 	{
+ 		/* 
+ 		 * Set a target variable. Construct an subarray (slice) when it is requested.
+ 		 * When slice isn't requested, then target type is same as array element type,
+ 		 * otherwise target type is same as original array.
+ 		 */
+ 		if (stmt->slice > 0)
+ 		{
+ 			int	j;
+ 			ArrayBuildState *astate = NULL;
+ 			Datum slice;
+ 
+ 			for (j = 0; j < slice_len; j++)
+ 			{
+ 				if (array_get_isnull(arraynullsptr, offset++))
+ 				{
+ 					isnull = true;
+ 					value = (Datum) 0;
+ 				}
+ 				else
+ 				{
+ 					isnull = false;
+ 					value = fetch_att(ptr, elmbyval, elmlen);
+ 					ptr = att_addlength_pointer(ptr, elmlen, ptr);
+ 					ptr = (char *) att_align_nominal(ptr, elmalign);
+ 				}
+ 				astate = accumArrayResult(astate, value, isnull, 
+ 									array_typelem, 
+ 									CurrentMemoryContext);
+ 			}
+ 			isnull = false;
+ 			slice = makeMdArrayResult(astate, stmt->slice,
+ 								    ARR_DIMS(arr) + ARR_NDIM(arr) - stmt->slice,
+ 								    ARR_LBOUND(arr) ? ARR_LBOUND(arr) + ARR_NDIM(arr) - stmt->slice : NULL,
+ 									    CurrentMemoryContext, true);
+ 			exec_assign_value(estate, ctrl_var, slice, valtype, &isnull);
+ 		}
+ 		else
+ 		{
+ 			/* get a value */
+ 			if (array_get_isnull(arraynullsptr, offset++))
+ 			{
+ 				isnull = true;
+ 				value = (Datum) 0;
+ 			}
+ 			else
+ 			{
+ 				isnull = false;
+ 				value = fetch_att(ptr, elmbyval, elmlen);
+ 
+ 				ptr = att_addlength_pointer(ptr, elmlen, ptr);
+ 				ptr = (char *) att_align_nominal(ptr, elmalign);
+ 			}
+ 			exec_assign_value(estate, ctrl_var, value, array_typelem, &isnull);
+ 		}
+ 
+ 		/*
+ 		 * Execute the statements
+ 		 */
+ 		rc = exec_stmts(estate, stmt->body);
+ 
+ 		if (rc == PLPGSQL_RC_RETURN)
+ 			break;				/* break out of the loop */
+ 		else if (rc == PLPGSQL_RC_EXIT)
+ 		{
+ 			if (estate->exitlabel == NULL)
+ 				/* unlabelled exit, finish the current loop */
+ 				rc = PLPGSQL_RC_OK;
+ 			else if (stmt->label != NULL &&
+ 					 strcmp(stmt->label, estate->exitlabel) == 0)
+ 			{
+ 				/* labelled exit, matches the current stmt's label */
+ 				estate->exitlabel = NULL;
+ 				rc = PLPGSQL_RC_OK;
+ 			}
+ 
+ 			/*
+ 			 * otherwise, this is a labelled exit that does not match the
+ 			 * current statement's label, if any: return RC_EXIT so that the
+ 			 * EXIT continues to propagate up the stack.
+ 			 */
+ 			break;
+ 		}
+ 		else if (rc == PLPGSQL_RC_CONTINUE)
+ 		{
+ 			if (estate->exitlabel == NULL)
+ 				/* unlabelled continue, so re-run the current loop */
+ 				rc = PLPGSQL_RC_OK;
+ 			else if (stmt->label != NULL &&
+ 					 strcmp(stmt->label, estate->exitlabel) == 0)
+ 			{
+ 				/* label matches named continue, so re-run loop */
+ 				estate->exitlabel = NULL;
+ 				rc = PLPGSQL_RC_OK;
+ 			}
+ 			else
+ 			{
+ 				/*
+ 				 * otherwise, this is a named continue that does not match the
+ 				 * current statement's label, if any: return RC_CONTINUE so
+ 				 * that the CONTINUE will propagate up the stack.
+ 				 */
+ 				break;
+ 			}
+ 		}
+ 	}
+ 
+ 	pfree(arr);
+ 
+ 	/*
+ 	 * Set the FOUND variable to indicate the result of executing the loop
+ 	 * (namely, whether we looped one or more times). This must be set here so
+ 	 * that it does not interfere with the value of the FOUND variable inside
+ 	 * the loop processing itself.
+ 	 */
+ 	exec_set_found(estate, found);
+ 
+ 	return rc;
+ }
+ 
+ 
+ /* ----------
   * exec_stmt_exit			Implements EXIT and CONTINUE
   *
   * This begins the process of exiting / restarting a loop.
***************
*** 3179,3185 ****
  	SPI_cursor_close(portal);
  
  	return rc;
! }
  
  
  /* ----------
--- 3441,3447 ----
  	SPI_cursor_close(portal);
  
  	return rc;
! } 
  
  
  /* ----------
*** ./pl_funcs.c.orig	2011-01-16 14:18:59.000000000 +0100
--- ./pl_funcs.c	2011-01-21 22:55:19.000000000 +0100
***************
*** 230,235 ****
--- 230,237 ----
  			return _("FOR over SELECT rows");
  		case PLPGSQL_STMT_FORC:
  			return _("FOR over cursor");
+ 		case PLPGSQL_STMT_FOREACH_A:
+ 			return _("FOREACH over array");
  		case PLPGSQL_STMT_EXIT:
  			return "EXIT";
  		case PLPGSQL_STMT_RETURN:
***************
*** 293,298 ****
--- 295,301 ----
  static void dump_close(PLpgSQL_stmt_close *stmt);
  static void dump_perform(PLpgSQL_stmt_perform *stmt);
  static void dump_expr(PLpgSQL_expr *expr);
+ static void dump_foreach_a(PLpgSQL_stmt_foreach_a *stmt);
  
  
  static void
***************
*** 376,381 ****
--- 379,387 ----
  		case PLPGSQL_STMT_PERFORM:
  			dump_perform((PLpgSQL_stmt_perform *) stmt);
  			break;
+ 		case PLPGSQL_STMT_FOREACH_A:
+ 			dump_foreach_a((PLpgSQL_stmt_foreach_a *) stmt);
+ 			break;
  		default:
  			elog(ERROR, "unrecognized cmd_type: %d", stmt->cmd_type);
  			break;
***************
*** 596,601 ****
--- 602,625 ----
  }
  
  static void
+ dump_foreach_a(PLpgSQL_stmt_foreach_a *stmt)
+ {
+ 	dump_ind();
+ 	printf("FOREACHA %s", (stmt->rec != NULL) ? stmt->rec->refname :
+ 						    (stmt->row != NULL) ? stmt->row->refname : stmt->var->refname);
+ 	if (stmt->slice != 0)
+ 		printf("SLICE %d ", stmt->slice);
+ 	printf("IN ");
+ 	dump_expr(stmt->expr);
+ 	printf("\n");
+ 
+ 	dump_stmts(stmt->body);
+ 
+ 	dump_ind();
+ 	printf("    ENDOFOREACHA");
+ }
+ 
+ static void
  dump_open(PLpgSQL_stmt_open *stmt)
  {
  	dump_ind();
*** ./plpgsql.h.orig	2011-01-16 14:18:59.000000000 +0100
--- ./plpgsql.h	2011-01-21 22:56:26.000000000 +0100
***************
*** 87,92 ****
--- 87,93 ----
  	PLPGSQL_STMT_CASE,
  	PLPGSQL_STMT_LOOP,
  	PLPGSQL_STMT_WHILE,
+ 	PLPGSQL_STMT_FOREACH_A,
  	PLPGSQL_STMT_FORI,
  	PLPGSQL_STMT_FORS,
  	PLPGSQL_STMT_FORC,
***************
*** 427,432 ****
--- 428,447 ----
  
  
  typedef struct
+ {								/* FOREACH item in array loop */
+ 	int			cmd_type;
+ 	int			lineno;
+ 	char	   *label;
+ 	PLpgSQL_var *var;
+ 	PLpgSQL_rec *rec;
+ 	PLpgSQL_row *row;
+ 	PLpgSQL_expr	*expr;
+ 	int	   slice;
+ 	List	   *body;			/* List of statements */
+ } PLpgSQL_stmt_foreach_a;
+ 
+ 
+ typedef struct
  {								/* FOR statement with integer loopvar	*/
  	int			cmd_type;
  	int			lineno;
*** ./pl_scanner.c.orig	2011-01-16 14:18:59.000000000 +0100
--- ./pl_scanner.c	2011-01-17 11:31:54.000000000 +0100
***************
*** 77,82 ****
--- 77,83 ----
  	PG_KEYWORD("exit", K_EXIT, RESERVED_KEYWORD)
  	PG_KEYWORD("fetch", K_FETCH, RESERVED_KEYWORD)
  	PG_KEYWORD("for", K_FOR, RESERVED_KEYWORD)
+ 	PG_KEYWORD("foreach", K_FOREACH, RESERVED_KEYWORD)
  	PG_KEYWORD("from", K_FROM, RESERVED_KEYWORD)
  	PG_KEYWORD("get", K_GET, RESERVED_KEYWORD)
  	PG_KEYWORD("if", K_IF, RESERVED_KEYWORD)
***************
*** 133,138 ****
--- 134,140 ----
  	PG_KEYWORD("row_count", K_ROW_COUNT, UNRESERVED_KEYWORD)
  	PG_KEYWORD("rowtype", K_ROWTYPE, UNRESERVED_KEYWORD)
  	PG_KEYWORD("scroll", K_SCROLL, UNRESERVED_KEYWORD)
+ 	PG_KEYWORD("slice", K_SLICE, UNRESERVED_KEYWORD)
  	PG_KEYWORD("sqlstate", K_SQLSTATE, UNRESERVED_KEYWORD)
  	PG_KEYWORD("type", K_TYPE, UNRESERVED_KEYWORD)
  	PG_KEYWORD("use_column", K_USE_COLUMN, UNRESERVED_KEYWORD)
#13Cédric Villemain
cedric.villemain.debian@gmail.com
In reply to: Pavel Stehule (#12)
Re: REVIEW: WIP: plpgsql - foreach in

2011/1/24 Pavel Stehule <pavel.stehule@gmail.com>:

Hello

Other comments- I don't like using 'i' and 'j', you really should use
better variable names, especially in large loops which contain other
loops.  I'd also suggest changing the outer loop to be equivilant to the
number of iterations that will be done instead of the number of items
and then to *not* update 'i' inside the inner-loop.  That structure is
really just confusing, imv (I certainly didn't entirely follow what was
happening there the first time I read it).  Isn't there a function you
could use to pull out the array slice you need on each iteration through
the array?

I looked on code again. There are a few results:

I'll change identifiers 'i' and 'j' with any, that you send. It's
usual identifiers for nested loops and in this case they has really
well known semantic - it's subscript of array.

But others changes are more difficult

we have to iterate over array's items because it allow seq. access to
array's data. I need a global index for function "array_get_isnull". I
can't to use a buildin functions like array_slize_size or
array_get_slice, because there is high overhead of array_seek
function. I redesigned the initial part of main cycle, but code is
little bit longer :(, but I hope, it is more readable.

btw, having array_get_isnul accessible from contrib code is nice (I
used to copy/paste it for my own needs)

Regards

Pavel

I don't know a better short index identifiers than I used. But I am
not against to change.

I'll try to redesign main cycle.

Regards

Pavel Stehule

       Thanks,

               Stephen

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)

iEYEARECAAYFAk086MEACgkQrzgMPqB3kigCzQCffx0iVSMjU2UbOgAOaY/MvtOp
iKsAnA5tdhKxTssdXJ+Rda4qkhNVm26g
=Yn5O
-----END PGP SIGNATURE-----

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

--
Cédric Villemain               2ndQuadrant
http://2ndQuadrant.fr/     PostgreSQL : Expertise, Formation et Support

#14Itagaki Takahiro
itagaki.takahiro@gmail.com
In reply to: Pavel Stehule (#12)
Re: REVIEW: WIP: plpgsql - foreach in

On Mon, Jan 24, 2011 at 20:10, Pavel Stehule <pavel.stehule@gmail.com> wrote:

we have to iterate over array's items because it allow seq. access to
array's data. I need a global index for function "array_get_isnull". I
can't to use a buildin functions like array_slize_size or
array_get_slice, because there is high overhead of array_seek
function. I redesigned the initial part of main cycle, but code is
little bit longer :(, but I hope, it is more readable.

The attached patch only includes changes in plpgsql. I tested it
combined with the previous one, that includes other directories.

* src/pl/plpgsql/src/gram.y
| for_variable K_SLICE ICONST
The parameter for SLICE must be an integer literal. Is it a design?
I got a syntax error when I wrote a function like below. However,
FOREACH returns different types on SLICE = 0 or SLICE >= 1.
(element type for 0, or array type for >=1) So, we cannot write
a true generic function that accepts all SLICE values even if
the syntax supports an expr for the parameter.

CREATE FUNCTION foreach_test(int[], int) RETURNS SETOF int[] AS
$$
DECLARE
a integer[];
BEGIN
FOREACH a SLICE $2 IN $1 LOOP -- syntax error
RETURN NEXT a;
END LOOP;
END;
$$ LANGUAGE plpgsql;

* /doc/src/sgml/plpgsql.sgml
+ FOREACH <replaceable>target</replaceable> <optional> SCALE
s/SCALE/SLICE/ ?

* Why do you use "foreach_a" for some identifiers?
We use "foreach" only for arrays, so "_a" suffix doesn't seem needed.

* accumArrayResult() for SLICE has a bit overhead.
If we want to optimize it, we could use memcpy() because slices are
placed in continuous memory. But I'm not sure the worth; I guess
FOREACH will be used with SLICE = 0 in many cases.

--
Itagaki Takahiro

#15Pavel Stehule
pavel.stehule@gmail.com
In reply to: Itagaki Takahiro (#14)
Re: REVIEW: WIP: plpgsql - foreach in

2011/1/26 Itagaki Takahiro <itagaki.takahiro@gmail.com>:

On Mon, Jan 24, 2011 at 20:10, Pavel Stehule <pavel.stehule@gmail.com> wrote:

we have to iterate over array's items because it allow seq. access to
array's data. I need a global index for function "array_get_isnull". I
can't to use a buildin functions like array_slize_size or
array_get_slice, because there is high overhead of array_seek
function. I redesigned the initial part of main cycle, but code is
little bit longer :(, but I hope, it is more readable.

The attached patch only includes changes in plpgsql. I tested it
combined with the previous one, that includes other directories.

* src/pl/plpgsql/src/gram.y
| for_variable K_SLICE ICONST
The parameter for SLICE must be an integer literal. Is it a design?
I got a syntax error when I wrote a function like below. However,
FOREACH returns different types on SLICE = 0 or SLICE >= 1.
(element type for 0, or array type for >=1)  So, we cannot write
a true generic function that accepts all SLICE values even if
the syntax supports an expr for the parameter.

Yes, it is design. You wrote a reason. A parametrized SLICE needs only
a few lines more, but a) I really don't know a use case, b) there is
significant difference between slice 0 and slice 1

CREATE FUNCTION foreach_test(int[], int) RETURNS SETOF int[] AS
$$
DECLARE
 a integer[];
BEGIN
 FOREACH a SLICE $2 IN $1 LOOP -- syntax error
   RETURN NEXT a;
 END LOOP;
END;
$$ LANGUAGE plpgsql;

* /doc/src/sgml/plpgsql.sgml
+ FOREACH <replaceable>target</replaceable> <optional> SCALE
s/SCALE/SLICE/ ?

* Why do you use "foreach_a" for some identifiers?
We use "foreach" only for arrays, so "_a" suffix doesn't seem needed.

yes, it isn't needed. But FOREACH is a new construct, that can be used
for more different iterations - maybe iterations over hstore,
element's set, ...

so _a means - this is implementation for arrays.

* accumArrayResult() for SLICE has a bit overhead.
If we want to optimize it, we could use memcpy() because slices are
placed in continuous memory. But I'm not sure the worth; I guess
FOREACH will be used with SLICE = 0 in many cases.

I agree with you, so SLICE > 0 will not be used often.

accumArrayResult isn't expensive function - for non varlena types. The
cutting of some array needs a redundant code to CopyArrayEls and
construct_md_array. All core functions are not good for our purpose,
because doesn't expect a seq array reading :(. Next - simply copy can
be done only for arrays without null bitmap, else you have to do some
bitmap rotations :(. So, I don't would do this optimization in this
moment. It has sense for multidimensional numeric arrays, but can be
solved in next version. It's last commitfest now, and I don't do this
patch more complex then it is now.

Regards

Pavel Stehule

Show quoted text

--
Itagaki Takahiro

#16Itagaki Takahiro
itagaki.takahiro@gmail.com
In reply to: Stephen Frost (#10)
Re: REVIEW: WIP: plpgsql - foreach in

On Mon, Jan 24, 2011 at 13:05, Stephen Frost <sfrost@snowman.net> wrote:

FOR var in ARRAY array_expression ...

I like that a lot more than inventing a new top-level keyword,

AFAIR, the syntax is not good at an array literal.
FOR var IN ARRAY ARRAY[1,2,5] LOOP ...
And it was the only drawback compared with FOREACH var IN expr.

--
Itagaki Takahiro

#17Stephen Frost
sfrost@snowman.net
In reply to: Itagaki Takahiro (#16)
Re: REVIEW: WIP: plpgsql - foreach in

* Itagaki Takahiro (itagaki.takahiro@gmail.com) wrote:

On Mon, Jan 24, 2011 at 13:05, Stephen Frost <sfrost@snowman.net> wrote:

FOR var in ARRAY array_expression ...

I like that a lot more than inventing a new top-level keyword,

AFAIR, the syntax is not good at an array literal.
FOR var IN ARRAY ARRAY[1,2,5] LOOP ...

I don't really see why that's "not good"? So you have 'ARRAY' twice..
So what? That's better than having a new top-level FOREACH that doesn't
have any reason to exist except to be different from FOR and to not do
the same thing..

Thanks,

Stephen

#18Stephen Frost
sfrost@snowman.net
In reply to: Pavel Stehule (#11)
Re: REVIEW: WIP: plpgsql - foreach in

* Pavel Stehule (pavel.stehule@gmail.com) wrote:

FOR keyword - please, look on thread about my proposal FOR-IN-ARRAY

I did, and I still don't agree w/ using FOREACH.

I work with FOUND variable, because I like a consistent behave with
FOR statement. When FOUND is true after cycle, you are sure, so there
was a minimally one iteration.

Then the documentation around FOUND needs to be updated for FOREACH, and
there's probably other places that need to be changed too.

There also appears to be some extra whitespace changes that aren't
necessary and a number of places where you don't follow the indentation
conventions (eg: variable definitions in exec_stmt_foreach_a()).

I am really not sure about correct indentation of variables :(, if you
know a correct number of spaces, please, fix it.

It's not a matter of a 'correct number of space'- make it the same as
what it is in the rest of the code... The gist of it is to make the
variable names all line up (with maximum use of tabs at 4-spaces per
tab, of course):

int my_var;
char *my_string;
double my_double;

etc, etc.

I'll try to redesign main cycle.

Thanks,

Stephen

#19Pavel Stehule
pavel.stehule@gmail.com
In reply to: Stephen Frost (#17)
Re: REVIEW: WIP: plpgsql - foreach in

2011/1/29 Stephen Frost <sfrost@snowman.net>:

* Itagaki Takahiro (itagaki.takahiro@gmail.com) wrote:

On Mon, Jan 24, 2011 at 13:05, Stephen Frost <sfrost@snowman.net> wrote:

FOR var in ARRAY array_expression ...

I like that a lot more than inventing a new top-level keyword,

AFAIR, the syntax is not good at an array literal.
  FOR var IN ARRAY ARRAY[1,2,5] LOOP ...

I don't really see why that's "not good"?  So you have 'ARRAY' twice..
So what?  That's better than having a new top-level FOREACH that doesn't
have any reason to exist except to be different from FOR and to not do
the same thing..

I don't see a problem too, but we didn't find a compromise with this
syntax, so I left it. It is true, so current implementation of FOR
stmt is really baroque and next argument is a compatibility with
PL/SQL. My idea is so FOR stmt will be a compatible with PL/SQL
original, and FOREACH can be a platform for PostgreSQL specific code.

Regards

Pavel

Show quoted text

       Thanks,

               Stephen

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)

iEYEARECAAYFAk1D/u8ACgkQrzgMPqB3kij2IwCfZ3W+mGc7LedBdnt9lCa0vYjk
m6QAn0xh7r6oTs+T47k+EuwZRpU2T0X8
=ruBa
-----END PGP SIGNATURE-----

#20Pavel Stehule
pavel.stehule@gmail.com
In reply to: Stephen Frost (#18)
Re: REVIEW: WIP: plpgsql - foreach in

I'll try to redesign main cycle.

       Thanks,

please, can you look on code that I sent last time?

Pavel

Show quoted text

               Stephen

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)

iEUEARECAAYFAk1EAJwACgkQrzgMPqB3kig5bACdH0fm8Klh7Dq1GlICV/Z8yEd4
KQoAlRZEeTrBkKK6TwjZrKmFDDeRfKE=
=JPG4
-----END PGP SIGNATURE-----

#21Stephen Frost
sfrost@snowman.net
In reply to: Pavel Stehule (#19)
Re: REVIEW: WIP: plpgsql - foreach in

* Pavel Stehule (pavel.stehule@gmail.com) wrote:

I don't see a problem too, but we didn't find a compromise with this
syntax, so I left it. It is true, so current implementation of FOR
stmt is really baroque and next argument is a compatibility with
PL/SQL. My idea is so FOR stmt will be a compatible with PL/SQL
original, and FOREACH can be a platform for PostgreSQL specific code.

I see that as an absolutely horrible idea. If you want that, it should
be "PGFOR" or something, but I don't buy off on the idea that we should
invent new top-level PG-specific keywords for PL/PgSQL because they're
PG-specific. I also don't see why FOR wouldn't still be as compatible
w/ PL/SQL as it was before (except in the possible case where someone
actually has 'ARRAY' there already, but I'm pretty sure we can convince
ourselves that such a construct is very unlikely to appear in the wild).

I certainly don't think we should *not* do something under FOR because
we're worried that people might use it and then get unhappy when they
port that code to PL/SQL.

Thanks,

Stephen

#22Stephen Frost
sfrost@snowman.net
In reply to: Pavel Stehule (#20)
Re: REVIEW: WIP: plpgsql - foreach in

* Pavel Stehule (pavel.stehule@gmail.com) wrote:

please, can you look on code that I sent last time?

I'm looking at it now and I still don't like the big set of conditionals
at the beginning which sets things up. I do think the loop is a bit
better, but have you considered factoring out the array slicing..? If
the built-ins don't give you what you want, write your own? Or make
them do what you need?

Point is, that array-slicing logic has a very clear and defined purpose,
input and output, and it could be another function.

Err, oh, except you have this horribly named 'ptr' variable that's
being used across the loops. Gah. Alright, I'd suggest actually making
an array iterator function then, which can single-step and pull out
slices, along with a single-step iterator, and then changing the top
level to be a while() loop on that. Notionally it's like this:

while (curr_slice_ptr =
array_slice(arr, curr_slice_ptr, slice_len, &slice, &isnull))
{
exec_assign_value(estate, ctrl_var, slice, valtype, &isnull);
rc = exec_stmts(estate, stmt->body);
/* handle return codes */
}

array_slice() could be defined to return a single value if slice_len is
1, or you could just pull out the single element if it returned a
1-element array; either way would work, imv, so long as it's commented
appropriately.

Those are my thoughts at the moment. To be honest, I'd really like to
see this patch included in 9.1, since I can see myself using this
feature, so if you need help with some of this rework, let me know.

Thanks,

Stephen

#23Pavel Stehule
pavel.stehule@gmail.com
In reply to: Stephen Frost (#21)
Re: REVIEW: WIP: plpgsql - foreach in

2011/1/29 Stephen Frost <sfrost@snowman.net>:

* Pavel Stehule (pavel.stehule@gmail.com) wrote:

I don't see a problem too, but we didn't find a compromise with this
syntax, so I left it. It is true, so current implementation of FOR
stmt is really baroque and next argument is a compatibility with
PL/SQL. My idea is so FOR stmt will be a compatible with PL/SQL
original, and FOREACH can be a platform for PostgreSQL specific code.

I see that as an absolutely horrible idea.  If you want that, it should
be "PGFOR" or something, but I don't buy off on the idea that we should
invent new top-level PG-specific keywords for PL/PgSQL because they're
PG-specific.  I also don't see why FOR wouldn't still be as compatible
w/ PL/SQL as it was before (except in the possible case where someone
actually has 'ARRAY' there already, but I'm pretty sure we can convince
ourselves that such a construct is very unlikely to appear in the wild).

you can rename it as some different than original ADA concept, if you
like. It is similar like FORALL cycle from PL/SQL. Native ADA's FOR
cycle doesn't know iteration over array.

You have a similar opinion like me about design this statement. But
there are others with strong negative opinion. For someone ARRAY ARRAY
should be a problem. So FOREACH is third way - more, it increase a
possibility for enhancing plpgsql in future.

I certainly don't think we should *not* do something under FOR because
we're worried that people might use it and then get unhappy when they
port that code to PL/SQL.

PL/pgSQL is some like Frankenstein :) Fast, functional but sometime
strange - more stranger than origin. I don't think so it necessary to
do live harder for people who have to work with both databases.

the main issue was a maintainability of more complex FOR statement.

Pavel

Show quoted text

       Thanks,

               Stephen

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)

iEYEARECAAYFAk1EBDwACgkQrzgMPqB3kijDLQCcCpb15jTvU3rIdh5j9ipaqq+X
G+wAn2WrxDkgArf5xHxt4bi8EpE0HVFP
=Fx/8
-----END PGP SIGNATURE-----

#24Stephen Frost
sfrost@snowman.net
In reply to: Pavel Stehule (#23)
Re: REVIEW: WIP: plpgsql - foreach in

* Pavel Stehule (pavel.stehule@gmail.com) wrote:

You have a similar opinion like me about design this statement. But
there are others with strong negative opinion. For someone ARRAY ARRAY
should be a problem. So FOREACH is third way - more, it increase a
possibility for enhancing plpgsql in future.

I look forward to hearing from the silent majority on this then.

the main issue was a maintainability of more complex FOR statement.

That would be a reason to not have this functionality at all, not a
reason to add confusion with a new top-level command.

Thanks,

Stephen

#25Tom Lane
tgl@sss.pgh.pa.us
In reply to: Stephen Frost (#24)
Re: REVIEW: WIP: plpgsql - foreach in

Stephen Frost <sfrost@snowman.net> writes:

* Pavel Stehule (pavel.stehule@gmail.com) wrote:

You have a similar opinion like me about design this statement. But
there are others with strong negative opinion. For someone ARRAY ARRAY
should be a problem. So FOREACH is third way - more, it increase a
possibility for enhancing plpgsql in future.

I look forward to hearing from the silent majority on this then.

Well, I haven't been exactly silent, but I was one of the people telling
Pavel not to use FOR in the first place. The trouble is that we've
already overloaded FOR to within an inch of its life. Adding yet
another potential syntax to follow FOR ... IN ... is just a bad idea,
especially since Pavel has evidently got ambitions to add yet more
nonstandard hac^H^H^Hfeatures here.

I have to agree that FOREACH is pretty ugly too, but I do *not* want to
use a syntax that can so easily be confused with the existing types of
for-loops. We'd pay a significant price in the ability to issue syntax
error messages that were actually relevant to what the user thought he
was doing, for example.

See also
http://archives.postgresql.org/pgsql-hackers/2010-12/msg01579.php
which tries to draw a clear distinction between what FOR does and what
FOREACH does.

regards, tom lane

#26Stephen Frost
sfrost@snowman.net
In reply to: Tom Lane (#25)
Re: REVIEW: WIP: plpgsql - foreach in

* Tom Lane (tgl@sss.pgh.pa.us) wrote:

See also
http://archives.postgresql.org/pgsql-hackers/2010-12/msg01579.php
which tries to draw a clear distinction between what FOR does and what
FOREACH does.

Thanks for that, somehow I had missed that post previously. I think I
can get behind the idea of FOREACH being used for 'vertical'
(multi-value in a single value) loops while FOR is used for 'horizontal'
(multi-row). This patch certainly needs to be improved to document
this, in the grammar, in the code via comments, and in the actual
documentation. It also needs to touch any place that talks about the
other kinds of loops to be sure that FOREACH is included and that it's
behavior is documented accordingly.

Thanks again,

Stephen

#27Pavel Stehule
pavel.stehule@gmail.com
In reply to: Stephen Frost (#26)
Re: REVIEW: WIP: plpgsql - foreach in

2011/1/29 Stephen Frost <sfrost@snowman.net>:

* Tom Lane (tgl@sss.pgh.pa.us) wrote:

See also
http://archives.postgresql.org/pgsql-hackers/2010-12/msg01579.php
which tries to draw a clear distinction between what FOR does and what
FOREACH does.

Thanks for that, somehow I had missed that post previously.  I think I
can get behind the idea of FOREACH being used for 'vertical'
(multi-value in a single value) loops while FOR is used for 'horizontal'
(multi-row).  This patch certainly needs to be improved to document
this, in the grammar, in the code via comments, and in the actual
documentation.  It also needs to touch any place that talks about the
other kinds of loops to be sure that FOREACH is included and that it's
behavior is documented accordingly.

Stephen, please, update documentation freely. Current documentation is
really minimal.

Regards

Pavel

Show quoted text

       Thanks again,

               Stephen

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.10 (GNU/Linux)

iEYEARECAAYFAk1EZJEACgkQrzgMPqB3kijrawCfbvtV/2QoJ6nLvKZENcslQgz+
do8An2Q7MvgubhLqrfbVCMiG29+RG3cp
=RpHJ
-----END PGP SIGNATURE-----