Re: REVIEW: WIP: plpgsql - foreach in

Pavel,

* Pavel Stehule (pavel(dot)stehule(at)gmail(dot)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