Re: Patch: plan invalidation vs stored procedures - Mailing list pgsql-hackers

<div dir="ltr">Hi<br /><br />> The reason that this case wasn't covered in 8.3 is that there didn't<br />> seem
tobe a use-case that justified doing the extra work.  I still<br /> > haven't seen one. <br />You just stopped
readingthe thread where it was discussed after your troll remark? <br /><br />> Other than inline-able SQL functions
thereis no reason to invalidate a stored plan <br />> based on the fact that some function it called changed
contents.<br/> Isn't it reason enough for this patch?<br />ERROR:  cache lookup failed for function is normal and good
behaviourand should not be recoverd from because it never happen if you PostgreSQL right :)<br /><br />Usecase 1:
Inlinedfunctions <br /> postgres=# create or replace function salary_without_income_tax(i_salary in numeric, salary out
numeric) returns numeric as $$ select $1 * 0.76 as salary $$ language sql;<br /> postgres=# prepare c2 as select
salary,salary_without_income_tax(salary) from salaries;<br /> postgres=#  execute c2;<br />  salary |
salary_without_income_tax<br /> --------+---------------------------<br />   10000 |                   7600.00<br />
postgres=#create or replace function salary_without_income_tax(i_salary in numeric, salary out numeric ) returns
numericas $$ select $1 * 0.74 as salary $$ language sql;<br /> postgres=#  execute c2; salary |
salary_without_income_tax<br /> --------+---------------------------<br />   10000 |                   7600.00<br /><br
/>Use case 2: While rewriting existing modules due to changes in business requirements then in addition to new code we
haveto refactor lots of old functions one natural thing to do would be to get rid of return types as they are even more
inconvenientto use than out parameters. Another reason is keep coding style consistent over modules so future
maintenacewill be less painful in the assholes.<br /> postgres=# create type public.ret_status as ( status integer,
status_texttext);<br />CREATE TYPE<br />postgres=# create or replace function x ( i_param text ) returns
public.ret_statusas $$ select 200::int, 'ok'::text; $$ language sql;<br /> CREATE FUNCTION<br />postgres=# create or
replacefunction x ( i_param text, status OUT int, status_text OUT text ) returns record as $$ select 200::int,
'ok'::text;$$ language sql;<br />ERROR:  cannot change return type of existing function<br /> HINT:  Use DROP FUNCTION
first<br/><br />Usecase 3.: Extra out parameters are needed in existing functions. I assure you if you have 5 years of
legacycode that is constantly changing it does happen (often).<br /> postgres=# create or replace function xl ( i_param
text,status OUT int, status_text OUT text, more_text OUT text ) returns record as $$ select 200::int, 'ok'::text,
'cat'::text;$$ language sql;<br /><div class="Ih2E3d"> ERROR:  cannot change return type of existing function<br
/></div>DETAIL: Row type defined by OUT parameters is different.<br />HINT:  Use DROP FUNCTION first.<br /><br
/>Usecase4: Things are even worse when you need to change the type that is used in functions. You have to drop and
recreatethe type and all the functions that are using it. Sometimes type is used in several functions and only some of
themneed changes.<br /> postgres=# create type public.ret_status_ext as ( status integer, status_text text, more_money
numeric);                                      <br />CREATE TYPE<br />postgres=# create or replace function x ( i_param
text) returns public.ret_status_ext as $$ select 200::int, 'ok'::text; $$ language sql;<br /> ERROR:  cannot change
returntype of existing function<br />HINT:  Use DROP FUNCTION first.<br /><br />And whenever we do drop and create as
hinted then we receive error flood that won't stop until something is manually done to get rid of it<br /> postgres=#
dropfunction x(text);<br />DROP FUNCTION<br />postgres=# create or replace function x ( i_param text ) returns
public.ret_status_extas $$ select 200::int, $1, 2.3 $$ language sql;<br />CREATE FUNCTION<br />postgres=# execute c;<br
/>ERROR:  cache lookup failed for function 24598<br /><br />I hope i have answered your question  "Why do you not use
CREATEOR REPLACE FUNCTION?" That leaves us to deal with functions in our usual bad, wrong and stupid ways.<br /> * We
createa function with new name and redirect all the calls to it. (stupid as it creates extra development, testing, code
reviewingand releasing work and leaves around old code). <br />* We pause pgBouncer and after release let it reconnect
allconnections (bad as it creates downtime).<br /> * We invalidate all procedures using update to pg_proc (simply wrong
wayto do it but still our best workaround short of restarting postgres).<br />postgres=# update pg_proc set proname =
proname;<br/>UPDATE 2152<br />postgres=#  execute c2;<br />  salary | salary_without_income_tax <br
/>--------+---------------------------<br/>  10000 |                   7400.00<br /> <br />> Perhaps Skype needs to
rethinkhow they are modifying functions.<br /> We have had to change the way we use functions to suit PostgreSQL for 5
yearsnow. That creates us quite a lot of extra work both on development side and DBA side plus the constantly hanging
dangerof downtime. Our DBA teams job is to reduce all possible causes for downtime and this patch is solution to one of
them.Sadly we just get trolled into the ground :)<br /><br />All in all it's not the job of PostgreSQL to tell the user
whatwe he can or can't replace in functions. It should be up to the user to decide what and how to replace so that all
surroundingapplications will say working. <br /><br />regards<br />Asko<br /><br /><div class="gmail_quote">PS: It all
confusespoor developers "Hi Asko, I work on web backend and am in the process of changing infodb.eurorates_exchange()
andinfodb._eurorates_lookup db functions found in dbs. Problem is the _eurorates_lookup function did not use IN OUT
paramsbut a type, we have been told we should update all functions to use IN OUT params. In doing so I will also need
todrop the function when it comes to deployment. This function is in the accounts partition and I know we are not
supposedto drop functions in partitions due to cache problems it creates. Could you tell me what I should do?
Thanks"<br/><br /><br /><br />On Tue, Aug 19, 2008 at 3:29 AM, Tom Lane <span dir="ltr"><<a
href="mailto:tgl@sss.pgh.pa.us">tgl@sss.pgh.pa.us</a>></span>wrote:<br /><blockquote class="gmail_quote"
style="border-left:1px solid rgb(204, 204, 204); margin: 0pt 0pt 0pt 0.8ex; padding-left: 1ex;"><div
class="Ih2E3d">"AskoOja" <<a href="mailto:ascoja@gmail.com">ascoja@gmail.com</a>> writes:<br /></div><div
class="Ih2E3d">>For users of stored procedures it is protection from downtime. For Skype it<br /> > has been
around20% of databse related downtime this year.<br /><br /></div>Perhaps Skype needs to rethink how they are modifying
functions.<br/><br /> The reason that this case wasn't covered in 8.3 is that there didn't<br /> seem to be a use-case
thatjustified doing the extra work.  I still<br /> haven't seen one.  Other than inline-able SQL functions there is
no<br/> reason to invalidate a stored plan based on the fact that some function<br /> it called changed contents.<br
/><br/>                        regards, tom lane<br /></blockquote></div><br /></div>