Re: some last patches breaks plan cache

2018-04-01 1:00 GMT+02:00 Tomas Vondra <tomas(dot)vondra(at)2ndquadrant(dot)com>:

>
>
> On 03/31/2018 08:28 PM, Tomas Vondra wrote:
> >
> >
> > On 03/31/2018 07:56 PM, Tomas Vondra wrote:
> >> On 03/31/2018 07:38 PM, Pavel Stehule wrote:
> >>> Hi
> >>>
> >>> CREATE OR REPLACE PROCEDURE public.proc(a integer, INOUT b integer, c
> >>> integer)
> >>> LANGUAGE plpgsql
> >>> AS $procedure$
> >>> begin
> >>> b := a + c;
> >>> end;
> >>> $procedure$
> >>>
> >>> CREATE OR REPLACE PROCEDURE public.testproc()
> >>> LANGUAGE plpgsql
> >>> AS $procedure$
> >>> declare r int;
> >>> begin
> >>> call proc(10, r, 20);
> >>> end;
> >>> $procedure$
> >>>
> >>> postgres=# call testproc();
> >>> CALL
> >>> postgres=# call testproc();
> >>> ERROR: SPI_execute_plan_with_paramlist failed executing query "CALL
> >>> proc(10, r, 20)": SPI_ERROR_ARGUMENT
> >>> CONTEXT: PL/pgSQL function testproc() line 4 at CALL
> >>> postgres=#
> >>>
> >>> second call fails
> >>
> >> Yeah.
> >>
> >> d92bc83c48bdea9888e64cf1e2edbac9693099c9 seems to have broken this :-/
> >>
> >
> > FWIW it seems the issue is somewhere in exec_stmt_call, which does this:
> >
> > /*
> > * Don't save the plan if not in atomic context. Otherwise,
> > * transaction ends would cause warnings about plan leaks.
> > */
> > exec_prepare_plan(estate, expr, 0, estate->atomic);
> >
> > When executed outside transaction, CALL has estate->atomic=false, and so
> > calls exec_prepare_plan() with keepplan=false. And on the second call it
> > gets bogus Plan, of course (with the usual 0x7f7f7f7f7f7f7f7f patterns).
> >
> > When in a transaction, it sets keepplan=true, and everything works fine.
> >
> > So either estate->atomic is not sufficient on it's own, or we need to
> > reset the expr->plan somewhere.
> >
>
> The attached patch fixes this, but I'm not really sure it's the right
> fix - I'd expect there to be a more principled way, doing resetting the
> plan pointer when 'plan->saved == false'.
>
>
it fixes some issue, but not all

I see changes in plpgsql_check regress tests

CREATE OR REPLACE PROCEDURE public.testproc()
LANGUAGE plpgsql
AS $procedure$
declare r int;
begin
call proc(10, r + 10, 20);
end;
$procedure$

postgres=# call testproc();
ERROR: argument 2 is an output argument but is not writable
CONTEXT: PL/pgSQL function testproc() line 4 at CALL
postgres=# call testproc();
ERROR: SPI_execute_plan_with_paramlist failed executing query "CALL
proc(10, r + 10, 20)": SPI_ERROR_ARGUMENT
CONTEXT: PL/pgSQL function testproc() line 4 at CALL

> regards
>
> --
> Tomas Vondra http://www.2ndQuadrant.com
> PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
>