[HACKERS] patch: remove redundant code from pl_exec.c
Hello
This patch remove redundant rows from PL/pgSQL executor (-89 lines).
Doesn't change a functionality.
Regards
Pavel Stehule
*** ./src/pl/plpgsql/src/pl_exec.c.orig 2010-12-16 10:25:37.0 +0100
--- ./src/pl/plpgsql/src/pl_exec.c 2010-12-17 10:50:31.793623763 +0100
***
*** 204,209
--- 204,211
PLpgSQL_expr *dynquery, List *params,
const char *portalname, int cursorOptions);
+ static bool can_leave_loop(PLpgSQL_execstate *estate,
+ PLpgSQL_any_loop *stmt, int *rc);
/* --
* plpgsql_exec_function Called by the call handler for
***
*** 1566,1571
--- 1568,1637
return exec_stmts(estate, stmt-else_stmts);
}
+ /*
+ * function returns true, when outer cycle should be leaved
+ */
+ static bool
+ can_leave_loop(PLpgSQL_execstate *estate, PLpgSQL_any_loop *stmt, int *rc)
+ {
+ switch (*rc)
+ {
+ case PLPGSQL_RC_OK:
+ return false;
+
+ case PLPGSQL_RC_RETURN:
+ return true;
+
+ case PLPGSQL_RC_EXIT:
+ if (estate-exitlabel == NULL)
+ {
+ /* unlabelled exit, finish the current loop */
+ *rc = PLPGSQL_RC_OK;
+ }
+ if (stmt-label != NULL strcmp(stmt-label, estate-exitlabel) == 0)
+ {
+ /* labelled exit, matches the current stmt's label */
+ estate-exitlabel = NULL;
+ *rc = PLPGSQL_RC_OK;
+ }
+
+ /*
+ * otherwise, this is a labelled exit that does not match the
+ * current statement's label, if any: return RC_EXIT so that the
+ * EXIT continues to propagate up the stack.
+ */
+ return true;
+
+ case PLPGSQL_RC_CONTINUE:
+ if (estate-exitlabel == NULL)
+ {
+ /* anonymous continue, so re-run the loop */
+ *rc = PLPGSQL_RC_OK;
+ }
+ else if (stmt-label != NULL
+ strcmp(stmt-label, estate-exitlabel) == 0)
+ {
+ /* label matches named continue, so re-run loop */
+ estate-exitlabel = NULL;
+ *rc = PLPGSQL_RC_OK;
+ }
+ else
+ {
+ /*
+ * otherwise, this is a named continue that does not match the
+ * current statement's label, if any: return RC_CONTINUE so
+ * that the CONTINUE will propagate up the stack.
+ */
+ return true;
+ }
+
+ return false;
+
+ default:
+ elog(ERROR, unrecognized rc: %d, *rc);
+ return false; /* be compiler quiet */
+ }
+ }
/* --
* exec_stmt_loop Loop over statements until
***
*** 1575,1621
static int
exec_stmt_loop(PLpgSQL_execstate *estate, PLpgSQL_stmt_loop *stmt)
{
for (;;)
{
! int rc = exec_stmts(estate, stmt-body);
!
! switch (rc)
! {
! case PLPGSQL_RC_OK:
! break;
!
! case PLPGSQL_RC_EXIT:
! if (estate-exitlabel == NULL)
! return PLPGSQL_RC_OK;
! if (stmt-label == NULL)
! return PLPGSQL_RC_EXIT;
! if (strcmp(stmt-label, estate-exitlabel) != 0)
! return PLPGSQL_RC_EXIT;
! estate-exitlabel = NULL;
! return PLPGSQL_RC_OK;
!
! case PLPGSQL_RC_CONTINUE:
! if (estate-exitlabel == NULL)
! /* anonymous continue, so re-run the loop */
! break;
! else if (stmt-label != NULL
! strcmp(stmt-label, estate-exitlabel) == 0)
! /* label matches named continue, so re-run loop */
! estate-exitlabel = NULL;
! else
! /* label doesn't match named continue, so propagate upward */
! return PLPGSQL_RC_CONTINUE;
! break;
!
! case PLPGSQL_RC_RETURN:
! return rc;
! default:
! elog(ERROR, unrecognized rc: %d, rc);
! }
}
! return PLPGSQL_RC_OK;
}
--- 1641,1657
static int
exec_stmt_loop(PLpgSQL_execstate *estate, PLpgSQL_stmt_loop *stmt)
{
+ int rc;
+
for (;;)
{
! rc = exec_stmts(estate, stmt-body);
! if (can_leave_loop(estate, (PLpgSQL_any_loop *) stmt, rc))
! break;
}
! return rc;
}
***
*** 1628,1636
static int
exec_stmt_while(PLpgSQL_execstate *estate, PLpgSQL_stmt_while *stmt)
{
for (;;)
{
- int rc;
bool value;
bool isnull;
--- 1664,1673
static int
exec_stmt_while(PLpgSQL_execstate *estate, PLpgSQL_stmt_while *stmt)
{
+ int rc;
+
for (;;)
{
bool value;
bool isnull;
***
*** 1642,1684
rc = exec_stmts(estate, stmt-body);
! switch (rc)
! {
! case PLPGSQL_RC_OK:
! break;
!
! case PLPGSQL_RC_EXIT:
! if (estate-exitlabel == NULL)
! return PLPGSQL_RC_OK;
! if (stmt-label == NULL)
! return PLPGSQL_RC_EXIT;
! if (strcmp(stmt-label, estate-exitlabel) != 0)
! return PLPGSQL_RC_EXIT;
! estate-exitlabel = NULL;
! return PLPGSQL_RC_OK;
!
! case PLPGSQL_RC_CONTINUE:
! if (estate-exitlabel == NULL)
! /* anonymous continue, so re-run loop */
! break;
! else if (stmt-label != NULL
! strcmp(stmt-label, estate-exitlabel) == 0)
! /* label matches named continue, so re-run loop */
! estate-exitlabel = NULL;
! else
! /* label
Re: [HACKERS] patch: remove redundant code from pl_exec.c
Pavel Stehule pavel.steh...@gmail.com writes: This patch remove redundant rows from PL/pgSQL executor (-89 lines). Doesn't change a functionality. I'm not really impressed with this idea: there's no a priori reason that all those loop types would necessarily have exactly the same control logic. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] patch: remove redundant code from pl_exec.c
Excerpts from Pavel Stehule's message of vie dic 17 07:02:00 -0300 2010: Hello This patch remove redundant rows from PL/pgSQL executor (-89 lines). Doesn't change a functionality. Hmm I'm not sure but I think the new code has some of the result values inverted. Did you test this thoroughly? I think this would be a nice simplification because the repetitive coding is ugly and confusing, but I'm nervous about the unstated assumption that all loop structs are castable to the new struct. Seems like it could be easily broken in the future. -- Álvaro Herrera alvhe...@commandprompt.com The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] patch: remove redundant code from pl_exec.c
2010/12/17 Tom Lane t...@sss.pgh.pa.us: Pavel Stehule pavel.steh...@gmail.com writes: This patch remove redundant rows from PL/pgSQL executor (-89 lines). Doesn't change a functionality. I'm not really impressed with this idea: there's no a priori reason that all those loop types would necessarily have exactly the same control logic. this code processes a rc from EXIT, CONTINUE and RETURN statement. All these statements are defined independent to outer loops, so there are no reason why this code has be different. And actually removed code was almost same. There was different a process for FOR statement, because there isn't possible direct return from cycle, because is necessary to release a allocated memory. There is no reason why the processing should be same, but actually is same. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] patch: remove redundant code from pl_exec.c
2010/12/17 Alvaro Herrera alvhe...@commandprompt.com: Excerpts from Pavel Stehule's message of vie dic 17 07:02:00 -0300 2010: Hello This patch remove redundant rows from PL/pgSQL executor (-89 lines). Doesn't change a functionality. Hmm I'm not sure but I think the new code has some of the result values inverted. Did you test this thoroughly? I think this would be a nice simplification because the repetitive coding is ugly and confusing, but I'm nervous about the unstated assumption that all loop structs are castable to the new struct. Seems like it could be easily broken in the future. All regress tests was successful. A common structure isn't a new. There is same for FOR loops, there is some similar in parser yylval, and it is only explicit description of used construction for stmt structures. I should not to modify any other structure. But I am not strong in this. A interface can be changed and enhanced about pointer to label. Just I am not satisfied from current state, where same things are implemented with minimal difference. Pavel -- Álvaro Herrera alvhe...@commandprompt.com The PostgreSQL Company - Command Prompt, Inc. PostgreSQL Replication, Consulting, Custom Development, 24x7 support -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] patch: remove redundant code from pl_exec.c
Pavel Stehule pavel.steh...@gmail.com writes: 2010/12/17 Tom Lane t...@sss.pgh.pa.us: I'm not really impressed with this idea: there's no a priori reason that all those loop types would necessarily have exactly the same control logic. There is no reason why the processing should be same, but actually is same. Yes, and it might need to be different in future, whereupon this patch would make life extremely difficult. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] patch: remove redundant code from pl_exec.c
2010/12/17 Tom Lane t...@sss.pgh.pa.us: Pavel Stehule pavel.steh...@gmail.com writes: 2010/12/17 Tom Lane t...@sss.pgh.pa.us: I'm not really impressed with this idea: there's no a priori reason that all those loop types would necessarily have exactly the same control logic. There is no reason why the processing should be same, but actually is same. Yes, and it might need to be different in future, whereupon this patch would make life extremely difficult. Do you know about some possible change? I can't to argument with this argument. But I can use a same argument. Isn't be more practical to have a centralized management for return code? There is same probability to be some features in future that will need a modify this fragment - for example a new return code value. Without centralized management, you will have to modify four fragments instead one. Regards Pavel Stehule regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
