Re: [HACKERS] plan invalidation vs stored procedures
On Wed, Aug 6, 2008 at 3:29 PM, Florian Pflug <[EMAIL PROTECTED]> wrote: > Merlin Moncure wrote: >> >> you missed the point...if your return type is a composite type that is >> backed by the table (CREATE TABLE, not CREATE TYPE), then you can >> 'alter' the type by altering the table. This can be done without full >> drop recreate of the function. > > Which - at least IMHO - clearly shows that we ought to support > ALTER TYPE for composite types ;-) > > Is there anything fundamental standing in the way of that, or is it just > that nobody yet cared enough about this? I look at it from another perspective. I see very little value in 'create type as'...it just creates a table that you can't insert to and can't alter (but I agree). merlin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] plan invalidation vs stored procedures
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Hi, Le 6 août 08 à 20:42, Marko Kreen a écrit : But you missed my point: if you don't have functions backed by table, the DROP+CREATE results in inappropriate behaviour that can be avoided. Just wanted to say I agree with Marko here: it seems we have here a pretty nice (and required) core feature which does not work all of the time. It might be a difficult case to get right, but we certainly are NOT getting it right now. Whatever the workarounds, it would be nice that plan invalidation worked whenever it makes sense, and DROP FUNCTION certainly makes sense. Allowing REPLACE function to change return type (adding an OUT parameter, the TABLE column list, etc) would certainly be a good feature to add, but having existing feature set behave correctly is more important. It seems to qualify the complaint as a bug. Regards, - -- Dimitri Fontaine PostgreSQL DBA, Architecte -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.9 (Darwin) iEYEARECAAYFAkiaAPsACgkQlBXRlnbh1bl3CQCgvtP09HyFgzdqWVEpmGBA4bXy 3YUAoKLCrP+2AGqmctL9IBFmpUUmdssD =b8HX -END PGP SIGNATURE- -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] plan invalidation vs stored procedures
On 8/6/08, Florian Pflug <[EMAIL PROTECTED]> wrote: > Merlin Moncure wrote: > > you missed the point...if your return type is a composite type that is > > backed by the table (CREATE TABLE, not CREATE TYPE), then you can > > 'alter' the type by altering the table. This can be done without full > > drop recreate of the function. > > Which - at least IMHO - clearly shows that we ought to support > ALTER TYPE for composite types ;-) > > Is there anything fundamental standing in the way of that, or is it just > that nobody yet cared enough about this? Yes, that would be really good idea. Although as I mentioned in previous email, even for tables it works by accident, thus we need some invalidation mechanism for both tables and types in PLs. And then maybe even rettype (OUT param) change with CREATE OR REPLACE? -- marko -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] plan invalidation vs stored procedures
Merlin Moncure wrote: you missed the point...if your return type is a composite type that is backed by the table (CREATE TABLE, not CREATE TYPE), then you can 'alter' the type by altering the table. This can be done without full drop recreate of the function. Which - at least IMHO - clearly shows that we ought to support ALTER TYPE for composite types ;-) Is there anything fundamental standing in the way of that, or is it just that nobody yet cared enough about this? regrads, Florian Pflug -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] plan invalidation vs stored procedures
On 8/6/08, Merlin Moncure <[EMAIL PROTECTED]> wrote: > On Wed, Aug 6, 2008 at 2:28 PM, Marko Kreen <[EMAIL PROTECTED]> wrote: > > On 8/6/08, Merlin Moncure <[EMAIL PROTECTED]> wrote: > >> On Wed, Aug 6, 2008 at 2:20 AM, Marko Kreen <[EMAIL PROTECTED]> wrote: > >> > But the main problem is that if the DROP/CREATE happens, the failure > >> > mode is very nasty - you get permanent error on existing backends. > >> > (Main case I'm talking about is functions calling other functions.) > >> > > >> > Some sorta recovery mode would be nice to have, it does not even > >> > need function perfectly. Giving error once and then recover would > >> > be better than requiring manual action from admin. > >> > >> sure -- this a known issue --, but the point is that there are not > >> that many reasons why you have to drop/create a function if you are > >> careful. hiding function prototypes is actually pretty powerful > >> although you have to deal with creating the extra types. > > > > Um. If you are talking about about returning type defined by CREATE TYPE > > then you are wrong as changing type requires DROP+CREATE for both type > > and function. > > you missed the point...if your return type is a composite type that is > backed by the table (CREATE TABLE, not CREATE TYPE), then you can > 'alter' the type by altering the table. This can be done without full > drop recreate of the function. Yes, although I suspect it works by accident and can lead to crash with creative use. (eg. plpgsql 'record' cache, any PL's rettype cache are not invalidated when doing the ALTER TABLE) But you missed my point: if you don't have functions backed by table, the DROP+CREATE results in inappropriate behaviour that can be avoided. -- marko -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] plan invalidation vs stored procedures
Don't you think we try to be careful but still we manage to overlook several times in year something and cause some stupid downtime. On Wed, Aug 6, 2008 at 9:13 PM, Merlin Moncure <[EMAIL PROTECTED]> wrote: > On Wed, Aug 6, 2008 at 2:20 AM, Marko Kreen <[EMAIL PROTECTED]> wrote: > > But the main problem is that if the DROP/CREATE happens, the failure > > mode is very nasty - you get permanent error on existing backends. > > (Main case I'm talking about is functions calling other functions.) > > > > Some sorta recovery mode would be nice to have, it does not even > > need function perfectly. Giving error once and then recover would > > be better than requiring manual action from admin. > > sure -- this a known issue --, but the point is that there are not > that many reasons why you have to drop/create a function if you are > careful. hiding function prototypes is actually pretty powerful > although you have to deal with creating the extra types. > > merlin > > -- > Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) > To make changes to your subscription: > http://www.postgresql.org/mailpref/pgsql-hackers >
Re: [HACKERS] plan invalidation vs stored procedures
On Wed, Aug 6, 2008 at 2:28 PM, Marko Kreen <[EMAIL PROTECTED]> wrote: > On 8/6/08, Merlin Moncure <[EMAIL PROTECTED]> wrote: >> On Wed, Aug 6, 2008 at 2:20 AM, Marko Kreen <[EMAIL PROTECTED]> wrote: >> > But the main problem is that if the DROP/CREATE happens, the failure >> > mode is very nasty - you get permanent error on existing backends. >> > (Main case I'm talking about is functions calling other functions.) >> > >> > Some sorta recovery mode would be nice to have, it does not even >> > need function perfectly. Giving error once and then recover would >> > be better than requiring manual action from admin. >> >> sure -- this a known issue --, but the point is that there are not >> that many reasons why you have to drop/create a function if you are >> careful. hiding function prototypes is actually pretty powerful >> although you have to deal with creating the extra types. > > Um. If you are talking about about returning type defined by CREATE TYPE > then you are wrong as changing type requires DROP+CREATE for both type > and function. you missed the point...if your return type is a composite type that is backed by the table (CREATE TABLE, not CREATE TYPE), then you can 'alter' the type by altering the table. This can be done without full drop recreate of the function. merlin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] plan invalidation vs stored procedures
On 8/6/08, Merlin Moncure <[EMAIL PROTECTED]> wrote: > On Wed, Aug 6, 2008 at 2:20 AM, Marko Kreen <[EMAIL PROTECTED]> wrote: > > But the main problem is that if the DROP/CREATE happens, the failure > > mode is very nasty - you get permanent error on existing backends. > > (Main case I'm talking about is functions calling other functions.) > > > > Some sorta recovery mode would be nice to have, it does not even > > need function perfectly. Giving error once and then recover would > > be better than requiring manual action from admin. > > sure -- this a known issue --, but the point is that there are not > that many reasons why you have to drop/create a function if you are > careful. hiding function prototypes is actually pretty powerful > although you have to deal with creating the extra types. Um. If you are talking about about returning type defined by CREATE TYPE then you are wrong as changing type requires DROP+CREATE for both type and function. Again - the main problem is if you are not careful or really need to do the DROP+CREATE, it will result in permanent errors in all backends. To fix it, admin needs to manually intervene. And this is silly, as we already have all the mechanisms needed to survive the situation gracefully. -- marko -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] plan invalidation vs stored procedures
On Wed, Aug 6, 2008 at 2:20 AM, Marko Kreen <[EMAIL PROTECTED]> wrote: > But the main problem is that if the DROP/CREATE happens, the failure > mode is very nasty - you get permanent error on existing backends. > (Main case I'm talking about is functions calling other functions.) > > Some sorta recovery mode would be nice to have, it does not even > need function perfectly. Giving error once and then recover would > be better than requiring manual action from admin. sure -- this a known issue --, but the point is that there are not that many reasons why you have to drop/create a function if you are careful. hiding function prototypes is actually pretty powerful although you have to deal with creating the extra types. merlin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] plan invalidation vs stored procedures
On Wed, 2008-08-06 at 15:41 +0200, Pavel Stehule wrote: > 2008/8/6 Hannu Krosing <[EMAIL PROTECTED]>: > > On Wed, 2008-08-06 at 12:13 +0200, Pavel Stehule wrote: > >> 2008/8/6 Hannu Krosing <[EMAIL PROTECTED]>: > >> > On Tue, 2008-08-05 at 16:17 +0200, Pavel Stehule wrote: > > .. > >> >> you cannot change header of function. It's same as change C header of > >> >> function without complete recompilation. > >> > > >> > SQL is not C. > >> > > >> > You don't have to recompile the whole SQL database when you add columns > >> > to tables, so why should you need to do it, when you add a column to > >> > table-returning function ? > >> > > >> > >> I thing, it's possible - but it's neccessary completly new dictionary > >> with dependencies (some dependencies are dynamic - polymorphic > >> functions) so it's dificult task. > > > > I think that you can safely err on the side of caution, that is, save > > more dependendcies than actually affected. > > > > > Or you even add dependencies from inside the pl, either at compile/check > > or run time (cached of course), so that you hit the exact right function > > oid and can reuse the function lookup already done. > > > actually functions doesn't see into SQL statements - but I though is > could be hook on new item in plan cache, so there can be some > registration that try to analyze all called functions from plan and > add some info to some buffer. There is lot of some. Some have to write > it :) Actually I think we need a callback in either execute or verify methods so that various pl-s can implement adding the dependency. or maybe not even a callback, pl implementation could just add needed entries to pg_catalog.pg_shdepend when doing direct function calls. And it should inspect prepared plans for a list of called functions. the latter probably needs some help from planner, so that it would be easy to get a list of function oids used by a plan. -- Hannu -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] plan invalidation vs stored procedures
2008/8/6 Hannu Krosing <[EMAIL PROTECTED]>: > On Wed, 2008-08-06 at 12:13 +0200, Pavel Stehule wrote: >> 2008/8/6 Hannu Krosing <[EMAIL PROTECTED]>: >> > On Tue, 2008-08-05 at 16:17 +0200, Pavel Stehule wrote: > .. >> >> you cannot change header of function. It's same as change C header of >> >> function without complete recompilation. >> > >> > SQL is not C. >> > >> > You don't have to recompile the whole SQL database when you add columns >> > to tables, so why should you need to do it, when you add a column to >> > table-returning function ? >> > >> >> I thing, it's possible - but it's neccessary completly new dictionary >> with dependencies (some dependencies are dynamic - polymorphic >> functions) so it's dificult task. > > I think that you can safely err on the side of caution, that is, save > more dependendcies than actually affected. > > Or you even add dependencies from inside the pl, either at compile/check > or run time (cached of course), so that you hit the exact right function > oid and can reuse the function lookup already done. > actually functions doesn't see into SQL statements - but I though is could be hook on new item in plan cache, so there can be some registration that try to analyze all called functions from plan and add some info to some buffer. There is lot of some. Some have to write it :) Pavel > - > Hannu > > > -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] plan invalidation vs stored procedures
On Wed, 2008-08-06 at 12:13 +0200, Pavel Stehule wrote: > 2008/8/6 Hannu Krosing <[EMAIL PROTECTED]>: > > On Tue, 2008-08-05 at 16:17 +0200, Pavel Stehule wrote: .. > >> you cannot change header of function. It's same as change C header of > >> function without complete recompilation. > > > > SQL is not C. > > > > You don't have to recompile the whole SQL database when you add columns > > to tables, so why should you need to do it, when you add a column to > > table-returning function ? > > > > I thing, it's possible - but it's neccessary completly new dictionary > with dependencies (some dependencies are dynamic - polymorphic > functions) so it's dificult task. I think that you can safely err on the side of caution, that is, save more dependendcies than actually affected. Or you even add dependencies from inside the pl, either at compile/check or run time (cached of course), so that you hit the exact right function oid and can reuse the function lookup already done. - Hannu -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] plan invalidation vs stored procedures
2008/8/6 Hannu Krosing <[EMAIL PROTECTED]>: > On Tue, 2008-08-05 at 16:17 +0200, Pavel Stehule wrote: >> 2008/8/5 Asko Oja <[EMAIL PROTECTED]>: >> > postgres=# create or replace function pavel ( i_param text, status OUT int, >> > status_text OUT text ) returns record as $$ select 200::int, 'ok'::text; $$ >> > language sql; >> > CREATE FUNCTION >> > postgres=# create or replace function pavel ( i_param text, status OUT int, >> > status_text OUT text, more_text OUT text ) returns record as $$ select >> > 200::int, 'ok'::text, 'tom'::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. >> > >> > On Tue, Aug 5, 2008 at 5:04 PM, Asko Oja <[EMAIL PROTECTED]> wrote: >> >> >> >> > This is simply a bad, wrong, stupid way to do it. Why do you not use >> >> > CREATE OR REPLACE FUNCTION? >> >> I totally agree we should get this fixed first :) >> >> >> >> postgres=# create or replace function pavel ( 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. >> >> >> >> you cannot change header of function. It's same as change C header of >> function without complete recompilation. > > SQL is not C. > > You don't have to recompile the whole SQL database when you add columns > to tables, so why should you need to do it, when you add a column to > table-returning function ? > I thing, it's possible - but it's neccessary completly new dictionary with dependencies (some dependencies are dynamic - polymorphic functions) so it's dificult task. Pavel > -- > Hannu > > > -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] plan invalidation vs stored procedures
On Tue, 2008-08-05 at 16:17 +0200, Pavel Stehule wrote: > 2008/8/5 Asko Oja <[EMAIL PROTECTED]>: > > postgres=# create or replace function pavel ( i_param text, status OUT int, > > status_text OUT text ) returns record as $$ select 200::int, 'ok'::text; $$ > > language sql; > > CREATE FUNCTION > > postgres=# create or replace function pavel ( i_param text, status OUT int, > > status_text OUT text, more_text OUT text ) returns record as $$ select > > 200::int, 'ok'::text, 'tom'::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. > > > > On Tue, Aug 5, 2008 at 5:04 PM, Asko Oja <[EMAIL PROTECTED]> wrote: > >> > >> > This is simply a bad, wrong, stupid way to do it. Why do you not use > >> > CREATE OR REPLACE FUNCTION? > >> I totally agree we should get this fixed first :) > >> > >> postgres=# create or replace function pavel ( 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. > >> > > you cannot change header of function. It's same as change C header of > function without complete recompilation. SQL is not C. You don't have to recompile the whole SQL database when you add columns to tables, so why should you need to do it, when you add a column to table-returning function ? -- Hannu -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] plan invalidation vs stored procedures
On Tue, 2008-08-05 at 16:16 +0200, Pavel Stehule wrote: > 2008/8/5 Martin Pihlak <[EMAIL PROTECTED]>: > >>> DROP FUNCTION > >>> create function foo() returns integer as $$ begin return 2; end; $$ > >>> language plpgsql; > >>> CREATE FUNCTION > >>> execute c1; > >>> psql:test.sql:11: ERROR: cache lookup failed for function 36555 > >> > >> This is simply a bad, wrong, stupid way to do it. Why do you not use > >> CREATE OR REPLACE FUNCTION? > >> > > > > Well, the test case was an illustration. The actual reason for DROP and > > CREATE is > > the inability to change function return type. In our case there are plpgsql > > OUT > > parameters involved, and there is no other way to add additional OUT > > parameters > > without dropping the function first. I'd be glad if this was fixed, but I > > still > > think that proper plan invalidation for function changes is needed (inlined > > functions, ALTER FUNCTION stuff etc.) > > It isn't possible. Probably some wrong is in your database design. Yup. It is called evolving a design. Them stupid people did not design all their possible future uses of functions properly at first try. I'm sure that it is possible to work around postgreSQL's inability to properly invalidate plans by treating SQL as C++, where every change needs a complete recompile & restart, but that enforces an unneccessary cost in downtime. - Hannu -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] plan invalidation vs stored procedures
On 8/5/08, Merlin Moncure <[EMAIL PROTECTED]> wrote: > On Tue, Aug 5, 2008 at 10:12 AM, Martin Pihlak <[EMAIL PROTECTED]> wrote: > >>> DROP FUNCTION > >>> create function foo() returns integer as $$ begin return 2; end; $$ > language plpgsql; > >>> CREATE FUNCTION > >>> execute c1; > >>> psql:test.sql:11: ERROR: cache lookup failed for function 36555 > >> > >> This is simply a bad, wrong, stupid way to do it. Why do you not use > >> CREATE OR REPLACE FUNCTION? > >> > > > > Well, the test case was an illustration. The actual reason for DROP and > CREATE is > > the inability to change function return type. In our case there are > plpgsql OUT > > parameters involved, and there is no other way to add additional OUT > parameters > > without dropping the function first. I'd be glad if this was fixed, but I > still > > think that proper plan invalidation for function changes is needed (inlined > > functions, ALTER FUNCTION stuff etc.) > > > one workaround is to use a table based custom composite type: > > create table foo_output(a int, b text); > > create function foo() returns foo_output as ... > > alter table foo_output add column c int; > > create or replace foo() if necessary. This also works for 'in' variables. > > voila! :-) note you can't use standard composite type because there > is no way to 'alter' it. Yes. Or require always new name for function. But the main problem is that if the DROP/CREATE happens, the failure mode is very nasty - you get permanent error on existing backends. (Main case I'm talking about is functions calling other functions.) Some sorta recovery mode would be nice to have, it does not even need function perfectly. Giving error once and then recover would be better than requiring manual action from admin. -- marko -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] plan invalidation vs stored procedures
On Tue, Aug 5, 2008 at 10:12 AM, Martin Pihlak <[EMAIL PROTECTED]> wrote: >>> DROP FUNCTION >>> create function foo() returns integer as $$ begin return 2; end; $$ >>> language plpgsql; >>> CREATE FUNCTION >>> execute c1; >>> psql:test.sql:11: ERROR: cache lookup failed for function 36555 >> >> This is simply a bad, wrong, stupid way to do it. Why do you not use >> CREATE OR REPLACE FUNCTION? >> > > Well, the test case was an illustration. The actual reason for DROP and > CREATE is > the inability to change function return type. In our case there are plpgsql > OUT > parameters involved, and there is no other way to add additional OUT > parameters > without dropping the function first. I'd be glad if this was fixed, but I > still > think that proper plan invalidation for function changes is needed (inlined > functions, ALTER FUNCTION stuff etc.) one workaround is to use a table based custom composite type: create table foo_output(a int, b text); create function foo() returns foo_output as ... alter table foo_output add column c int; create or replace foo() if necessary. This also works for 'in' variables. voila! :-) note you can't use standard composite type because there is no way to 'alter' it. merlin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] plan invalidation vs stored procedures
On 8/5/08, Pavel Stehule <[EMAIL PROTECTED]> wrote: > >> ERROR: cannot change return type of existing function > >> HINT: Use DROP FUNCTION first. > > you cannot change header of function. It's same as change C header of > function without complete recompilation. Thats why plan invalidation for DROP+CREATE is needed. -- marko -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] plan invalidation vs stored procedures
2008/8/5 Asko Oja <[EMAIL PROTECTED]>: > postgres=# create or replace function pavel ( i_param text, status OUT int, > status_text OUT text ) returns record as $$ select 200::int, 'ok'::text; $$ > language sql; > CREATE FUNCTION > postgres=# create or replace function pavel ( i_param text, status OUT int, > status_text OUT text, more_text OUT text ) returns record as $$ select > 200::int, 'ok'::text, 'tom'::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. > > On Tue, Aug 5, 2008 at 5:04 PM, Asko Oja <[EMAIL PROTECTED]> wrote: >> >> > This is simply a bad, wrong, stupid way to do it. Why do you not use >> > CREATE OR REPLACE FUNCTION? >> I totally agree we should get this fixed first :) >> >> postgres=# create or replace function pavel ( 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. >> you cannot change header of function. It's same as change C header of function without complete recompilation. >> On Tue, Aug 5, 2008 at 4:51 PM, Tom Lane <[EMAIL PROTECTED]> wrote: >>> >>> Martin Pihlak <[EMAIL PROTECTED]> writes: >>> > create function foo() returns integer as $$ begin return 1; end; $$ >>> > language plpgsql; >>> > CREATE FUNCTION >>> > prepare c1 as select * from foo(); >>> > PREPARE >>> > execute c1; >>> > foo >>> > - >>> >1 >>> > (1 row) >>> >>> > drop function foo(); >>> > DROP FUNCTION >>> > create function foo() returns integer as $$ begin return 2; end; $$ >>> > language plpgsql; >>> > CREATE FUNCTION >>> > execute c1; >>> > psql:test.sql:11: ERROR: cache lookup failed for function 36555 >>> >>> This is simply a bad, wrong, stupid way to do it. Why do you not use >>> CREATE OR REPLACE FUNCTION? >>> >>>regards, tom lane >>> >>> -- >>> Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) >>> To make changes to your subscription: >>> http://www.postgresql.org/mailpref/pgsql-hackers >> > > -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] plan invalidation vs stored procedures
2008/8/5 Martin Pihlak <[EMAIL PROTECTED]>: >>> DROP FUNCTION >>> create function foo() returns integer as $$ begin return 2; end; $$ >>> language plpgsql; >>> CREATE FUNCTION >>> execute c1; >>> psql:test.sql:11: ERROR: cache lookup failed for function 36555 >> >> This is simply a bad, wrong, stupid way to do it. Why do you not use >> CREATE OR REPLACE FUNCTION? >> > > Well, the test case was an illustration. The actual reason for DROP and > CREATE is > the inability to change function return type. In our case there are plpgsql > OUT > parameters involved, and there is no other way to add additional OUT > parameters > without dropping the function first. I'd be glad if this was fixed, but I > still > think that proper plan invalidation for function changes is needed (inlined > functions, ALTER FUNCTION stuff etc.) It isn't possible. Probably some wrong is in your database design. regards Pavel Stehule > > regards, > Martin > > -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] plan invalidation vs stored procedures
>> DROP FUNCTION >> create function foo() returns integer as $$ begin return 2; end; $$ language >> plpgsql; >> CREATE FUNCTION >> execute c1; >> psql:test.sql:11: ERROR: cache lookup failed for function 36555 > > This is simply a bad, wrong, stupid way to do it. Why do you not use > CREATE OR REPLACE FUNCTION? > Well, the test case was an illustration. The actual reason for DROP and CREATE is the inability to change function return type. In our case there are plpgsql OUT parameters involved, and there is no other way to add additional OUT parameters without dropping the function first. I'd be glad if this was fixed, but I still think that proper plan invalidation for function changes is needed (inlined functions, ALTER FUNCTION stuff etc.) regards, Martin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] plan invalidation vs stored procedures
postgres=# create or replace function pavel ( i_param text, status OUT int, status_text OUT text ) returns record as $$ select 200::int, 'ok'::text; $$ language sql; CREATE FUNCTION postgres=# create or replace function pavel ( i_param text, status OUT int, status_text OUT text, more_text OUT text ) returns record as $$ select 200::int, 'ok'::text, 'tom'::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. On Tue, Aug 5, 2008 at 5:04 PM, Asko Oja <[EMAIL PROTECTED]> wrote: > > This is simply a bad, wrong, stupid way to do it. Why do you not use > > CREATE OR REPLACE FUNCTION? > I totally agree we should get this fixed first :) > > postgres=# create or replace function pavel ( 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. > > On Tue, Aug 5, 2008 at 4:51 PM, Tom Lane <[EMAIL PROTECTED]> wrote: > >> Martin Pihlak <[EMAIL PROTECTED]> writes: >> > create function foo() returns integer as $$ begin return 1; end; $$ >> language plpgsql; >> > CREATE FUNCTION >> > prepare c1 as select * from foo(); >> > PREPARE >> > execute c1; >> > foo >> > - >> >1 >> > (1 row) >> >> > drop function foo(); >> > DROP FUNCTION >> > create function foo() returns integer as $$ begin return 2; end; $$ >> language plpgsql; >> > CREATE FUNCTION >> > execute c1; >> > psql:test.sql:11: ERROR: cache lookup failed for function 36555 >> >> This is simply a bad, wrong, stupid way to do it. Why do you not use >> CREATE OR REPLACE FUNCTION? >> >>regards, tom lane >> >> -- >> Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) >> To make changes to your subscription: >> http://www.postgresql.org/mailpref/pgsql-hackers >> > >
Re: [HACKERS] plan invalidation vs stored procedures
> This is simply a bad, wrong, stupid way to do it. Why do you not use > CREATE OR REPLACE FUNCTION? I totally agree we should get this fixed first :) postgres=# create or replace function pavel ( 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. On Tue, Aug 5, 2008 at 4:51 PM, Tom Lane <[EMAIL PROTECTED]> wrote: > Martin Pihlak <[EMAIL PROTECTED]> writes: > > create function foo() returns integer as $$ begin return 1; end; $$ > language plpgsql; > > CREATE FUNCTION > > prepare c1 as select * from foo(); > > PREPARE > > execute c1; > > foo > > - > >1 > > (1 row) > > > drop function foo(); > > DROP FUNCTION > > create function foo() returns integer as $$ begin return 2; end; $$ > language plpgsql; > > CREATE FUNCTION > > execute c1; > > psql:test.sql:11: ERROR: cache lookup failed for function 36555 > > This is simply a bad, wrong, stupid way to do it. Why do you not use > CREATE OR REPLACE FUNCTION? > >regards, tom lane > > -- > Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) > To make changes to your subscription: > http://www.postgresql.org/mailpref/pgsql-hackers >
Re: [HACKERS] plan invalidation vs stored procedures
Martin Pihlak <[EMAIL PROTECTED]> writes: > create function foo() returns integer as $$ begin return 1; end; $$ language > plpgsql; > CREATE FUNCTION > prepare c1 as select * from foo(); > PREPARE > execute c1; > foo > - >1 > (1 row) > drop function foo(); > DROP FUNCTION > create function foo() returns integer as $$ begin return 2; end; $$ language > plpgsql; > CREATE FUNCTION > execute c1; > psql:test.sql:11: ERROR: cache lookup failed for function 36555 This is simply a bad, wrong, stupid way to do it. Why do you not use CREATE OR REPLACE FUNCTION? regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] plan invalidation vs stored procedures
Hi Thanks for pointing to another thing to fix :) postgres=# create type public.ret_status as ( status integer, status_text text); CREATE TYPE postgres=# create or replace function pavel ( i_param text ) returns public.ret_status as $$ select 200::int, 'ok'::text; $$ language sql; CREATE FUNCTION postgres=# create or replace function pavel ( 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. Asko On Tue, Aug 5, 2008 at 4:00 PM, Pavel Stehule <[EMAIL PROTECTED]>wrote: > 2008/8/5 Martin Pihlak <[EMAIL PROTECTED]>: > > Pavel Stehule wrote: > >> Hello > >> > >> try version 8.3. There lot of dependencies are solved. > >> > > > > Yes, 8.3 was the version I was testing with. Same results on the HEAD: > > > > $ psql -e -f test.sql > > select version(); > > version > > > > > -- > > PostgreSQL 8.4devel on i686-pc-linux-gnu, compiled by GCC gcc (GCC) > 4.1.3 20070929 (prerelease) > > (Ubuntu 4.1.2-16ubuntu2) > > (1 row) > > > > create function foo() returns integer as $$ begin return 1; end; $$ > language plpgsql; > > CREATE FUNCTION > > prepare c1 as select * from foo(); > > PREPARE > > execute c1; > > foo > > - > > 1 > > (1 row) > > > > drop function foo(); > > DROP FUNCTION > > create function foo() returns integer as $$ begin return 2; end; $$ > language plpgsql; > > CREATE FUNCTION > > execute c1; > > psql:test.sql:11: ERROR: cache lookup failed for function 36555 > > > > regards, > > Martin > > > > use CREATE OR REPLACE FUNCTION syntax without DROP FUNCTION, CREATE > FUNCTION .. > > Regards > Pavel Stehule > > -- > Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) > To make changes to your subscription: > http://www.postgresql.org/mailpref/pgsql-hackers >
Re: [HACKERS] plan invalidation vs stored procedures
2008/8/5 Martin Pihlak <[EMAIL PROTECTED]>: > Pavel Stehule wrote: >> Hello >> >> try version 8.3. There lot of dependencies are solved. >> > > Yes, 8.3 was the version I was testing with. Same results on the HEAD: > > $ psql -e -f test.sql > select version(); > version > > -- > PostgreSQL 8.4devel on i686-pc-linux-gnu, compiled by GCC gcc (GCC) 4.1.3 > 20070929 (prerelease) > (Ubuntu 4.1.2-16ubuntu2) > (1 row) > > create function foo() returns integer as $$ begin return 1; end; $$ language > plpgsql; > CREATE FUNCTION > prepare c1 as select * from foo(); > PREPARE > execute c1; > foo > - > 1 > (1 row) > > drop function foo(); > DROP FUNCTION > create function foo() returns integer as $$ begin return 2; end; $$ language > plpgsql; > CREATE FUNCTION > execute c1; > psql:test.sql:11: ERROR: cache lookup failed for function 36555 > > regards, > Martin > use CREATE OR REPLACE FUNCTION syntax without DROP FUNCTION, CREATE FUNCTION .. Regards Pavel Stehule -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] plan invalidation vs stored procedures
Hi Sadly PostgreSQL inability to invalidate plan cache when function is dropped causes us downtime and costs money. ERROR: cache lookup failed for function 24865) This time our developers just rewrote function to use OUT parameters instead of return type. Currently i had to forbid dropping functions in our most critical databases but that makes developers unhappy. And as i understand it is not fixed in 8.3: Comment from code * Currently, we use only relcache invalidation events to invalidate plans. * This means that changes such as modification of a function definition do * not invalidate plans using the function. This is not 100% OK --- for * example, changing a SQL function that's been inlined really ought to * cause invalidation of the plan that it's been inlined into --- but the * cost of tracking additional types of object seems much higher than the * gain, so we're just ignoring them for now. So we will have to get it fixed and better would be to do it so that solution suits everybody. Our current workaround include updating pg_proc after release or letting pgBouncer to reconnect all connections but neither solution is good and cause us to lose valuable minutes in error flood when we miss some crucial drop function. Asko On Tue, Aug 5, 2008 at 1:40 PM, Pavel Stehule <[EMAIL PROTECTED]>wrote: > Hello > > try version 8.3. There lot of dependencies are solved. > > Regards > Pavel Stehule > > 2008/8/5 Martin Pihlak <[EMAIL PROTECTED]>: > > Howdy, > > > > What is the status of plan invalidation vs stored procedures? From > > the initial design discussion I understand that function change handling > > was postponed to "some time in the future". Is anybody already working > > on that or maybe some ideas of how to implement this? > > > > The business case for the feature is that most of our db logic is inside > > stored procedures and hence use cached plans. Every time a function is > > dropped and recreated we get a storm of "cache lookup failed" errors. > > If we are lucky, the DBA will detect it and apply appropriate > workarounds. > > If not ... things get messy. > > > > We are considering of hacking up a proprietary solution to address our > > specific problems (e.g. invalidate every plan on pg_proc changes). But I > > think that this is something that would be useful to a wider audience and > > deserves a more general solution. How about it? > > > > regards, > > Martin > > > > > > -- > > Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) > > To make changes to your subscription: > > http://www.postgresql.org/mailpref/pgsql-hackers > > > > -- > Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) > To make changes to your subscription: > http://www.postgresql.org/mailpref/pgsql-hackers >
Re: [HACKERS] plan invalidation vs stored procedures
Pavel Stehule wrote: > Hello > > try version 8.3. There lot of dependencies are solved. > Yes, 8.3 was the version I was testing with. Same results on the HEAD: $ psql -e -f test.sql select version(); version -- PostgreSQL 8.4devel on i686-pc-linux-gnu, compiled by GCC gcc (GCC) 4.1.3 20070929 (prerelease) (Ubuntu 4.1.2-16ubuntu2) (1 row) create function foo() returns integer as $$ begin return 1; end; $$ language plpgsql; CREATE FUNCTION prepare c1 as select * from foo(); PREPARE execute c1; foo - 1 (1 row) drop function foo(); DROP FUNCTION create function foo() returns integer as $$ begin return 2; end; $$ language plpgsql; CREATE FUNCTION execute c1; psql:test.sql:11: ERROR: cache lookup failed for function 36555 regards, Martin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] plan invalidation vs stored procedures
Hello try version 8.3. There lot of dependencies are solved. Regards Pavel Stehule 2008/8/5 Martin Pihlak <[EMAIL PROTECTED]>: > Howdy, > > What is the status of plan invalidation vs stored procedures? From > the initial design discussion I understand that function change handling > was postponed to "some time in the future". Is anybody already working > on that or maybe some ideas of how to implement this? > > The business case for the feature is that most of our db logic is inside > stored procedures and hence use cached plans. Every time a function is > dropped and recreated we get a storm of "cache lookup failed" errors. > If we are lucky, the DBA will detect it and apply appropriate workarounds. > If not ... things get messy. > > We are considering of hacking up a proprietary solution to address our > specific problems (e.g. invalidate every plan on pg_proc changes). But I > think that this is something that would be useful to a wider audience and > deserves a more general solution. How about it? > > regards, > Martin > > > -- > Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) > To make changes to your subscription: > http://www.postgresql.org/mailpref/pgsql-hackers > -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] plan invalidation vs stored procedures
Howdy, What is the status of plan invalidation vs stored procedures? From the initial design discussion I understand that function change handling was postponed to "some time in the future". Is anybody already working on that or maybe some ideas of how to implement this? The business case for the feature is that most of our db logic is inside stored procedures and hence use cached plans. Every time a function is dropped and recreated we get a storm of "cache lookup failed" errors. If we are lucky, the DBA will detect it and apply appropriate workarounds. If not ... things get messy. We are considering of hacking up a proprietary solution to address our specific problems (e.g. invalidate every plan on pg_proc changes). But I think that this is something that would be useful to a wider audience and deserves a more general solution. How about it? regards, Martin -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
Re: [HACKERS] Plan invalidation vs temp sequences
Gregory Stark wrote:
"Tom Lane" <[EMAIL PROTECTED]> writes:
There doesn't seem to be any very nice way to fix this. There is
not any existing support mechanism (comparable to query_tree_walker)
for scanning whole plan trees, which means that searching a cached plan
for regclass Consts is going to involve a chunk of new code no matter
how we approach it. We might want to do that someday --- in particular,
if we ever try to extend the plan inval mechanism to react to
redefinitions of non-table objects, we'd likely need some such thing
anyway. I'm disinclined to try to do it for 8.3 though. The use-case
for temp sequences seems a bit narrow and there are several workarounds
(see followups to bug report), so I'm feeling this is a
fix-some-other-day kind of issue.
Given that sequences are in fact relations is there some way to work around
the issue at least in this case by stuffing the sequence's relid someplace
which the plan invalldation code can check for it?
Hm... couldn't this be worked around by doing
create or replace function dynamic_oid(text) returning regclass as
'select $1::regclass' language 'pl/pgsql' stable;
And then writing
nextval(dynamic_oid('mysequence')).
I didn't test this, but it it actually works, maybe we should just stick this
into the docs somewhere. It's probably too late to add that function to the
backend, though...
As long as mysequence is really a temporary sequence, this wont even have
searchpath issues I think, because those are always on top of the searchpatch
anyway, aren't they?
greetings, Florian Pflug
---(end of broadcast)---
TIP 1: if posting/reading through Usenet, please send an appropriate
subscribe-nomail command to [EMAIL PROTECTED] so that your
message can get through to the mailing list cleanly
---(end of broadcast)---
TIP 5: don't forget to increase your free space map settings
Re: [HACKERS] Plan invalidation vs temp sequences
I wrote: > Well, we *have* the sequence's Oid in the regclass constant, the problem > is the difficulty of digging through the plan tree to find it. I did > consider having the planner extract it and save it aside somewhere, but > there doesn't seem to be any very convenient place to do that, short of > an extra traversal of the query tree, which is pretty annoying/expensive > for data that will probably never be needed for most queries. Actually ... now that I've consumed a bit more caffeine, it seems this could be done relatively cheaply in setrefs.c. We could add a list-of-relation-OIDs to PlannedStmt, and charge setrefs.c with creating the list, and simplify plancache.c to just use list_member_oid() instead of groveling over the rangetable for itself. I'll go try that out. regards, tom lane ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [HACKERS] Plan invalidation vs temp sequences
"Florian G. Pflug" <[EMAIL PROTECTED]> writes:
> Gregory Stark wrote:
>> Given that sequences are in fact relations is there some way to work around
>> the issue at least in this case by stuffing the sequence's relid someplace
>> which the plan invalldation code can check for it?
Well, we *have* the sequence's Oid in the regclass constant, the problem
is the difficulty of digging through the plan tree to find it. I did
consider having the planner extract it and save it aside somewhere, but
there doesn't seem to be any very convenient place to do that, short of
an extra traversal of the query tree, which is pretty annoying/expensive
for data that will probably never be needed for most queries.
> Hm... couldn't this be worked around by doing
> create or replace function dynamic_oid(text) returning regclass as
> 'select $1::regclass' language 'pl/pgsql' stable;
> And then writing
> nextval(dynamic_oid('mysequence')).
The cast-to-text workaround that I suggested does exactly that.
regards, tom lane
---(end of broadcast)---
TIP 2: Don't 'kill -9' the postmaster
Re: [HACKERS] Plan invalidation vs temp sequences
Gregory Stark wrote:
"Tom Lane" <[EMAIL PROTECTED]> writes:
There doesn't seem to be any very nice way to fix this. There is
not any existing support mechanism (comparable to query_tree_walker)
for scanning whole plan trees, which means that searching a cached plan
for regclass Consts is going to involve a chunk of new code no matter
how we approach it. We might want to do that someday --- in particular,
if we ever try to extend the plan inval mechanism to react to
redefinitions of non-table objects, we'd likely need some such thing
anyway. I'm disinclined to try to do it for 8.3 though. The use-case
for temp sequences seems a bit narrow and there are several workarounds
(see followups to bug report), so I'm feeling this is a
fix-some-other-day kind of issue.
Given that sequences are in fact relations is there some way to work around
the issue at least in this case by stuffing the sequence's relid someplace
which the plan invalldation code can check for it?
Hm... couldn't this be worked around by doing
create or replace function dynamic_oid(text) returning regclass as
'select $1::regclass' language 'pl/pgsql' stable;
And then writing
nextval(dynamic_oid('mysequence')).
I didn't test this, but it it actually works, maybe we should just stick this
into the docs somewhere. It's probably too late to add that function to the
backend, though...
As long as mysequence is really a temporary sequence, this wont even have
searchpath issues I think, because those are always on top of the searchpatch
anyway, aren't they?
greetings, Florian Pflug
---(end of broadcast)---
TIP 1: if posting/reading through Usenet, please send an appropriate
subscribe-nomail command to [EMAIL PROTECTED] so that your
message can get through to the mailing list cleanly
Re: [HACKERS] Plan invalidation vs temp sequences
"Tom Lane" <[EMAIL PROTECTED]> writes: > There doesn't seem to be any very nice way to fix this. There is > not any existing support mechanism (comparable to query_tree_walker) > for scanning whole plan trees, which means that searching a cached plan > for regclass Consts is going to involve a chunk of new code no matter > how we approach it. We might want to do that someday --- in particular, > if we ever try to extend the plan inval mechanism to react to > redefinitions of non-table objects, we'd likely need some such thing > anyway. I'm disinclined to try to do it for 8.3 though. The use-case > for temp sequences seems a bit narrow and there are several workarounds > (see followups to bug report), so I'm feeling this is a > fix-some-other-day kind of issue. Given that sequences are in fact relations is there some way to work around the issue at least in this case by stuffing the sequence's relid someplace which the plan invalldation code can check for it? -- Gregory Stark EnterpriseDB http://www.enterprisedb.com ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [HACKERS] Plan invalidation vs temp sequences
Tom Lane wrote:
> ... We might want to do that someday --- in particular,
> if we ever try to extend the plan inval mechanism to react to
> redefinitions of non-table objects, we'd likely need some such thing
> anyway. I'm disinclined to try to do it for 8.3 though. The use-case
> for temp sequences seems a bit narrow and there are several workarounds
> (see followups to bug report), so I'm feeling this is a
> fix-some-other-day kind of issue.
Agreed. I was a bit worried about this kind of usage:
CREATE OR REPLACE FUNCTION testfunc(val int) RETURNS int AS $$
DECLARE
BEGIN
CREATE TEMPORARY SEQUENCE tempseq;
CREATE TEMPORARY TABLE inttable (key integer DEFAULT
nextval('tempseq'), data text);
INSERT INTO inttable (data) VALUES ('foo');
DROP TABLE inttable;
DROP SEQUENCE tempseq;
return 1;
END;
$$ LANGUAGE plpgsql;
but that seems to work, because creating/dropping the temp table
triggers the plan invalidation.
--
Heikki Linnakangas
EnterpriseDB http://www.enterprisedb.com
---(end of broadcast)---
TIP 9: In versions below 8.0, the planner will ignore your desire to
choose an index scan if your joining column's datatypes do not
match
[HACKERS] Plan invalidation vs temp sequences
In bug #3662
http://archives.postgresql.org/pgsql-bugs/2007-10/msg00047.php
we see that it doesn't work to do nextval('seq') on a temp sequence
in a plpgsql function except via EXECUTE, because the sequence OID gets
embedded into the cached plan, same as any other temp table. This is to
be expected in existing releases, but I notice it still doesn't work in
CVS HEAD, which is a tad annoying given the existence of the plan inval
mechanism. The reason is that plancache.c only searches the plan's
RangeTable for references to a just-invalidated relation; and the
argument of nextval() isn't mentioned in the RangeTable (it is in fact
just a Const of type regclass).
There doesn't seem to be any very nice way to fix this. There is
not any existing support mechanism (comparable to query_tree_walker)
for scanning whole plan trees, which means that searching a cached plan
for regclass Consts is going to involve a chunk of new code no matter
how we approach it. We might want to do that someday --- in particular,
if we ever try to extend the plan inval mechanism to react to
redefinitions of non-table objects, we'd likely need some such thing
anyway. I'm disinclined to try to do it for 8.3 though. The use-case
for temp sequences seems a bit narrow and there are several workarounds
(see followups to bug report), so I'm feeling this is a
fix-some-other-day kind of issue.
Comments?
regards, tom lane
---(end of broadcast)---
TIP 9: In versions below 8.0, the planner will ignore your desire to
choose an index scan if your joining column's datatypes do not
match
Re: [HACKERS] Plan invalidation
"Pavan Deolasee" <[EMAIL PROTECTED]> writes: > On 4/3/07, Tom Lane <[EMAIL PROTECTED]> wrote: >> If the invalidation were something that *had* to be accounted for, >> such as a dropped index, then there should be adequate locking for it; >> plancache is not introducing any new bug that wasn't there before. >> > Oh yes, I was wondering about the other parts of the code, not > plan invalidation. Never mind, it was just a thought. Well, as that comment notes, we've always had to worry about being sure that the relcache data structures are up-to-date (or sufficiently up-to-date, anyway). I think it's reasonably well debugged. regards, tom lane ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [HACKERS] Plan invalidation
On 4/3/07, Tom Lane <[EMAIL PROTECTED]> wrote: I'm not particularly worried about missing a potential improvement in the plan during the first command after a change is committed. Me too. Just noticed it, so brought it up. If the invalidation were something that *had* to be accounted for, such as a dropped index, then there should be adequate locking for it; plancache is not introducing any new bug that wasn't there before. Oh yes, I was wondering about the other parts of the code, not plan invalidation. Never mind, it was just a thought. Thanks, Pavan -- EnterpriseDB http://www.enterprisedb.com
Re: [HACKERS] Plan invalidation
"Pavan Deolasee" <[EMAIL PROTECTED]> writes: > I traced it a bit and it seems that the invalidation messages > are not accepted in session 2 because the locks are already held > on the relation. Right, because of this coding in LockRelationOid(): /* * Now that we have the lock, check for invalidation messages, so that we * will update or flush any stale relcache entry before we try to use it. * We can skip this in the not-uncommon case that we already had the same * type of lock being requested, since then no one else could have * modified the relcache entry in an undesirable way. (In the case where * our own xact modifies the rel, the relcache update happens via * CommandCounterIncrement, not here.) */ if (res != LOCKACQUIRE_ALREADY_HELD) AcceptInvalidationMessages(); We could remove the optimization and do AcceptInvalidationMessages always, but I think that cure would be a great deal worse than the disease --- it would hugely increase the contention for SInvalLock. I'm not particularly worried about missing a potential improvement in the plan during the first command after a change is committed. If the invalidation were something that *had* to be accounted for, such as a dropped index, then there should be adequate locking for it; plancache is not introducing any new bug that wasn't there before. regards, tom lane ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
[HACKERS] Plan invalidation
I noticed that the plan invalidation is not immediately effective. Not sure whether it's worth fixing or has any other side-effects, but thought would just post it. I was testing the following scenario: session1session2 CREATE TABLE test (int a, int b); INSERT INTO TABLE SET enable_seqscan = off BEGIN PREPARE myplan AS SELECT * FROM TEST WHERE a = 100; EXPLAIN EXECUTE myplan; (seq scan) CREATE INDEX -> EXPLAIN EXECUTE myplan (seq scan) EXPLAIN EXECUTE myplan (index scan) The second "EXPLAIN" in session 2 uses seq scan because the plan is not invalidated and replanned properly. Ideally it should have used the index scan. I traced it a bit and it seems that the invalidation messages are not accepted in session 2 because the locks are already held on the relation. At the end of the command, session 2 calls CommandCounterIncrement() and gets the invalidation messages. Hence the next EXPLAIN revalidates the plan properly. May be this is not such an important issue. But I was wondering if there are other places in the code where we might miss or receive invalidation messages with a delay, mostly because of *lack* of lock conflict ? Thanks, Pavan -- EnterpriseDBhttp://www.enterprisedb.com ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [JDBC] [HACKERS] Plan invalidation vs. unnamed prepared statements
I am from pgsql-jdbc, so I may not be "in the thread", so please ignore places where my misunderstanding goes out. The main two questions, IMHO, is: 1) What is the key to plan cache. Current option is some statement key (id). Another option would be statement text (you still need to store it if you want to replan at some point). In this case you can use same plan for multiple statements going from different sessions. That's the point Simon was talking about. This should significantly reduce planning, especially on multiple similar clients. Now, as I understand, every connection prepare same statements and plan then independent. Such change would make Application servers prepare new connections much faster (given they prepare a number of same statements for each connection, which is the case for my engine). This should work for both named and unnamed. Note that adding unnamed statements to cache (and not removing on statement disposal) may require much larger cache. BTW: This is used by IBM DB2 UDB. 2) Specific plans when parameters are known. This is the point about using partial index(and sometimes even using full index- i.e. specifying frequent value of some index or one of two tables in a join). I'd say the best would be to have generic plan and try to replan, starting from generic plan results (dispose any possibility that gives values worse then generic plan). Such a replan should be much faster then original planning because you have rather high starting point. Another option is to catch possibilities at original planning and select correct plan when parameters are known - you check all possible uses with "this will be frequent value, this will match this partial index, ..." the question is the number of such plans. But since all of them must be better then generic (and it is possible to make a three, i.e. "A and B are not frequent" -> "A is frequent" -> "A is frequent and B meets partial index" and children must be better then parent), I'd say there won't be many (and you can always limit it's number and leave only the best if one goes out of number or even collect usages and leave the plans that are used). ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [HACKERS] Plan invalidation vs. unnamed prepared statements
"Simon Riggs" <[EMAIL PROTECTED]> writes: > On Tue, 2007-03-06 at 12:22 -0500, Tom Lane wrote: >> A. Just accept the extra overhead, thereby preserving the current >> behavior of unnamed statements, and gaining the benefit that plan >> invalidation will work correctly in the few cases where an unnamed >> statement's plan lasts long enough to need replanning. > With connection pooling, multiple sessions will execute each statement. > If we check the cache each time this does seem more expensive for each > individual session, but we should gain synergy from other similar > sessions. It seems fairly unlikely to me that client code would try to share an unnamed statement across multiple application threads; the entire point is that it's for one-off queries. Or did you miss the point that the plan cache is local per-backend? > ISTM there will be some cases where the current behaviour will not be > maintained if we implement A exactly. One thing I've not seen mentioned > is the effect of constants on various plans. There is none. regards, tom lane ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [HACKERS] Plan invalidation vs. unnamed prepared statements
On Tue, 2007-03-06 at 12:22 -0500, Tom Lane wrote: > A. Just accept the extra overhead, thereby preserving the current > behavior of unnamed statements, and gaining the benefit that plan > invalidation will work correctly in the few cases where an unnamed > statement's plan lasts long enough to need replanning. With connection pooling, multiple sessions will execute each statement. If we check the cache each time this does seem more expensive for each individual session, but we should gain synergy from other similar sessions. Taken across multiple sessions, A will be a win because it will reduce planning overhead by ~99%. > C. Don't store the unnamed statement in the plan cache. To make sure > it's not used anymore when the plan might be stale, don't analyze or > plan at Parse-message time, but postpone *all* that work until Bind; > and always discard the plan after Execute. We could still do "raw > parsing" at Parse time, since that's independent of database state, > but all but the most trivial syntactic errors would now occur at Bind > not Parse time, as well as the majority of the time expenditure. ISTM there will be some cases where the current behaviour will not be maintained if we implement A exactly. One thing I've not seen mentioned is the effect of constants on various plans. The current system plans at Bind time so it can make longer term decisions based upon the values of initial parameters. So I'd say we need to check the cache at Parse time, but if we do need to plan, continue to do this at Bind time (and so don't write to plan cache until that point). That might mean we end up giving some of our benefit away if multiple sessions all concurrently plan a previously unplanned query. That does seem less likely and in any case much better than taking a step backwards in query planning of parameterised queries. Also, some of those plans are only currently possible with actual constants, specifically predicate proving for partial indexes and constraint exclusion. Parameter to constant folding may change the plan completely and make it non-reusable anyhow. How would we cope with that type of prepared query with plan inval? -- Simon Riggs EnterpriseDB http://www.enterprisedb.com ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [HACKERS] Plan invalidation vs. unnamed prepared statements
Tom Lane wrote: Gregory Stark <[EMAIL PROTECTED]> writes: Can we forcibly discard it if *any* messages are received that might invalidate a plan? So basically it would work fine unless anyone in the system does any DDL at all? I guess that has the downside of introducing random unpredictable failures. Ugh :-( Or stash the query string and replan it (possibly in the query cache this time) if someone executes it a second time? I think that's either my plan A or C. The main problem with uncontrolled replanning is that there's no way to detect a change in the query properties. For example suppose the query is "SELECT * FROM foo" and we've already told the client (via Describe Statement) that that returns two integer columns. If an inval now arrives because of "ALTER TABLE foo ADD COLUMN" (or perhaps worse, ALTER COLUMN TYPE), we've got a problem. If we just blindly replan then we'll return tuples that do not match the previously given row description, which will certainly break most clients. It will always be a good question what user expects as a result of 'SELECT * FROM...'. For example, client may use ODBC or some other interface for DB communication. One the first step he retrieves information about the table and it's datatypes, on the second tries to fetch rows (using interface functions). Client application won't even guess that table could be changed between these two steps. It's impossible to avoid such situations, because we can't know how the user retrieves information about results he will expect. The plan caching module has enough infrastructure to detect and complain about these sorts of situations, and it also knows how to manage lock acquisition so that once we've decided a plan is still good, the tables won't change underneath us while we use the plan. I don't see any way to make comparable guarantees without the overhead that goes with the cache manager. It's a required overhead. Result should be valid on the execution time, not on prepare. Cache manager is the best for this. regards, tom lane ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster ---(end of broadcast)--- TIP 3: Have you checked our extensive FAQ? http://www.postgresql.org/docs/faq
Re: [JDBC] [HACKERS] Plan invalidation vs. unnamed prepared statements
I think C is how the JDBC driver is written. We name the statements if they have been used more than prepareThreshold times. So we have a mechanism by which to allow statements to be cached, or not. Dave On 6-Mar-07, at 1:14 PM, Tom Lane wrote: Gregory Stark <[EMAIL PROTECTED]> writes: Can we forcibly discard it if *any* messages are received that might invalidate a plan? So basically it would work fine unless anyone in the system does any DDL at all? I guess that has the downside of introducing random unpredictable failures. Ugh :-( Or stash the query string and replan it (possibly in the query cache this time) if someone executes it a second time? I think that's either my plan A or C. The main problem with uncontrolled replanning is that there's no way to detect a change in the query properties. For example suppose the query is "SELECT * FROM foo" and we've already told the client (via Describe Statement) that that returns two integer columns. If an inval now arrives because of "ALTER TABLE foo ADD COLUMN" (or perhaps worse, ALTER COLUMN TYPE), we've got a problem. If we just blindly replan then we'll return tuples that do not match the previously given row description, which will certainly break most clients. The plan caching module has enough infrastructure to detect and complain about these sorts of situations, and it also knows how to manage lock acquisition so that once we've decided a plan is still good, the tables won't change underneath us while we use the plan. I don't see any way to make comparable guarantees without the overhead that goes with the cache manager. regards, tom lane ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [HACKERS] Plan invalidation vs. unnamed prepared statements
Gregory Stark <[EMAIL PROTECTED]> writes: > Can we forcibly discard it if *any* messages are received that might > invalidate a plan? So basically it would work fine unless anyone in the system > does any DDL at all? I guess that has the downside of introducing random > unpredictable failures. Ugh :-( > Or stash the query string and replan it (possibly in the query cache this > time) if someone executes it a second time? I think that's either my plan A or C. The main problem with uncontrolled replanning is that there's no way to detect a change in the query properties. For example suppose the query is "SELECT * FROM foo" and we've already told the client (via Describe Statement) that that returns two integer columns. If an inval now arrives because of "ALTER TABLE foo ADD COLUMN" (or perhaps worse, ALTER COLUMN TYPE), we've got a problem. If we just blindly replan then we'll return tuples that do not match the previously given row description, which will certainly break most clients. The plan caching module has enough infrastructure to detect and complain about these sorts of situations, and it also knows how to manage lock acquisition so that once we've decided a plan is still good, the tables won't change underneath us while we use the plan. I don't see any way to make comparable guarantees without the overhead that goes with the cache manager. regards, tom lane ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [HACKERS] Plan invalidation vs. unnamed prepared statements
"Tom Lane" <[EMAIL PROTECTED]> writes: > B. Don't store the unnamed statement in the plan cache. To make sure > it's not used anymore when the plan might be stale, forcibly discard > the unnamed statement after execution. This would get rid of a lot > of overhead but would mean a significant change in the protocol-level > behavior. It's hard to guess how many clients might be broken by it > --- conceivably not any, but that seems too optimistic :-( Can we forcibly discard it if *any* messages are received that might invalidate a plan? So basically it would work fine unless anyone in the system does any DDL at all? I guess that has the downside of introducing random unpredictable failures. Or stash the query string and replan it (possibly in the query cache this time) if someone executes it a second time? Can't say I like either of those options much, just trying to brainstorm. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
[HACKERS] Plan invalidation vs. unnamed prepared statements
[ cc'd to pgsql-jdbc which seems the group most likely to be affected by any protocol change ] So I've been working on a plan cache module per my earlier proposal, and I've run up against a problem with getting exec_parse_message to use it. The problem is that the current rather hackish handling of unnamed prepared statements doesn't fit in. Per the documentation, unnamed statements are supposed to be "optimized for the case of executing a query only once and then discarding it". In the current code this largely just means that we avoid copying the parse/plan trees into the normal PreparedStatement cache, preferring to let them sit in the context where they were generated (which means that any detritus generated by the parser/planner can't be recovered until we discard the unnamed statement, but that seems a good tradeoff in this situation). To use the plan cache for unnamed statements, there's going to have to be more overhead (more tree-copying) in this code path; moreover having the unnamed statement's plan in the cache will result in distributed overhead for checking it to see if it's still valid. This overhead is largely going to be wasted if the statement is always discarded immediately after use. I can think of several options for dealing with this: A. Just accept the extra overhead, thereby preserving the current behavior of unnamed statements, and gaining the benefit that plan invalidation will work correctly in the few cases where an unnamed statement's plan lasts long enough to need replanning. B. Don't store the unnamed statement in the plan cache. To make sure it's not used anymore when the plan might be stale, forcibly discard the unnamed statement after execution. This would get rid of a lot of overhead but would mean a significant change in the protocol-level behavior. It's hard to guess how many clients might be broken by it --- conceivably not any, but that seems too optimistic :-( C. Don't store the unnamed statement in the plan cache. To make sure it's not used anymore when the plan might be stale, don't analyze or plan at Parse-message time, but postpone *all* that work until Bind; and always discard the plan after Execute. We could still do "raw parsing" at Parse time, since that's independent of database state, but all but the most trivial syntactic errors would now occur at Bind not Parse time, as well as the majority of the time expenditure. This still amounts to a change in the protocol semantics, although it's a lot more subtle than plan B. Also there's a problem if the client does Describe Statement before Bind: we still have to run parse analysis before we can answer, and if we then throw that away, we have no very good way to guarantee that the statement still has the same description when it's subsequently executed; plus we end up doing parse analysis twice. D. Don't store the unnamed statement in the plan cache, and just ignore the possibility that its plan might become stale before use. That's exactly what happens now, but considering that the whole point of the plan inval work is to seal off such pitfalls, I can't say that I care for this alternative. Comments? I'm leaning to plan A but wanted to see if anyone would support plan B or sees a way to fix plan C. regards, tom lane ---(end of broadcast)--- TIP 7: You can help support the PostgreSQL project by donating at http://www.postgresql.org/about/donate
Re: [HACKERS] Plan invalidation design
On Feb 18, 9:35 am, [EMAIL PROTECTED] (Tom Lane) wrote: > Russell Smith <[EMAIL PROTECTED]> writes: > > > If you replan and immutable function, aren't you possibly messing up a > > functional index that is using the old function. Hey, if you change an > > immutable function that has an index, you are in trouble already. > > True. While I agree that if you change an immutable function used by an index, your index will break, I do not understand how re-planning it will cause problems. Is the worry that the index will not pick up on the new plan? Andrew ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [HACKERS] Plan invalidation design
Simon Riggs wrote: > On Sat, 2007-02-17 at 12:48 -0500, Tom Lane wrote: > > > Relcache inval casts a fairly wide net; for example, adding or dropping an > > index will invalidate all plans using the index's table whether or not > > they used that particular index, and I believe that VACUUM will also > > result in a relcache inval due to updating the table's pg_class row. > > I think this is a good thing though --- for instance, after adding an > > index it seems a good idea to replan to see if the new index is useful, > > and replanning after a VACUUM is useful if the table has changed size > > enough to warrant a different plan. OTOH this might mean that plans on a > > high-update-traffic table never survive very long because of autovacuum's > > efforts. If that proves to be a problem in practice we can look at ways > > to dial down the number of replans, but for the moment I think it's more > > important to be sure we *can* replan at need than to find ways to avoid > > replans. > > Just some info on that: In an update-intensive scenario, I'm seeing > VACUUMs every 2 minutes on the heaviest hit tables on CVS HEAD on a > medium-powered 4-CPU server. Re-planning multiple queries on 100+ > sessions every few minutes would not be good. I would think the inval would be sent if relpages changed by more than a certain threshold, say 10%. In steady state, a high-update table that's under continuous vacuum should not change size much, thus no replan. But clearly the point here is to get the inval to be sent at all, and look for inhibitions mechanisms later. > Presumably ANALYZE would have the same effect? It would be nice to have a way to calculate a delta from the previous statistics snapshot and send an inval if it's appropriate. Can it be done? -- Alvaro Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. ---(end of broadcast)--- TIP 1: if posting/reading through Usenet, please send an appropriate subscribe-nomail command to [EMAIL PROTECTED] so that your message can get through to the mailing list cleanly
Re: [HACKERS] Plan invalidation design
On Sat, 2007-02-17 at 12:48 -0500, Tom Lane wrote: > Relcache inval casts a fairly wide net; for example, adding or dropping an > index will invalidate all plans using the index's table whether or not > they used that particular index, and I believe that VACUUM will also > result in a relcache inval due to updating the table's pg_class row. > I think this is a good thing though --- for instance, after adding an > index it seems a good idea to replan to see if the new index is useful, > and replanning after a VACUUM is useful if the table has changed size > enough to warrant a different plan. OTOH this might mean that plans on a > high-update-traffic table never survive very long because of autovacuum's > efforts. If that proves to be a problem in practice we can look at ways > to dial down the number of replans, but for the moment I think it's more > important to be sure we *can* replan at need than to find ways to avoid > replans. Just some info on that: In an update-intensive scenario, I'm seeing VACUUMs every 2 minutes on the heaviest hit tables on CVS HEAD on a medium-powered 4-CPU server. Re-planning multiple queries on 100+ sessions every few minutes would not be good. It seems a reasonable working assumption that HOT will reduce that requirement considerably, but its something to watch. Thanks for drawing attention to it. Presumably ANALYZE would have the same effect? -- Simon Riggs EnterpriseDB http://www.enterprisedb.com ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [HACKERS] Plan invalidation design
Gregory Stark wrote: [snip] Hm. The set of output columns could change? How? If you prepare "select *" and add a column, you're saying the query should start failing? That seems strange given the behaviour of views, which is that once parsed the list of columns is written in stone. It seems prepared queries should work the same way that views work and remember which physical column they were referring to previously. (Personally I don't like that behaviour but it feels like this should be consistent with it.) I guess you do have a serious problem if you redefine the type of a column or redefine a view (though I think you would have to drop and recreate it, CREATE OR REPLACE wouldn't let you change the output columns). I would think it best to move towards changing views to not have output columns set in stone. It seems unreasonable that you can add/drop/alter columns in a table as much as you like, but you can't touch a view. I also not excited about the current view restrictions. Which means we don't want to start backing the idea by putting in more code that acts in the same way. I'm guessing from what Tom is saying, that the reason we have views set in stone is because they are/can be an example of inlined SQL. Particularly when views are built on views. Any further enlightenment welcome, but probably off topic for this thread. Russell Smith ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [HACKERS] Plan invalidation design
On 2/19/07, Tom Lane <[EMAIL PROTECTED]> wrote: Gregory Stark <[EMAIL PROTECTED]> writes: > If you prepare "select *" and add a column, you're saying the query should > start failing? Either fail or change output; which you like better? The whole point of this exercise is to support plpgsql functions that do something like create temp table foo ... select * into rec from foo ... drop table foo ... If that's the case, do you think there is a simpler way to handle this problem than plan invalidation? Maybe I'm oversimplifying things a bit here, but how about something like: create local table foo ... select * into rec from foo ... this isn't completely unsurprising...we have 'SET LOCAL, etc. merlin ---(end of broadcast)--- TIP 7: You can help support the PostgreSQL project by donating at http://www.postgresql.org/about/donate
Re: [HACKERS] Plan invalidation design
Gregory Stark <[EMAIL PROTECTED]> writes: > If you prepare "select *" and add a column, you're saying the query should > start failing? Either fail or change output; which you like better? The whole point of this exercise is to support plpgsql functions that do something like create temp table foo ... select * into rec from foo ... drop table foo ... and from the system's point of view successive executions of this sequence are talking about completely different tables. There's no reason to suppose they have the same expansion of "*". I'd also like to think that the semantics of a plpgsql function are not going to be different the first time it's executed in a session than subsequent times, which suggests that indeed it ought to cope with "*" expanding differently from last time. To do what you're suggesting, we'd have to redesign parse analysis so that expansion of "*" was a completely separate step from everything else, producing a parse tree that's still raw except for that one change; and I'm not sure what other strange decisions we'd have to make. I don't find that an attractive idea. regards, tom lane ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [HACKERS] Plan invalidation design
"Tom Lane" <[EMAIL PROTECTED]> writes: > Russell Smith <[EMAIL PROTECTED]> writes: >> Tom Lane wrote: >>> 2. Given a handle for a previously stored query, check to see if the plan >>> is still up to date; if not, regenerate it from the raw parse tree (note >>> this could result in failure, eg if a column used by the query has been >>> dropped). Then return the plan tree. >>> >> What do we do in the case of failure? Die in the same way we do now >> when you can't use the plan that's been made? > > Well, the difference is that at plan use you might get an error that > currently could only occur at initial query parsing. I don't see that > this is a big deal, but it will be a change in behavior. > > One thing I realized since yesterday is that it'll have to be possible > for the caller to tell whether the plan has changed since he last saw > it (perhaps via a re-plan counter included in the cache entry). It's > entirely possible that the set of output columns will have changed, > and so the caller may need to re-do derived work. For example plpgsql > will need to re-do its analysis of whether a plan is "simple". Hm. The set of output columns could change? How? If you prepare "select *" and add a column, you're saying the query should start failing? That seems strange given the behaviour of views, which is that once parsed the list of columns is written in stone. It seems prepared queries should work the same way that views work and remember which physical column they were referring to previously. (Personally I don't like that behaviour but it feels like this should be consistent with it.) I guess you do have a serious problem if you redefine the type of a column or redefine a view (though I think you would have to drop and recreate it, CREATE OR REPLACE wouldn't let you change the output columns). >>> We probably want to return a direct pointer to the cached plan tree >>> instead of making a copy. This should be safe, because the executor now >>> treats plan trees as read-only, but it does mean that when plan >>> invalidation occurs the cached plan tree might still be in use. > >> excuse my ignorance here, but under what circumstances is a plan in use >> for a single backend at the same time as it's invalidated. Invalidation messages can occur at certain. If you access any new table or object while the plan is still running, either because you're in a plpgsql loop fetching records from it, or because some function you're calling in the query runs some other sql against another table then you'll receive any pending invalidation messages. It should only be possible to receive messages from operations that are legal to execute while someone is using the object. So, for example, creating new indexes. So if you're actively in the process of using the plan it shouldn't be necessary to junk it. Perhaps that means it would be handy to have two kinds of invalidation messages. Hard invalidations mean that anybody with cached plans should immediately junk them and throw up nasty errors and assertion failures if they're in a state when that shouldn't happen. And Soft invalidations mean you shouldn't start any new queries but any that are executing are still ok. -- Gregory Stark EnterpriseDB http://www.enterprisedb.com ---(end of broadcast)--- TIP 2: Don't 'kill -9' the postmaster
Re: [HACKERS] Plan invalidation design
Russell Smith <[EMAIL PROTECTED]> writes: > Tom Lane wrote: >> 2. Given a handle for a previously stored query, check to see if the plan >> is still up to date; if not, regenerate it from the raw parse tree (note >> this could result in failure, eg if a column used by the query has been >> dropped). Then return the plan tree. >> > What do we do in the case of failure? Die in the same way we do now > when you can't use the plan that's been made? Well, the difference is that at plan use you might get an error that currently could only occur at initial query parsing. I don't see that this is a big deal, but it will be a change in behavior. One thing I realized since yesterday is that it'll have to be possible for the caller to tell whether the plan has changed since he last saw it (perhaps via a re-plan counter included in the cache entry). It's entirely possible that the set of output columns will have changed, and so the caller may need to re-do derived work. For example plpgsql will need to re-do its analysis of whether a plan is "simple". What we might want in some cases is for the caller to decide to error out if the set of output columns changes. I think this is likely appropriate for queries prepared via the Parse protocol message, because we may have already told the client what the column set is, and it won't be prepared to deal with getting a different set of columns back. I'm not sure now whether that's appropriate for every call site, but if it is then we could avoid some of these definitional issues. >> We probably want to return a direct pointer to the cached plan tree >> instead of making a copy. This should be safe, because the executor now >> treats plan trees as read-only, but it does mean that when plan >> invalidation occurs the cached plan tree might still be in use. > excuse my ignorance here, but under what circumstances is a plan in use > for a single backend at the same time as it's invalidated. There shouldn't be any structural changes in a table once you've acquired lock on it, but there could be statistics changes, eg from VACUUM; and the relcache inval mechanism currently doesn't distinguish those cases. We'd need some such concept anyway if we ever extend the invalidation to cover functions, because there's no locking on them. > What other circumstances could you have a syntax error from a query that > has been successfully planned and parsed? DROP COLUMN, DROP FUNCTION, ... lots of possibilities. > I've read this paragraph 3 times now and am still quite unclear about > the requirements for the original query to be stored. Is the plan cache > going to replace the syntax check which I thought would have been done > in gram.y. We don't need to re-do that syntax check, precisely because it's purely a syntax check and doesn't involve any database state. > If you replan and immutable function, aren't you possibly messing up a > functional index that is using the old function. Hey, if you change an > immutable function that has an index, you are in trouble already. True. > Replanning pl/pgsql with CREATE TEMP TABLE would be a good use here. > You loose the preplanning benefits, but we remove the ongoing problem > where people report that their temp-table isn't working. Yeah, that's one of the main arguments why this is worth the trouble. > Even function alterations to pl/pgsql should a replan. But of more > interest is being able to use the old function for currently running > transactions when the function is changed. Last time I tried to edit a > pl/pgsql function while it was being used by a transaction, the > transaction failed because the function definition changed. I fixed a couple of bugs in that area recently --- the current behavior should be that any active execution of a plpgsql function will finish out using the function definition that was current when it started. But that's something that's local to the PL function manager and doesn't really have anything to do with plans using the function. Inlined SQL functions are the exception to the rule that a plan doesn't know exactly what a function it calls does. > Is the race condition here any more likely to happen than the failure of > a re plan when something has changed from underneath the original query? It's not really the same thing --- the problem is making sure that your check for table changes is accurate, and doesn't miss a change that commits just after you look. regards, tom lane ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
Re: [HACKERS] Plan invalidation design
Tom Lane wrote: I'm starting to think about the long-wanted plan invalidation mechanism. Here's a sketch --- anyone see any problems? * Create a new module, say src/backend/utils/cache/plancache.c, that we will put in charge of all long-lived plans --- or at least those cached by PREPARE, plpgsql, and RI triggers. I'm unsure whether we should make all SPI plans work this way or not; it's possible that doing so would change SPI's API behavior enough to break user-written code. Any thoughts on that? * plancache.c will have two basic functions: 1. Given a query's raw parse tree (that is, the raw output of gram.y), analyze and plan the query. Store both the parse tree and plan in a backend-local cache table, and return a handle for the table entry as well as the plan tree. 2. Given a handle for a previously stored query, check to see if the plan is still up to date; if not, regenerate it from the raw parse tree (note this could result in failure, eg if a column used by the query has been dropped). Then return the plan tree. What do we do in the case of failure? Die in the same way we do now when you can't use the plan that's been made? We probably want to return a direct pointer to the cached plan tree instead of making a copy. This should be safe, because the executor now treats plan trees as read-only, but it does mean that when plan invalidation occurs the cached plan tree might still be in use. We'll probably need to have a notion of a reference count: so the two functions above would increment the plan's refcount and there would be a third "ReleasePlanCache" function to call when done using a plan (and, hence, these references would need to be supported by the ResourceManager mechanism). excuse my ignorance here, but under what circumstances is a plan in use for a single backend at the same time as it's invalidated. What potential failures does this introduce? If you are using the old plan, and the new plan fails as mentioned above. Where are we then? Note that the source object for caching is a raw parse tree. This should work since we already require that gram.y not look into the database during its processing; therefore, the raw tree need never be invalidated. It'd be conceptually simpler if we passed in a query string instead, but I don't think that works for PREPARE, because it might be embedded in a multi-command string. (We do probably want to pass in the original query string too, if available, because it's needed for syntax error reporting.) nodes/copyfuncs.c will need some expansion, as I don't believe it has coverage for all raw-parse-tree node types. If the syntax has become invalid, that is because the columns in the query, or tables have changed. Is this information not available in the plan tree? What other circumstances could you have a syntax error from a query that has been successfully planned and parsed? I've read this paragraph 3 times now and am still quite unclear about the requirements for the original query to be stored. Is the plan cache going to replace the syntax check which I thought would have been done in gram.y. Invalidation will be detected by having plancache.c watch for relcache invalidation events, using the existing inval.c callback mechanism. On any relcache inval, traverse the plan cache looking for plans that mention the invalidated relation in their rangetables, and mark them as needing to be regenerated before next use. (If they currently have refcount zero, we could delete the plan part of the cache entry immediately.) Relcache inval casts a fairly wide net; for example, adding or dropping an index will invalidate all plans using the index's table whether or not they used that particular index, and I believe that VACUUM will also result in a relcache inval due to updating the table's pg_class row. I think this is a good thing though --- for instance, after adding an index it seems a good idea to replan to see if the new index is useful, and replanning after a VACUUM is useful if the table has changed size enough to warrant a different plan. OTOH this might mean that plans on a high-update-traffic table never survive very long because of autovacuum's efforts. If that proves to be a problem in practice we can look at ways to dial down the number of replans, but for the moment I think it's more important to be sure we *can* replan at need than to find ways to avoid replans. Note that I'm currently intending to detect only relcache invals, not changes to functions or operators used in the plan. (Relcache inval will cover view redefinitions, though.) We could extend it to handle that later, but it looks like a lot more mechanism and overhead for not a lot of gain. AFAICS there are only three cases where there'd be a benefit: * if you redefine an immutable function, any places where its result has been pre-computed by constant-folding wouldn't get updated without inval. If you replan and immutable function, aren't y
Re: [HACKERS] Plan invalidation design
Tom Lane wrote: place. But the question to answer is why the re-plan won't yield just the same plan as before. oh and when the estimated cost repeatedly do not match the actual cost, we of course want to generate an email with all relevant information that is send to this list ;) regards, Lukas ---(end of broadcast)--- TIP 7: You can help support the PostgreSQL project by donating at http://www.postgresql.org/about/donate
Re: [HACKERS] Plan invalidation design
Tom Lane wrote: Lukas Kahwe Smith <[EMAIL PROTECTED]> writes: I remember that there was discussion about invalidating plans who's estimated cost turn out to be severely off when executed. That's something we might think about after the infrastructure is in place. But the question to answer is why the re-plan won't yield just the same plan as before. Yeah, also invalidating plans like this only really makes sense once we have the ability to keep multiple plans around for different sets of parameters. Otherwise we could also end up in a situation where after every execution we determine that a re-plan is necessary because the parameters used differ in distribution. regards, Lukas ---(end of broadcast)--- TIP 7: You can help support the PostgreSQL project by donating at http://www.postgresql.org/about/donate
Re: [HACKERS] Plan invalidation design
Lukas Kahwe Smith <[EMAIL PROTECTED]> writes: > I remember that there was discussion about invalidating plans who's > estimated cost turn out to be severely off when executed. That's something we might think about after the infrastructure is in place. But the question to answer is why the re-plan won't yield just the same plan as before. regards, tom lane ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
Re: [HACKERS] Plan invalidation design
Tom Lane wrote: Relcache inval casts a fairly wide net; for example, adding or dropping an index will invalidate all plans using the index's table whether or not they used that particular index, and I believe that VACUUM will also result in a relcache inval due to updating the table's pg_class row. I think this is a good thing though --- for instance, after adding an index it seems a good idea to replan to see if the new index is useful, and replanning after a VACUUM is useful if the table has changed size enough to warrant a different plan. OTOH this might mean that plans on a high-update-traffic table never survive very long because of autovacuum's efforts. If that proves to be a problem in practice we can look at ways to dial down the number of replans, but for the moment I think it's more important to be sure we *can* replan at need than to find ways to avoid replans. I remember that there was discussion about invalidating plans who's estimated cost turn out to be severely off when executed. That is probably a more reliable metric (than invalidating with every VACCUM - unless of course the amount of changed rows is considered), though it will probably put a fixed overhead on all relevant queries. So it might not be feasible. Of course this checking after a query runs longer than expected also means that at least one execution will in fact have to run slow instead of preempting this from happening at all. Also while not directly related it might be thing to keep in mind. It would also be cool to support multiple plans for different sets of parameters, since obviously the data distribution and therefore the optimal plan will potentially vary greatly with different parameters. regards, Lukas PS: I moved "Plan invalidation" to confirmed on the wishlist .. ---(end of broadcast)--- TIP 9: In versions below 8.0, the planner will ignore your desire to choose an index scan if your joining column's datatypes do not match
[HACKERS] Plan invalidation design
I'm starting to think about the long-wanted plan invalidation mechanism. Here's a sketch --- anyone see any problems? * Create a new module, say src/backend/utils/cache/plancache.c, that we will put in charge of all long-lived plans --- or at least those cached by PREPARE, plpgsql, and RI triggers. I'm unsure whether we should make all SPI plans work this way or not; it's possible that doing so would change SPI's API behavior enough to break user-written code. Any thoughts on that? * plancache.c will have two basic functions: 1. Given a query's raw parse tree (that is, the raw output of gram.y), analyze and plan the query. Store both the parse tree and plan in a backend-local cache table, and return a handle for the table entry as well as the plan tree. 2. Given a handle for a previously stored query, check to see if the plan is still up to date; if not, regenerate it from the raw parse tree (note this could result in failure, eg if a column used by the query has been dropped). Then return the plan tree. We probably want to return a direct pointer to the cached plan tree instead of making a copy. This should be safe, because the executor now treats plan trees as read-only, but it does mean that when plan invalidation occurs the cached plan tree might still be in use. We'll probably need to have a notion of a reference count: so the two functions above would increment the plan's refcount and there would be a third "ReleasePlanCache" function to call when done using a plan (and, hence, these references would need to be supported by the ResourceManager mechanism). Note that the source object for caching is a raw parse tree. This should work since we already require that gram.y not look into the database during its processing; therefore, the raw tree need never be invalidated. It'd be conceptually simpler if we passed in a query string instead, but I don't think that works for PREPARE, because it might be embedded in a multi-command string. (We do probably want to pass in the original query string too, if available, because it's needed for syntax error reporting.) nodes/copyfuncs.c will need some expansion, as I don't believe it has coverage for all raw-parse-tree node types. Invalidation will be detected by having plancache.c watch for relcache invalidation events, using the existing inval.c callback mechanism. On any relcache inval, traverse the plan cache looking for plans that mention the invalidated relation in their rangetables, and mark them as needing to be regenerated before next use. (If they currently have refcount zero, we could delete the plan part of the cache entry immediately.) Relcache inval casts a fairly wide net; for example, adding or dropping an index will invalidate all plans using the index's table whether or not they used that particular index, and I believe that VACUUM will also result in a relcache inval due to updating the table's pg_class row. I think this is a good thing though --- for instance, after adding an index it seems a good idea to replan to see if the new index is useful, and replanning after a VACUUM is useful if the table has changed size enough to warrant a different plan. OTOH this might mean that plans on a high-update-traffic table never survive very long because of autovacuum's efforts. If that proves to be a problem in practice we can look at ways to dial down the number of replans, but for the moment I think it's more important to be sure we *can* replan at need than to find ways to avoid replans. Note that I'm currently intending to detect only relcache invals, not changes to functions or operators used in the plan. (Relcache inval will cover view redefinitions, though.) We could extend it to handle that later, but it looks like a lot more mechanism and overhead for not a lot of gain. AFAICS there are only three cases where there'd be a benefit: * if you redefine an immutable function, any places where its result has been pre-computed by constant-folding wouldn't get updated without inval. * if you have a SQL function that's been inlined into a plan, a change in the function definition wouldn't get reflected into the plan without inval. * if you alter a function and change its volatility property, that might possibly affect the shape of plans that use the function (for instance some optimization transformation might now be allowed or not). To my memory none of these problems have been complained of from the field. Making the cache module able to detect function-related invalidations would be a bit of work --- for example, if a function has been inlined, there is no recognizable reference to it at all in the plan tree, so we'd have to modify the planner to track such things and report them somehow. (The corresponding problem for views doesn't exist, because there is still a rangetable entry for a view after it's been expanded.) So I think this is a "maybe do someday" part, not something to do in the first release. One interest
Re: [HACKERS] Plan invalidation plans
Heikki Linnakangas <[EMAIL PROTECTED]> writes: > Is anyone working on plan invalidation? I might take a stab at it during > the 8.3 cycle. I haven't given it any thought yet, I thought I'd check > first to avoid duplicate work. I'd been planning to tackle it too, but would be happy to let someone else do it; or we could cooperate. regards, tom lane ---(end of broadcast)--- TIP 5: don't forget to increase your free space map settings
[HACKERS] Plan invalidation plans
Hi, Is anyone working on plan invalidation? I might take a stab at it during the 8.3 cycle. I haven't given it any thought yet, I thought I'd check first to avoid duplicate work. -- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com ---(end of broadcast)--- TIP 7: You can help support the PostgreSQL project by donating at http://www.postgresql.org/about/donate
