|From:||Stephen Frost <sfrost(at)snowman(dot)net>|
|To:||Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com>|
|Cc:||PostgreSQL Hackers <pgsql-hackers(at)postgresql(dot)org>|
|Subject:||Re: REVIEW: WIP: plpgsql - foreach in|
|Views:||Raw Message | Whole Thread | Download mbox | Resend email|
* Pavel Stehule (pavel(dot)stehule(at)gmail(dot)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
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.
|Next Message||Pavel Stehule||2011-01-29 13:05:27||Re: REVIEW: WIP: plpgsql - foreach in|
|Previous Message||Thom Brown||2011-01-29 12:27:32||Re: Snapshots no longer build|