Skip site navigation (1) Skip section navigation (2)

Re: [REVIEW] prepare plans of embedded sql on function start

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: (view raw, whole thread or download thread mbox)
Lists: pgsql-hackers

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

In response to

pgsql-hackers by date

Next:From: Fujii MasaoDate: 2011-09-11 04:24:10
Subject: Re: pg_last_xact_insert_timestamp
Previous:From: Bruce MomjianDate: 2011-09-11 02:18:44
Subject: Re: ts_rank

Privacy Policy | About PostgreSQL
Copyright © 1996-2018 The PostgreSQL Global Development Group