Greetings,
* Pavel Stehule (pavel.stehule@gmail.com) wrote:
> This patch remove redundant rows from PL/pgSQL executor (-89 lines).
While I can certainly appreciate wanting to remove redundant code, I
don't think this change actually improves the situation. The problem is
more than just that we might want to make a change to 'while' but not
'for', it's also that it makes it very difficult to follow the code
flow.
If you could have found a way to make it work for all calls to
exec_stmts(), it might be a bit better, but you can't without kludging
it up further. If you could, then it might be possible to move some of
this logic *into* exec_stmts(), but I don't see that being reasonable
either.
In the end, I'd recommend cleaning up the handling of the exec_stmts()
return code so that all of these pieces follow the same style and look
similar (I'd go with the switch-based approach and remove the if/else
branches). That'll make it easier for anyone coming along later who
does end up needing to change all three.
> Doesn't change a functionality.
I'm not convinced of this either, to be honest.. In exec_stmt_while(),
there is:
for(;;)
{
[...]
if (isnull || !value)
break;
rc = exec_stmts(estate, stmt->body);
[...]
}
return PLPGSQL_RC_OK;
You replaced the last return with:
return rc;
Which could mean returning an uninitialized rc if the above break;
happens.
In the end, I guess it's up to the committers on how they feel about
this, so here's an updated patch which fixes the above issue and
improves the comments/grammer. Passes all regressions and works in my
limited testing.
commit e6639d83db5b301e184bf158b1591007a3f79526
Author: Stephen Frost <sfrost@snowman.net>
Date: Wed Jan 19 14:28:36 2011 -0500
Improve comments in pl_exec.c wrt can_leave_loop()
This patch improves some of the comments around can_leave_loop(), and
fixes a couple of fall-through cases to make sure they behave the same
as before the code de-duplication patch that introduced
can_leave_loop().
commit f8262adcba662164dbc24d289cb036b3f0aee582
Author: Stephen Frost <sfrost@snowman.net>
Date: Wed Jan 19 14:26:27 2011 -0500
remove redundant code from pl_exec.c
This patch removes redundant handling of exec_stmts()'s return code
by creating a new function to be called after exec_stmts() is run.
This new function will then handle the return code, update it if
necessary, and return if the loop should continue or not.
Patch by Pavel Stehule.
Thanks,
Stephen
