Hi > The reason that this case wasn't covered in 8.3 is that there didn't > seem to be a use-case that justified doing the extra work. I still > haven't seen one. You just stopped reading the thread where it was discussed after your troll remark?
> Other than inline-able SQL functions there is no reason to invalidate a stored plan > based on the fact that some function it called changed contents. Isn't it reason enough for this patch? ERROR: cache lookup failed for function is normal and good behaviour and should not be recoverd from because it never happen if you PostgreSQL right :) Usecase 1: Inlined functions 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; postgres=# prepare c2 as select salary, salary_without_income_tax(salary) from salaries; postgres=# execute c2; salary | salary_without_income_tax --------+--------------------------- 10000 | 7600.00 postgres=# create or replace function salary_without_income_tax(i_salary in numeric, salary out numeric ) returns numeric as $$ select $1 * 0.74 as salary $$ language sql; postgres=# execute c2; salary | salary_without_income_tax --------+--------------------------- 10000 | 7600.00 Use case 2: While rewriting existing modules due to changes in business requirements then in addition to new code we have to refactor lots of old functions one natural thing to do would be to get rid of return types as they are even more inconvenient to use than out parameters. Another reason is keep coding style consistent over modules so future maintenace will be less painful in the assholes. postgres=# create type public.ret_status as ( status integer, status_text text); CREATE TYPE postgres=# create or replace function x ( i_param text ) returns public.ret_status as $$ select 200::int, 'ok'::text; $$ language sql; CREATE FUNCTION postgres=# create or replace function x ( i_param text, status OUT int, status_text OUT text ) returns record as $$ select 200::int, 'ok'::text; $$ language sql; ERROR: cannot change return type of existing function HINT: Use DROP FUNCTION first Usecase 3.: Extra out parameters are needed in existing functions. I assure you if you have 5 years of legacy code that is constantly changing it does happen (often). 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; ERROR: cannot change return type of existing function DETAIL: Row type defined by OUT parameters is different. HINT: Use DROP FUNCTION first. Usecase 4: Things are even worse when you need to change the type that is used in functions. You have to drop and recreate the type and all the functions that are using it. Sometimes type is used in several functions and only some of them need changes. postgres=# create type public.ret_status_ext as ( status integer, status_text text, more_money numeric); CREATE TYPE postgres=# create or replace function x ( i_param text ) returns public.ret_status_ext as $$ select 200::int, 'ok'::text; $$ language sql; ERROR: cannot change return type of existing function HINT: Use DROP FUNCTION first. 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 postgres=# drop function x(text); DROP FUNCTION postgres=# create or replace function x ( i_param text ) returns public.ret_status_ext as $$ select 200::int, $1, 2.3 $$ language sql; CREATE FUNCTION postgres=# execute c; ERROR: cache lookup failed for function 24598 I hope i have answered your question "Why do you not use CREATE OR REPLACE FUNCTION?" That leaves us to deal with functions in our usual bad, wrong and stupid ways. * We create a function with new name and redirect all the calls to it. (stupid as it creates extra development, testing, code reviewing and releasing work and leaves around old code). * We pause pgBouncer and after release let it reconnect all connections (bad as it creates downtime). * We invalidate all procedures using update to pg_proc (simply wrong way to do it but still our best workaround short of restarting postgres). postgres=# update pg_proc set proname = proname; UPDATE 2152 postgres=# execute c2; salary | salary_without_income_tax --------+--------------------------- 10000 | 7400.00 > Perhaps Skype needs to rethink how they are modifying functions. We have had to change the way we use functions to suit PostgreSQL for 5 years now. That creates us quite a lot of extra work both on development side and DBA side plus the constantly hanging danger of 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 :) All in all it's not the job of PostgreSQL to tell the user what we 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 surrounding applications will say working. regards Asko PS: It all confuses poor developers "Hi Asko, I work on web backend and am in the process of changing infodb.eurorates_exchange() and infodb._eurorates_lookup db functions found in dbs. Problem is the _eurorates_lookup function did not use IN OUT params but a type, we have been told we should update all functions to use IN OUT params. In doing so I will also need to drop the function when it comes to deployment. This function is in the accounts partition and I know we are not supposed to drop functions in partitions due to cache problems it creates. Could you tell me what I should do? Thanks" On Tue, Aug 19, 2008 at 3:29 AM, Tom Lane <[EMAIL PROTECTED]> wrote: > "Asko Oja" <[EMAIL PROTECTED]> writes: > > For users of stored procedures it is protection from downtime. For Skype > it > > has been around 20% of databse related downtime this year. > > Perhaps Skype needs to rethink how they are modifying functions. > > The reason that this case wasn't covered in 8.3 is that there didn't > seem to be a use-case that justified doing the extra work. I still > haven't seen one. Other than inline-able SQL functions there is no > reason to invalidate a stored plan based on the fact that some function > it called changed contents. > > regards, tom lane >
