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


>>
>
> 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-----
>>
>>
>
*** ./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)
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to