Re: [REVIEW] prepare plans of embedded sql on function start - Mailing list pgsql-hackers

Hello

thank you very much for review

> Will always throw an error because at prepare time, the temp junk table wont
> exist.  This patch implements new syntax to disable the check:
>
> create function test5() returns integer as $$
> #prepare_plans on_demand
> begin
> ...
>
> Was it Tom Lane that said, "if we add new syntax, we have to support it
> forever"?  As a helpful feature I can see people (myself included) enabling
> this system wide.  So what happens to all the plpgsql on pgxn that this
> happens to break?  Well it needs updated, no problem, but the fix will be to
> add "#prepare_plans on_demand" in the magic spot.  That breaks it for prior
> versions of PG.  Is this the syntax we want?  What if we add more "compiler
> flags" in the future:
>
> create function test5() returns integer as $$
> #prepare_plans on_demand
> #disable_xlog
> #work_mem 10MB
> begin
>  create temp table junk(id integer);
>  insert into junk(id) values(100);
>  drop table temp;
>  return 1;
> end;
> $$ language plpgsql;
>

I am sure, so we will support a plan based statements inside PL very
long time. But this is not only one way, how to change a behave. You
can use a plpgsql.prepare_plans variable too. Theoretically We can
live without prepare_plans option - it doesn't modify a function's
behave - so it must not be part of function body. But I think, so it
is more readable - and it stronger warning for developers -
"attention, this function was not checked deeply". I have no problem
to remove this option, and use only a custom GUC

> I don't have an answer to that.  Other sql implement via OPTION(...).
>
> One option I'd thought about, was to extended ANALYZE to support functions.
>  It would require no additional plpgsql syntax changes, no postgresql.conf
> settings.  If you wanted to prepare (prepare for a testing purpose, not a
> performance purpose) all the sql inside your function, youd:
>
> analyze test5();

I am not against to some analogy to what you mean - just dislike a use
of ANALYZE keyword. Just static check has a one significant
disadvantage - we should not to identify types of NEW and OLD
variables in triggers.



>
> I'd expect to get errors from that, because the junk table doesn't exist.
>  I'd expect it, and just never analyze it.
>
>
>
> Summary
> =======
> Its a tough one.  I see benefit here.  I can see myself using it.  If I had
> to put my finger on it, I'm not 100% sold on the syntax.  It is usable
> though, it does solve problems, so I'd use it.  (I'm not 100% sure ANALYZE
> is better, either).
>
> I'm going to leave this patch as "needs review", I think more eyes might be
> helpful.
>
> -Andy
>

Thank you very much

Pavel Stehule