Hi st 19. 8. 2020 v 19:22 odesÃlatel Konstantin Knizhnik < k.knizh...@postgrespro.ru> napsal:
> Hi hackers, > > More than month ago I have sent bug report to pgsql-bugs: > > > https://www.postgresql.org/message-id/flat/5d335911-fb25-60cd-4aa7-a5bd0954aea0%40postgrespro.ru > > with the proposed patch but have not received any response. > > I wonder if there is some other way to fix this issue and does somebody > working on it. > While the added check itself is trivial (just one line) the total patch > is not so small because I have added walker for > plpgsql statements tree. It is not strictly needed in this case (it is > possible to find some other way to determine that stored procedure > contains transaction control statements), but I hope such walker may be > useful in other cases. > > In any case, I will be glad to receive any response, > because this problem was reported by one of our customers and we need to > provide some fix. > It is better to include it in vanilla, rather than in our pgpro-ee fork. > > If it is desirable, I can add this patch to commitfest. > I don't like this design. It is not effective to repeat the walker for every execution. Introducing a walker just for this case looks like overengineering. Personally I am not sure if a walker for plpgsql is a good idea (I thought about it more times, when I wrote plpgsql_check). But anyway - there should be good reason for introducing the walker and clean use case. If you want to introduce stmt walker, then it should be a separate patch with some benefit on plpgsql environment length. Regards Pavel > Thanks in advance, > Konstantin > >
diff --git a/src/pl/plpgsql/src/pl_exec.c b/src/pl/plpgsql/src/pl_exec.c index d4a3d58daa..ccbddd4d7a 100644 --- a/src/pl/plpgsql/src/pl_exec.c +++ b/src/pl/plpgsql/src/pl_exec.c @@ -5997,6 +5997,12 @@ exec_for_query(PLpgSQL_execstate *estate, PLpgSQL_stmt_forq *stmt, */ PinPortal(portal); + /* + * Disable prefetch if procedure contains COMMIT or ROLLBACK statements + */ + if (prefetch_ok && estate->func->fn_xactctrl) + prefetch_ok = false; + /* * Fetch the initial tuple(s). If prefetching is allowed then we grab a * few more rows to avoid multiple trips through executor startup diff --git a/src/pl/plpgsql/src/pl_gram.y b/src/pl/plpgsql/src/pl_gram.y index 5a7e1a4444..5b27311b95 100644 --- a/src/pl/plpgsql/src/pl_gram.y +++ b/src/pl/plpgsql/src/pl_gram.y @@ -2215,6 +2215,8 @@ stmt_commit : K_COMMIT opt_transaction_chain ';' new->stmtid = ++plpgsql_curr_compile->nstatements; new->chain = $2; + plpgsql_curr_compile->fn_xactctrl = true; + $$ = (PLpgSQL_stmt *)new; } ; @@ -2229,6 +2231,8 @@ stmt_rollback : K_ROLLBACK opt_transaction_chain ';' new->stmtid = ++plpgsql_curr_compile->nstatements; new->chain = $2; + plpgsql_curr_compile->fn_xactctrl = true; + $$ = (PLpgSQL_stmt *)new; } ; diff --git a/src/pl/plpgsql/src/plpgsql.h b/src/pl/plpgsql/src/plpgsql.h index 0c3d30fb13..e9b9b0d335 100644 --- a/src/pl/plpgsql/src/plpgsql.h +++ b/src/pl/plpgsql/src/plpgsql.h @@ -1006,6 +1006,7 @@ typedef struct PLpgSQL_function bool fn_retisdomain; bool fn_retset; bool fn_readonly; + bool fn_xactctrl; char fn_prokind; int fn_nargs;
