From: | Pavel Stehule <pavel(dot)stehule(at)gmail(dot)com> |
---|---|
To: | Andy Colson <andy(at)squeakycode(dot)net> |
Cc: | pgsql-hackers <pgsql-hackers(at)postgresql(dot)org> |
Subject: | Re: [REVIEW] prepare plans of embedded sql on function start |
Date: | 2011-09-11 04:17:50 |
Message-ID: | CAFj8pRAkDhZB1JCPRX+hNMBnuAyZfqyk4GtwcpiyNGURpFW0fA@mail.gmail.com |
Views: | Raw Message | Whole Thread | Download mbox | Resend email |
Thread: | |
Lists: | 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
From | Date | Subject | |
---|---|---|---|
Next Message | Fujii Masao | 2011-09-11 04:24:10 | Re: pg_last_xact_insert_timestamp |
Previous Message | Bruce Momjian | 2011-09-11 02:18:44 | Re: ts_rank |