2011/1/24 Stephen Frost <sfr...@snowman.net>:
> Pavel,
>
> * Pavel Stehule (pavel.steh...@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
> 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
