*** a/src/pl/plpgsql/src/gram.y --- b/src/pl/plpgsql/src/gram.y *************** *** 192,198 **** static List *read_raise_options(void); %type stmt_return stmt_raise stmt_execsql %type stmt_dynexecute stmt_for stmt_perform stmt_getdiag %type stmt_open stmt_fetch stmt_move stmt_close stmt_null ! %type stmt_case stmt_foreach_a %type proc_exceptions %type exception_sect --- 192,198 ---- %type stmt_return stmt_raise stmt_execsql %type stmt_dynexecute stmt_for stmt_perform stmt_getdiag %type stmt_open stmt_fetch stmt_move stmt_close stmt_null ! %type stmt_case stmt_foreach_a %type proc_exceptions %type 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 $$