On Wed, 10 Jun 2020 at 17:12, Pavel Stehule <pavel.stehule@gmail.com> wrote: > st 10. 6. 2020 v 12:26 odesílatel Amit Khandekar <amitdkhan.pg@gmail.com> napsal: >> Could you show an example testcase that tests this recursive scenario, >> with which your earlier patch fails the test, and this v2 patch passes >> it ? I am trying to understand the recursive scenario and the re-use >> of expr->plan. > > > it hangs on plpgsql tests. So you can apply first version of patch > > and "make check"
I could not reproduce the make check hang with the v1 patch. But I could see a crash with the below testcase. So I understand the purpose of the plan_owner variable that you introduced in v2.
Consider this recursive test :
create or replace procedure p1(in r int) as $$ begin RAISE INFO 'r : % ', r; if r < 3 then call p1(r+1); end if; end $$ language plpgsql;
do $$ declare r int default 1; begin call p1(r); end; $$;
In p1() with r=2, when the stmt "call p1(r+1)" is being executed, consider this code of exec_stmt_call() with your v2 patch applied: if (expr->plan && !expr->plan->saved) { if (plan_owner) SPI_freeplan(expr->plan); expr->plan = NULL; }
Here, plan_owner is false. So SPI_freeplan() will not be called, and expr->plan is set to NULL. Now I have observed that the stmt pointer and expr pointer is shared between the p1() execution at this r=2 level and the p1() execution at r=1 level. So after the above code is executed at r=2, when the upper level (r=1) exec_stmt_call() lands to the same above code snippet, it gets the same expr pointer, but it's expr->plan is already set to NULL without being freed. From this logic, it looks like the plan won't get freed whenever the expr/stmt pointers are shared across recursive levels, since expr->plan is set to NULL at the lowermost level ? Basically, the handle to the plan is lost so no one at the upper recursion level can explicitly free it using SPI_freeplan(), right ? This looks the same as the main issue where the plan does not get freed for non-recursive calls. I haven't got a chance to check if we can develop a testcase for this, similar to your testcase where the memory keeps on increasing.
