Re: [HACKERS] Patch: plan invalidation vs stored procedures
Martin Pihlak [EMAIL PROTECTED] writes: Yes, creating a new message type was a bit short sighted -- attached is a patch that uses syscache invalidation messages instead. This also adds additional tupleId field to SharedInvalCatcacheMsg. This is used to identify the invalidated tuple in PROC messages, for now others still pass InvalidOid. Applied after rather heavy revision. Aside from the gripes I had yesterday, I found out on closer inspection that the patch did things all wrong for the case of a not-fully-planned cache item. I ended up discarding the existing code for that and instead using the planner machinery to extract dependencies of a parsed querytree. 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] Patch: plan invalidation vs stored procedures
Tom Lane wrote:
I hadn't read it yet, but that makes it wrong already. There's no need
for any new inval traffic --- the existing syscache inval messages on
pg_proc entries should serve fine.
Yes, creating a new message type was a bit short sighted -- attached is a patch
that uses syscache invalidation messages instead. This also adds additional
tupleId field to SharedInvalCatcacheMsg. This is used to identify the
invalidated tuple in PROC messages, for now others still pass InvalidOid.
More generally, if we are to try to invalidate on the strength of
pg_proc changes, what of other DDL changes? Operators, operator
classes, maybe? How about renaming a schema? I would like to see a
line drawn between things we find worth trying to track and things we
don't. If there is no such line, we're going to need a patch a lot
larger than this one.
The attached patch registers callbacks for namespace, operator and op family
catalog changes. PlanCacheCallback now takes catalog id as arg and can
take actions depending on the catalog type. Adding new catalogs is just a
matter of registering the callback in InitPlanCache. Of course, only tables
and functions have exact tracking -- other changes just invalidate all.
I'm wondering if the list of catalogs to be tracked should be fixed at all.
Maybe it would be better to call PlanCacheCallback directly on any syscache
entry invalidation? This way no catalog would be overlooked and the
cache_callback_list could be kept nice and short. PlanCacheCallback would
receive the catalog id and OID of the invalidated tuple and could then
decide whether it can do precise invalidation, flush the cache or just
skip the event. What do you think?
regards,
Martin
Index: src/backend/commands/prepare.c
===
RCS file: /projects/cvsroot/pgsql/src/backend/commands/prepare.c,v
retrieving revision 1.90
diff -c -r1.90 prepare.c
*** src/backend/commands/prepare.c 25 Aug 2008 22:42:32 - 1.90
--- src/backend/commands/prepare.c 28 Aug 2008 09:20:21 -
***
*** 189,197
/* Shouldn't have a non-fully-planned plancache entry */
if (!entry-plansource-fully_planned)
elog(ERROR, EXECUTE does not support unplanned prepared statements);
- /* Shouldn't get any non-fixed-result cached plan, either */
- if (!entry-plansource-fixed_result)
- elog(ERROR, EXECUTE does not support variable-result cached plans);
/* Evaluate parameters, if any */
if (entry-plansource-num_params 0)
--- 189,194
***
*** 463,469
cursor_options,
stmt_list,
true,
! true);
/* Now we can add entry to hash table */
entry = (PreparedStatement *) hash_search(prepared_queries,
--- 460,466
cursor_options,
stmt_list,
true,
! false);
/* Now we can add entry to hash table */
entry = (PreparedStatement *) hash_search(prepared_queries,
***
*** 524,534
TupleDesc
FetchPreparedStatementResultDesc(PreparedStatement *stmt)
{
! /*
! * Since we don't allow prepared statements' result tupdescs to change,
! * there's no need for a revalidate call here.
! */
! Assert(stmt-plansource-fixed_result);
if (stmt-plansource-resultDesc)
return CreateTupleDescCopy(stmt-plansource-resultDesc);
else
--- 521,529
TupleDesc
FetchPreparedStatementResultDesc(PreparedStatement *stmt)
{
! /* Revalidate the plan to allow changes in tupdescs. */
! RevalidateCachedPlan(stmt-plansource, false);
!
if (stmt-plansource-resultDesc)
return CreateTupleDescCopy(stmt-plansource-resultDesc);
else
***
*** 650,658
/* Shouldn't have a non-fully-planned plancache entry */
if (!entry-plansource-fully_planned)
elog(ERROR, EXPLAIN EXECUTE does not support unplanned prepared statements);
- /* Shouldn't get any non-fixed-result cached plan, either */
- if (!entry-plansource-fixed_result)
- elog(ERROR, EXPLAIN EXECUTE does not support variable-result cached plans);
/* Replan if needed, and acquire a transient refcount */
cplan = RevalidateCachedPlan(entry-plansource, true);
--- 645,650
Index: src/backend/nodes/copyfuncs.c
===
RCS file: /projects/cvsroot/pgsql/src/backend/nodes/copyfuncs.c,v
retrieving revision 1.401
diff -c -r1.401 copyfuncs.c
*** src/backend/nodes/copyfuncs.c 22 Aug 2008 00:16:03 - 1.401
--- src/backend/nodes/copyfuncs.c 28 Aug 2008 09:20:21 -
***
*** 84,89
--- 84,90
COPY_NODE_FIELD(returningLists);
COPY_NODE_FIELD(rowMarks);
COPY_NODE_FIELD(relationOids);
+ COPY_NODE_FIELD(functionOids);
COPY_SCALAR_FIELD(nParamExec);
return newnode;
***
*** 1882,1887
--- 1883,1889
COPY_NODE_FIELD(limitCount);
COPY_NODE_FIELD(rowMarks);
COPY_NODE_FIELD(setOperations);
+
Re: [HACKERS] Patch: plan invalidation vs stored procedures
On Aug 20, 2008, at 9:18 AM, Tom Lane wrote: However I have no hard evidence to back up drawing the line there rather than somewhere else. Anyone have data on what sort of DDL changes are common in their applications? I've worked in environments where we used stored functions extensively and where we didn't. Table DDL is generally fairly common in both cases, and if stored functions or views are used, it's very common for table DDL to trigger updates in views and functions. It's fairly common to have to update just functions to kill bugs or change functionality. Trigger changes are a bit less frequent, and views are probably the least frequent. -- Decibel!, aka Jim C. Nasby, Database Architect [EMAIL PROTECTED] Give your computer some brain candy! www.distributed.net Team #1828 smime.p7s Description: S/MIME cryptographic signature
Re: [HACKERS] Patch: plan invalidation vs stored procedures
Am 19.08.2008 um 20:47 schrieb Tom Lane: Bruce Momjian [EMAIL PROTECTED] writes: Joshua Drake wrote: Is our backpatch policy documented? It does not appear to be in developer FAQ. Seems we need to add it. I'm not sure that I *want* a formal written-down backpatch policy. Whether (and how far) to backpatch has always been a best-judgment call in the past, and we've gotten along fine with that. I think having a formal policy is just likely to lead to even more complaints: either patching or not patching could result in second-guessing by someone who feels he can construe the policy to match the result he prefers. Agreeing to you and some later posters in this thread, I would not vote for a formal policy either. But IMHO there should be a general, informal note about backpatching in developer docs/faqs. A place where you can point to, and a chance for new people to read about the postgres way of handling backpatching. Btw., how backpatching is handled here is one of the reasons I trust my data to postgres. Best Regards Michael -- 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] Patch: plan invalidation vs stored procedures
Le mercredi 20 août 2008, Tom Lane a écrit : That just begs the question of what's the difference between a bug and a limitation. AFAICS, having such a policy/guideline/whatchacallit in place wouldn't have done a single thing to stop the current flamewar, because the people who want this thing back-patched are insisting that it's a bug, while those who don't are saying it's a long-known limitation. As a person who previously insisted it was a bug, I'd like to take the opportunity to claim that I didn't realize this was a limitation of the design of plan invalidation, which now seems related to DDL operations. Realizing this earlier would have resulted in no mail at all on this thread from here. There's certainly a balance between -hackers readers not doing their homework and people in the know choosing not to re-estate known things... Also, there are a whole lot more considerations in a backpatch decision than just is it a bug. The (estimated) risk of creating new bugs and the extent to which the patch will change behavior that apps might be relying on are two big reasons why we might choose not to back-patch a bug fix. And this way the project works is what leads its users not to fear minor upgrades, which is something I (we all?) highly value. Regards, -- dim signature.asc Description: This is a digitally signed message part.
Re: [HACKERS] Patch: plan invalidation vs stored procedures
On Tue, 2008-08-19 at 19:45 -0400, Tom Lane wrote: Hannu Krosing [EMAIL PROTECTED] writes: If there is plan invalidation then you just change called1() to return one more field and that's it - no juggling with C) and D) and generally less things that can go wrong. That is a pure flight of fancy. Nope, this is description of real situation when you have to maintain lots and lots of functions. Adjusting a function's API generally requires source-code changes on the caller side too. Adding a column to table does not (even generally) require changing all queries accessing that table, why should adding a column to functions return type do ? There might be a few limited cases where you can avoid that, but that doesn't leave you with much of an argument that this is a critical bug fix. It's a corner case and little more. It is a corner case if you don't have a dynamic system, evolving over time, which relies heavily on functions . It is a complete non-issue if you don't use functions at all. FWIW, given that there will probably always be corner cases. I can see the attraction in Simon's suggestion of providing a way to manually issue a system-wide forced plan flush. That was also what I suggested as one blanket way of solving the bigger issue you brought up, that of not knowing where to stop tracking dependencies for plan invalidation. My thinking was, that this trades one-time inefficiency (replanning all stored plans) against more general but spread in time inefficiency of current patch (sending registration messages around for each function OID you depend on at each time you 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] Patch: plan invalidation vs stored procedures
The lack of plan invalidation is limitation that also has two bugs attached to it. I agree that full fledged patch to fix all the isssues should not be done in 8.3. I can't agree that effort to get the bugs fixed already in 8.3 should not be made. I can understand that hackers here have learned to live with these bugs and limitations but not all the users are reading these flame wars here and most of them are not even aware of these bugs until they are hit by them. Sql function bug is such that users probably won't even understand what hit them and how the data got mangled. - If there is nothing that can be done in 8.3 at least warning should be added into the documentation. It will be just one more don't in our long list don'ts for our developers. ERROR: cache lookup failed for function. - Could the plan be marked as invalid so it would fail only once so the next call to the function would get replanned and work again. At least it would be better than losing parts of application for indeterminate time. - Should update pg_proc set proname = proname; be the current solution to the problem or has someone something better to offer. We could scan released code for DROP FUNCTION and generate plan invalidation statement as last item of transaction releasing the code. - Could some less dangerous looking mechanism be added to 8.3 that wouldn't make users not used to PostgreSQL limitations gasp for air when they see the workarounds :) Calling the problem limitation will not make it go away. I am quite sure that new users consider it a bug until thay are converted to perceive it as lmitation. No matter how many time the usage of functions in database is called corner case it does not make it a corner case. In my experience it is quite common practice on all the database systems i have worked with. I do get the impression that Tom who would prefer to get all the pl's out of PostgreSQL and live happily ever after with pure SQL standard. On Wed, Aug 20, 2008 at 11:27 AM, Dimitri Fontaine [EMAIL PROTECTED]wrote: Le mercredi 20 août 2008, Tom Lane a écrit : That just begs the question of what's the difference between a bug and a limitation. AFAICS, having such a policy/guideline/whatchacallit in place wouldn't have done a single thing to stop the current flamewar, because the people who want this thing back-patched are insisting that it's a bug, while those who don't are saying it's a long-known limitation. As a person who previously insisted it was a bug, I'd like to take the opportunity to claim that I didn't realize this was a limitation of the design of plan invalidation, which now seems related to DDL operations. Realizing this earlier would have resulted in no mail at all on this thread from here. There's certainly a balance between -hackers readers not doing their homework and people in the know choosing not to re-estate known things... Also, there are a whole lot more considerations in a backpatch decision than just is it a bug. The (estimated) risk of creating new bugs and the extent to which the patch will change behavior that apps might be relying on are two big reasons why we might choose not to back-patch a bug fix. And this way the project works is what leads its users not to fear minor upgrades, which is something I (we all?) highly value. Regards, -- dim
Re: [HACKERS] Patch: plan invalidation vs stored procedures
On Wed, Aug 20, 2008 at 03:12:43PM +0300, Asko Oja wrote: - If there is nothing that can be done in 8.3 at least warning should be added into the documentation. It will be just one more don't in our long list don'ts for our developers. I am in favour of that change in the 8.3 branch. ERROR: cache lookup failed for function. - Could the plan be marked as invalid so it would fail only once so the next call to the function would get replanned and work again. At least it would be better than losing parts of application for indeterminate time. That seems to me to be a behaviour change, not a bug fix. I agree that the current behaviour is pretty annoying. That is not the same thing as a bug except in the loosest sense. The system works as specified, and therefore it's not a bug. If the specification is wrong, you need a new specification; that's a bug fix that is usually pronounced major release. - Could some less dangerous looking mechanism be added to 8.3 that wouldn't make users not used to PostgreSQL limitations gasp for air when they see the workarounds :) I think it a very bad idea even to suggest that we start undertaking things like adding mechanisms to minor releases, even with smileys at the end of the sentence. I appreciate (possibly more than many hackers) the limitations that are imposed on users by some of the decisions historically taken by developers in some of the previous major releases. But I very strongly agree with Dimitri: the super-conservative approach to maintenance releases that this project takes is a really big benefit to users, and is ultra important in mission critical environments. Otherwise, it becomes practically impossible to get minor releases into production. If you have to worry about the possibility of major changes between minor versions, you will have to treat every release as a major release. I don't think we have sufficient commercial integration support yet that we can follow the lead of the Linux kernel, where the system vendor has the effective obligation to make sure your kernel actually works. In addition, if someone wants to develop back-patches for 8.3 that give it new functionality otherwise planned for 8.4, I see nothing wrong with them doing so. That's the advantage offered by having the source. But the idea that the new functionality should be patched back by the project because one is impatient is not on. A -- Andrew Sullivan [EMAIL PROTECTED] +1 503 667 4564 x104 http://www.commandprompt.com/ -- 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] Patch: plan invalidation vs stored procedures
On Wed, Aug 20, 2008 at 09:16:56AM -0400, Andrew Sullivan wrote: On Wed, Aug 20, 2008 at 03:12:43PM +0300, Asko Oja wrote: - If there is nothing that can be done in 8.3 at least warning should be added into the documentation. It will be just one more don't in our long list don'ts for our developers. I am in favour of that change in the 8.3 branch. +1 ERROR: cache lookup failed for function. - Could the plan be marked as invalid so it would fail only once so the next call to the function would get replanned and work again. At least it would be better than losing parts of application for indeterminate time. That seems to me to be a behaviour change, not a bug fix. I agree that the current behaviour is pretty annoying. That is not the same thing as a bug except in the loosest sense. The system works as specified, and therefore it's not a bug. If the specification is wrong, you need a new specification; that's a bug fix that is usually pronounced major release. - Could some less dangerous looking mechanism be added to 8.3 that wouldn't make users not used to PostgreSQL limitations gasp for air when they see the workarounds :) I think it a very bad idea even to suggest that we start undertaking things like adding mechanisms to minor releases, even with smileys at the end of the sentence. I appreciate (possibly more than many hackers) the limitations that are imposed on users by some of the decisions historically taken by developers in some of the previous major releases. But I very strongly agree with Dimitri: the super-conservative approach to maintenance releases that this project takes is a really big benefit to users, and is ultra important in mission critical environments. Otherwise, it becomes practically impossible to get minor releases into production. If you have to worry about the possibility of major changes between minor versions, you will have to treat every release as a major release. +10 This policy has allowed us to upgrade to new minor releases with a minimum of testing for critical systems and basically none for non- critical systems. We would never upgrade for minor releases if this changes. We do not have the resources to perform full regression tests without having a very big carrot such as the new features a major release contains. Cheers, Ken -- 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] Patch: plan invalidation vs stored procedures
Asko Oja wrote: I do get the impression that Tom who would prefer to get all the pl's out of PostgreSQL and live happily ever after with pure SQL standard. I have not seen the slightest evidence of this, and don't believe it for a minute. I understand some of the frustration you are feeling, but statements like this don't help anything. (And yes, I too have recently been bitten nastily by cached plan problems, and want to see them fixed. I rather like Simon's suggestion of a command or function that would clear the plan cache.) cheers andrew -- 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] Patch: plan invalidation vs stored procedures
Asko Oja escribió: In the first message Martin asked There are probably a lot of details that I have overlooked. I'd be really thankful for some constructive comments and criticism. Especially, what needs to be done to have this in the core. Feedback appreciated. Can we get back to the topic? This is where the interesting questions are: http://archives.postgresql.org/message-id/10333.1219179364%40sss.pgh.pa.us I think the efforts to get the patch in 8.3 are wasted time. Better concentrate on getting something good for everyone in 8.4. -- Alvaro Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. -- 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] Patch: plan invalidation vs stored procedures
Thanks for a nice replay Andrew. So best solution for 8.3 is update pg_proc set proname = proname; whenever you need to drop and create functions or some in house patch. Lets get on with 8.4 Asko On Wed, Aug 20, 2008 at 4:16 PM, Andrew Sullivan [EMAIL PROTECTED]wrote: On Wed, Aug 20, 2008 at 03:12:43PM +0300, Asko Oja wrote: - If there is nothing that can be done in 8.3 at least warning should be added into the documentation. It will be just one more don't in our long list don'ts for our developers. I am in favour of that change in the 8.3 branch. ERROR: cache lookup failed for function. - Could the plan be marked as invalid so it would fail only once so the next call to the function would get replanned and work again. At least it would be better than losing parts of application for indeterminate time. That seems to me to be a behaviour change, not a bug fix. I agree that the current behaviour is pretty annoying. That is not the same thing as a bug except in the loosest sense. The system works as specified, and therefore it's not a bug. If the specification is wrong, you need a new specification; that's a bug fix that is usually pronounced major release. - Could some less dangerous looking mechanism be added to 8.3 that wouldn't make users not used to PostgreSQL limitations gasp for air when they see the workarounds :) I think it a very bad idea even to suggest that we start undertaking things like adding mechanisms to minor releases, even with smileys at the end of the sentence. I appreciate (possibly more than many hackers) the limitations that are imposed on users by some of the decisions historically taken by developers in some of the previous major releases. But I very strongly agree with Dimitri: the super-conservative approach to maintenance releases that this project takes is a really big benefit to users, and is ultra important in mission critical environments. Otherwise, it becomes practically impossible to get minor releases into production. If you have to worry about the possibility of major changes between minor versions, you will have to treat every release as a major release. I don't think we have sufficient commercial integration support yet that we can follow the lead of the Linux kernel, where the system vendor has the effective obligation to make sure your kernel actually works. In addition, if someone wants to develop back-patches for 8.3 that give it new functionality otherwise planned for 8.4, I see nothing wrong with them doing so. That's the advantage offered by having the source. But the idea that the new functionality should be patched back by the project because one is impatient is not on. A -- Andrew Sullivan [EMAIL PROTECTED] +1 503 667 4564 x104 http://www.commandprompt.com/ -- 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] Patch: plan invalidation vs stored procedures
Alvaro Herrera [EMAIL PROTECTED] writes: This is where the interesting questions are: http://archives.postgresql.org/message-id/10333.1219179364%40sss.pgh.pa.us Upthread, someone speculated about solving the problem by forcing plan cache flush on *any* catalog change. I think that's probably not acceptable from an efficiency standpoint. But maybe it'd be a good idea to special-case common cases and fall back to a stupid flush for less common cases, rather than invest all the work that'd be needed to track every direct and indirect dependency of every plan. My first thought along these lines is: * track table dependencies exactly (important for efficiency, plus we've got the code already) * track function dependencies exactly (seems function definitions might change often enough to make it important for efficiency; maybe only track PL function dependencies??) * brute-force flush for any other catalog change that could affect plans However I have no hard evidence to back up drawing the line there rather than somewhere else. Anyone have data on what sort of DDL changes are common in their applications? 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] Patch: plan invalidation vs stored procedures
David Fetter napsal(a): On Tue, Aug 19, 2008 at 09:50:53PM -0400, Tom Lane wrote: David Fetter [EMAIL PROTECTED] writes: On Tue, Aug 19, 2008 at 07:45:16PM -0400, Tom Lane wrote: FWIW, given that there will probably always be corner cases. I can see the attraction in Simon's suggestion of providing a way to manually issue a system-wide forced plan flush. Would that require a system-wide plan cache to implement? No, just a function that can issue a suitable sinval message. plancache.c would already respond in the desired way to a relcache inval message with OID = 0, though likely it'll be cleaner to invent an sinval message type specifically for the purpose. One thing to think about is whether the flush should be truly system-wide or just database-wide. I can see a lot more uses for the latter than the former --- I don't think there's a reason for cached plans to depend on any contents of the shared catalogs. They might during an on-line upgrade. At this moment we have offline catalog upgrade. On-line old catalog processing is nice idea but amount of work and impact is too high to do it. Catalog is usually small and its offline upgrade is fast. Zdenek -- 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] Patch: plan invalidation vs stored procedures
On Wed, 2008-08-20 at 08:50 -0400, Andrew Dunstan wrote: Asko Oja wrote: I do get the impression that Tom who would prefer to get all the pl's out of PostgreSQL and live happily ever after with pure SQL standard. I have not seen the slightest evidence of this, and don't believe it for a minute. I understand some of the frustration you are feeling, but statements like this don't help anything. Claiming that problems with functions are a corner case seems to indicate that kind of attitude. OTOH, it may still be, that building really large and complex live (evolving) databases using postgreSQL is also still a corner case, so any bug/limitation that manifests itself when doing DDL under 24/7 database carrying big loads is a corner case (And yes, I too have recently been bitten nastily by cached plan problems, and want to see them fixed. I rather like Simon's suggestion of a command or function that would clear the plan cache.) I guess this would be more robust. Mostly we use _dependencies_ to forbid stuff or to do DROP CASCADE, that is, to enforce user-visible behaviour. Cache invalidation seems much lighter and safer operations. We could even add an option to do a global cache invalidation at the end of any transaction which does DDL. That would of course need automatic re-planning the invalidated queries and keeping some intermediate form of query (with original * expanded to col lists, maybe something else, basically the same as is currently saved for view's) in order to do so. - 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] Patch: plan invalidation vs stored procedures
On Wed, Aug 20, 2008 at 05:03:19PM +0300, Asko Oja wrote: Lets get on with 8.4 Oh, I shoulda mentioned that, too -- I completely support doing this work for 8.4. (I can think of more than one case where this feature alone would be worth the upgrade.) A -- Andrew Sullivan [EMAIL PROTECTED] +1 503 667 4564 x104 http://www.commandprompt.com/ -- 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] Patch: plan invalidation vs stored procedures
Hi The reason that this case wasn't covered in 8.3 is that there didn't seem to be a use-case that justified doing the extra work. I still haven't seen one. You just stopped reading the thread where it was discussed after your troll remark? Other than inline-able SQL functions there is no reason to invalidate a stored plan based on the fact that some function it called changed contents. Isn't it reason enough for this patch? ERROR: cache lookup failed for function is normal and good behaviour and should not be recoverd from because it never happen if you PostgreSQL right :) Usecase 1: Inlined functions postgres=# create or replace function salary_without_income_tax(i_salary in numeric, salary out numeric ) returns numeric as $$ select $1 * 0.76 as salary $$ language sql; postgres=# prepare c2 as select salary, salary_without_income_tax(salary) from salaries; postgres=# execute c2; salary | salary_without_income_tax +--- 1 | 7600.00 postgres=# create or replace function salary_without_income_tax(i_salary in numeric, salary out numeric ) returns numeric as $$ select $1 * 0.74 as salary $$ language sql; postgres=# execute c2; salary | salary_without_income_tax +--- 1 | 7600.00 Use case 2: While rewriting existing modules due to changes in business requirements then in addition to new code we have to refactor lots of old functions one natural thing to do would be to get rid of return types as they are even more inconvenient to use than out parameters. Another reason is keep coding style consistent over modules so future maintenace will be less painful in the assholes. postgres=# create type public.ret_status as ( status integer, status_text text); CREATE TYPE postgres=# create or replace function x ( i_param text ) returns public.ret_status as $$ select 200::int, 'ok'::text; $$ language sql; CREATE FUNCTION postgres=# create or replace function x ( i_param text, status OUT int, status_text OUT text ) returns record as $$ select 200::int, 'ok'::text; $$ language sql; ERROR: cannot change return type of existing function HINT: Use DROP FUNCTION first Usecase 3.: Extra out parameters are needed in existing functions. I assure you if you have 5 years of legacy code that is constantly changing it does happen (often). postgres=# create or replace function xl ( i_param text, status OUT int, status_text OUT text, more_text OUT text ) returns record as $$ select 200::int, 'ok'::text, 'cat'::text; $$ language sql; ERROR: cannot change return type of existing function DETAIL: Row type defined by OUT parameters is different. HINT: Use DROP FUNCTION first. Usecase 4: Things are even worse when you need to change the type that is used in functions. You have to drop and recreate the type and all the functions that are using it. Sometimes type is used in several functions and only some of them need changes. postgres=# create type public.ret_status_ext as ( status integer, status_text text, more_money numeric); CREATE TYPE postgres=# create or replace function x ( i_param text ) returns public.ret_status_ext as $$ select 200::int, 'ok'::text; $$ language sql; ERROR: cannot change return type of existing function HINT: Use DROP FUNCTION first. And whenever we do drop and create as hinted then we receive error flood that won't stop until something is manually done to get rid of it postgres=# drop function x(text); DROP FUNCTION postgres=# create or replace function x ( i_param text ) returns public.ret_status_ext as $$ select 200::int, $1, 2.3 $$ language sql; CREATE FUNCTION postgres=# execute c; ERROR: cache lookup failed for function 24598 I hope i have answered your question Why do you not use CREATE OR REPLACE FUNCTION? That leaves us to deal with functions in our usual bad, wrong and stupid ways. * We create a function with new name and redirect all the calls to it. (stupid as it creates extra development, testing, code reviewing and releasing work and leaves around old code). * We pause pgBouncer and after release let it reconnect all connections (bad as it creates downtime). * We invalidate all procedures using update to pg_proc (simply wrong way to do it but still our best workaround short of restarting postgres). postgres=# update pg_proc set proname = proname; UPDATE 2152 postgres=# execute c2; salary | salary_without_income_tax +--- 1 | 7400.00 Perhaps Skype needs to rethink how they are modifying functions. We have had to change the way we use functions to suit PostgreSQL for 5 years now. That creates us quite a lot of extra work both on development side and DBA side plus the constantly hanging danger of downtime. Our DBA teams job is to reduce all possible causes for downtime and this patch is solution to one of them. Sadly we just get trolled into the ground :) All in all it's not the job of PostgreSQL to tell the
Re: [HACKERS] Patch: plan invalidation vs stored procedures
On Mon, 2008-08-18 at 22:41 +0200, Pavel Stehule wrote: 2008/8/18 Hannu Krosing [EMAIL PROTECTED]: On Mon, 2008-08-18 at 11:05 +0200, Pavel Stehule wrote: 2008/8/18 Dimitri Fontaine [EMAIL PROTECTED]: Hi, Le lundi 18 août 2008, Andrew Dunstan a écrit : On Sat, Aug 16, 2008 at 09:40:19PM -0400, Tom Lane wrote: This is not the kind of patch we put into stable branches. So what? That is not the only criterion for backpatching. I fail to understand why this problem is not qualified as a bug. Does it change of result some queries? Not in the long run, but not invalidating the functions (current behaviour) postpones seeing the results of function change until DBA manually restarts the error-producing client. It is protection to server's hang? Can't understand this question :( If you mean, does the change protect against hanging the server, then no, currently the server does not actually hang, it just becomes unusable until reconnect :( Hi I am sorry, but it's really new feature and not bug fix Could you please explain why you think so ? As I see it, the patch does not change visible behaviour, except removing some sonditions where client becomes unusable after some other backend does some legitimate changes. Is the current behavior planned or even defined by spec ? I agree, that the bug (if it is a bug) could also be circumvented by the calling function by detecting a failed cache lookup and doing replan/requery itself, but this would require all PL implementations and other functions with stored plans to do it independently. - 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] Patch: plan invalidation vs stored procedures
On Mon, 2008-08-18 at 20:29 -0400, Tom Lane wrote: Asko Oja [EMAIL PROTECTED] writes: For users of stored procedures it is protection from downtime. For Skype it has been around 20% of databse related downtime this year. Perhaps Skype needs to rethink how they are modifying functions. Why not suggest they just should stop using functions and move all business logic into client or 3rd tier ? (Actually I would not recommend that as functions are very good way to abstract database access AND provide better security AND speed up queries) The reason that this case wasn't covered in 8.3 is that there didn't seem to be a use-case that justified doing the extra work. I still haven't seen one. Other than inline-able SQL functions there is no reason to invalidate a stored plan based on the fact that some function it called changed contents. Maybe there should be something in postgreSQL docs that warns users against using functions in any non-trivial circumstances, as functions are not expected to behave like the rest of postgreSQL features and there is not plan to fix that ? 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] Patch: plan invalidation vs stored procedures
2008/8/19 Hannu Krosing [EMAIL PROTECTED]: On Mon, 2008-08-18 at 22:41 +0200, Pavel Stehule wrote: 2008/8/18 Hannu Krosing [EMAIL PROTECTED]: On Mon, 2008-08-18 at 11:05 +0200, Pavel Stehule wrote: 2008/8/18 Dimitri Fontaine [EMAIL PROTECTED]: Hi, Le lundi 18 août 2008, Andrew Dunstan a écrit : On Sat, Aug 16, 2008 at 09:40:19PM -0400, Tom Lane wrote: This is not the kind of patch we put into stable branches. So what? That is not the only criterion for backpatching. I fail to understand why this problem is not qualified as a bug. Does it change of result some queries? Not in the long run, but not invalidating the functions (current behaviour) postpones seeing the results of function change until DBA manually restarts the error-producing client. It is protection to server's hang? Can't understand this question :( If you mean, does the change protect against hanging the server, then no, currently the server does not actually hang, it just becomes unusable until reconnect :( Hi I am sorry, but it's really new feature and not bug fix Could you please explain why you think so ? As I see it, the patch does not change visible behaviour, except removing some sonditions where client becomes unusable after some other backend does some legitimate changes. Are you sure, so this behave hasn't any secondary effect? So this change doesn't breaks any application? Pavel Is the current behavior planned or even defined by spec ? I agree, that the bug (if it is a bug) could also be circumvented by the calling function by detecting a failed cache lookup and doing replan/requery itself, but this would require all PL implementations and other functions with stored plans to do it independently. - 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] Patch: plan invalidation vs stored procedures
2008/8/19 Hannu Krosing [EMAIL PROTECTED]: On Mon, 2008-08-18 at 22:41 +0200, Pavel Stehule wrote: 2008/8/18 Hannu Krosing [EMAIL PROTECTED]: On Mon, 2008-08-18 at 11:05 +0200, Pavel Stehule wrote: 2008/8/18 Dimitri Fontaine [EMAIL PROTECTED]: Hi, Le lundi 18 août 2008, Andrew Dunstan a écrit : On Sat, Aug 16, 2008 at 09:40:19PM -0400, Tom Lane wrote: This is not the kind of patch we put into stable branches. So what? That is not the only criterion for backpatching. I fail to understand why this problem is not qualified as a bug. Does it change of result some queries? Not in the long run, but not invalidating the functions (current behaviour) postpones seeing the results of function change until DBA manually restarts the error-producing client. It is protection to server's hang? Can't understand this question :( If you mean, does the change protect against hanging the server, then no, currently the server does not actually hang, it just becomes unusable until reconnect :( Hi I am sorry, but it's really new feature and not bug fix Could you please explain why you think so ? As I see it, the patch does not change visible behaviour, except removing some sonditions where client becomes unusable after some other backend does some legitimate changes. Is the current behavior planned or even defined by spec ? I agree, that the bug (if it is a bug) could also be circumvented by the calling function by detecting a failed cache lookup and doing replan/requery itself, but this would require all PL implementations and other functions with stored plans to do it independently. I am not against to this patch or this feature. But I am sure, so isn't well to do not necessary changes in stable version. 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] Patch: plan invalidation vs stored procedures
On Tue, 2008-08-19 at 12:42 +0200, Pavel Stehule wrote: 2008/8/19 Hannu Krosing [EMAIL PROTECTED]: On Mon, 2008-08-18 at 22:41 +0200, Pavel Stehule wrote: 2008/8/18 Hannu Krosing [EMAIL PROTECTED]: On Mon, 2008-08-18 at 11:05 +0200, Pavel Stehule wrote: 2008/8/18 Dimitri Fontaine [EMAIL PROTECTED]: Hi, Le lundi 18 août 2008, Andrew Dunstan a écrit : On Sat, Aug 16, 2008 at 09:40:19PM -0400, Tom Lane wrote: This is not the kind of patch we put into stable branches. So what? That is not the only criterion for backpatching. I fail to understand why this problem is not qualified as a bug. Does it change of result some queries? Not in the long run, but not invalidating the functions (current behaviour) postpones seeing the results of function change until DBA manually restarts the error-producing client. It is protection to server's hang? Can't understand this question :( If you mean, does the change protect against hanging the server, then no, currently the server does not actually hang, it just becomes unusable until reconnect :( Hi I am sorry, but it's really new feature and not bug fix Could you please explain why you think so ? As I see it, the patch does not change visible behaviour, except removing some sonditions where client becomes unusable after some other backend does some legitimate changes. Are you sure, so this behave hasn't any secondary effect? So this change doesn't breaks any application? I can't think of any. What it does, is it makes the changed function usable right after redefining the new function. Current behaviour is to make the calling function unusable until the backend is restarted, after which it still will use the new version of the function. Pavel Is the current behavior planned or even defined by spec ? I agree, that the bug (if it is a bug) could also be circumvented by the calling function by detecting a failed cache lookup and doing replan/requery itself, but this would require all PL implementations and other functions with stored plans to do it independently. - 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] Patch: plan invalidation vs stored procedures
Hannu Krosing [EMAIL PROTECTED] writes: Maybe there should be something in postgreSQL docs that warns users against using functions in any non-trivial circumstances, as functions are not expected to behave like the rest of postgreSQL features and there is not plan to fix that ? Now who's trolling :) -- Gregory Stark EnterpriseDB http://www.enterprisedb.com Get trained by Bruce Momjian - ask me about EnterpriseDB's PostgreSQL training! -- 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] Patch: plan invalidation vs stored procedures
Hannu Krosing [EMAIL PROTECTED] writes: On Mon, 2008-08-18 at 22:41 +0200, Pavel Stehule wrote: I am sorry, but it's really new feature and not bug fix Could you please explain why you think so ? For the same reasons that plan invalidation itself was a new feature and not a bug fix; notably, risk vs reward tradeoff and not wanting to change long-established behavior in stable branches. 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] Patch: plan invalidation vs stored procedures
Le mardi 19 août 2008, Tom Lane a écrit : For the same reasons that plan invalidation itself was a new feature and not a bug fix; I'm sorry but that doesn't help me a dime to understand current situation. It could well be just me, but... here's how I see it: - plan invalidation is a new feature in 8.3 - previous releases are out of business: new feature against stable code - now, we have found a bug in plan invalidation - HEAD (to be 8.4) will get some new code to fix it But 8.3 won't benefit from this bugfix? On the grounds that the feature which is now deployed on the field should *maybe* not get used the way it *is*? Sorry again, I really don't get it. -- dim signature.asc Description: This is a digitally signed message part.
Re: [HACKERS] Patch: plan invalidation vs stored procedures
On Tue, 19 Aug 2008 12:48:06 +0100 Gregory Stark [EMAIL PROTECTED] wrote: Hannu Krosing [EMAIL PROTECTED] writes: Maybe there should be something in postgreSQL docs that warns users against using functions in any non-trivial circumstances, as functions are not expected to behave like the rest of postgreSQL features and there is not plan to fix that ? Now who's trolling :) Although I read his remark as sarcastic after reading the entire thread I have to say it may be a good idea to have the something in the docs about the limitation. I never think about it anymore because I am used to the behavior. I can see where and entity like skype who has I am sure thousands of procedures would have this as a constant irritant. Do I think it should be pushed back to 8.3.x; no. It is a feature. I don't consider the existing behavior a bug. I consider it a limitation and we don't back patch fixes for limitations. Sincerely, Joshua D. Drake -- The PostgreSQL Company since 1997: http://www.commandprompt.com/ PostgreSQL Community Conference: http://www.postgresqlconference.org/ United States PostgreSQL Association: http://www.postgresql.us/ Donate to the PostgreSQL Project: http://www.postgresql.org/about/donate -- 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] Patch: plan invalidation vs stored procedures
Dimitri Fontaine [EMAIL PROTECTED] writes: - now, we have found a bug in plan invalidation [ shrug... ] You have not found a bug in plan invalidation. You have found omitted functionality --- functionality that was *intentionally* omitted from the 8.3 version. 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] Patch: plan invalidation vs stored procedures
Le mardi 19 août 2008, Tom Lane a écrit : [ shrug... ] You have not found a bug in plan invalidation. You have found omitted functionality --- functionality that was *intentionally* omitted from the 8.3 version. Thanks a lot for this clarification, now I understand you viewpoint. So, the 8.3 fix would be about documenting this intentionnal omit in the great manual, maybe in a Limits section of the sql-createfunction page? Another thing I do not understand well is how people are expected to work in 8.3 with a function based API, without hitting Skype problems. I'm having a project here where the project manager wants a database function API to keep data logic at serverside, should I tell him to reconsider this while 8.4 is not ready? We would then have to go live with an 8.3 based solution containing middleware code, then port it again to SQL functions when 8.4 is out stable. Not appealing, but I sure understand the no new feature in stable code base argument here. Regards, -- dim signature.asc Description: This is a digitally signed message part.
Re: [HACKERS] Patch: plan invalidation vs stored procedures
Dimitri Fontaine [EMAIL PROTECTED] writes: Another thing I do not understand well is how people are expected to work in 8.3 with a function based API, without hitting Skype problems. I could understand this level of complaining if this were a new problem that'd appeared in 8.3. But *every PG version that we've ever released* behaves the same way with respect to function drop/recreate. If the Skype folk have developed a way of working that is guaranteed not to work with any released version, one has to wonder what they were thinking. If you need to DROP rather than CREATE OR REPLACE functions, then 8.3 doesn't make things better for you than prior releases did, but it does't make them worse either. Making things better for that case is unequivocally a new feature. And it's rather a corner case at that, else there would have been enough prior complaints to put it on the radar screen for 8.3. What we've got at this point is a submitted patch for a new feature that hasn't even been accepted into HEAD yet. Lobbying to get it back-patched is entirely inappropriate IMHO. 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] Patch: plan invalidation vs stored procedures
Polite answers lead to polite discussions. Caling other people names lead to flame wars. It's perfectly ok for Skype to keep our own build of 8.3 with given patch and make it available for whoever might want it. At least now there is almost good enough description why the patch was needed althou it would have been more pleasant if the discussion had been constructive. We didn't keep close enough watch on the list when 8.3 plan invalidation was discussed and it came as bad surprise to us that some parts important to us were left out. By the way it's real nice what you are doing with in and exists improvements. Thanks. regards Asko On Tue, Aug 19, 2008 at 8:06 PM, Tom Lane [EMAIL PROTECTED] wrote: Dimitri Fontaine [EMAIL PROTECTED] writes: Another thing I do not understand well is how people are expected to work in 8.3 with a function based API, without hitting Skype problems. I could understand this level of complaining if this were a new problem that'd appeared in 8.3. But *every PG version that we've ever released* behaves the same way with respect to function drop/recreate. If the Skype folk have developed a way of working that is guaranteed not to work with any released version, one has to wonder what they were thinking. If you need to DROP rather than CREATE OR REPLACE functions, then 8.3 doesn't make things better for you than prior releases did, but it does't make them worse either. Making things better for that case is unequivocally a new feature. And it's rather a corner case at that, else there would have been enough prior complaints to put it on the radar screen for 8.3. What we've got at this point is a submitted patch for a new feature that hasn't even been accepted into HEAD yet. Lobbying to get it back-patched is entirely inappropriate IMHO. 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] Patch: plan invalidation vs stored procedures
2008/8/19 Dimitri Fontaine [EMAIL PROTECTED]: Le mardi 19 août 2008, Tom Lane a écrit : [ shrug... ] You have not found a bug in plan invalidation. You have found omitted functionality --- functionality that was *intentionally* omitted from the 8.3 version. Thanks a lot for this clarification, now I understand you viewpoint. So, the 8.3 fix would be about documenting this intentionnal omit in the great manual, maybe in a Limits section of the sql-createfunction page? Another thing I do not understand well is how people are expected to work in 8.3 with a function based API, without hitting Skype problems. I'm having a project here where the project manager wants a database function API to keep data logic at serverside, should I tell him to reconsider this while 8.4 is not ready? You could to use patched 8.3. We would then have to go live with an 8.3 based solution containing middleware code, then port it again to SQL functions when 8.4 is out stable. Not appealing, but I sure understand the no new feature in stable code base argument here. This problem isn't too hard without pooling. Not all systems are global - so usually is possible to find some window and recreate functions and close all user connections. Regards Pavel Stehule Regards, -- dim -- 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] Patch: plan invalidation vs stored procedures
Another thing I do not understand well is how people are expected to work in 8.3 with a function based API, without hitting Skype problems. People are expected to use same workarounds as Skype is using. For us another unneccessary downtime week ago was what set us moving/thinking :). When you use software with limitations then you learn to live with them. Good thing about postgres you can do something yourself to get some of the limitations removed. As Pavel said you are probably using your own build anyway so one more patch should not be a problem. regards Asko On Tue, Aug 19, 2008 at 8:48 PM, Pavel Stehule [EMAIL PROTECTED]wrote: 2008/8/19 Dimitri Fontaine [EMAIL PROTECTED]: Le mardi 19 août 2008, Tom Lane a écrit : [ shrug... ] You have not found a bug in plan invalidation. You have found omitted functionality --- functionality that was *intentionally* omitted from the 8.3 version. Thanks a lot for this clarification, now I understand you viewpoint. So, the 8.3 fix would be about documenting this intentionnal omit in the great manual, maybe in a Limits section of the sql-createfunction page? Another thing I do not understand well is how people are expected to work in 8.3 with a function based API, without hitting Skype problems. I'm having a project here where the project manager wants a database function API to keep data logic at serverside, should I tell him to reconsider this while 8.4 is not ready? You could to use patched 8.3. We would then have to go live with an 8.3 based solution containing middleware code, then port it again to SQL functions when 8.4 is out stable. Not appealing, but I sure understand the no new feature in stable code base argument here. This problem isn't too hard without pooling. Not all systems are global - so usually is possible to find some window and recreate functions and close all user connections. Regards Pavel Stehule Regards, -- dim -- 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] Patch: plan invalidation vs stored procedures
Joshua Drake wrote: On Tue, 19 Aug 2008 12:48:06 +0100 Gregory Stark [EMAIL PROTECTED] wrote: Hannu Krosing [EMAIL PROTECTED] writes: Maybe there should be something in postgreSQL docs that warns users against using functions in any non-trivial circumstances, as functions are not expected to behave like the rest of postgreSQL features and there is not plan to fix that ? Now who's trolling :) Although I read his remark as sarcastic after reading the entire thread I have to say it may be a good idea to have the something in the docs about the limitation. I never think about it anymore because I am used to the behavior. I can see where and entity like skype who has I am sure thousands of procedures would have this as a constant irritant. Do I think it should be pushed back to 8.3.x; no. It is a feature. I don't consider the existing behavior a bug. I consider it a limitation and we don't back patch fixes for limitations. The bottom line here is that we don't have the time to explain or justify our backpatch policy every time someone shows up with a bug that needs to be fixed. If you want to create your own version of Postgres, go ahead; no one is stopping you. But if we backpatched everything and we introduced bugs or change behavior, more people would complain. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- 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] Patch: plan invalidation vs stored procedures
On Tue, 19 Aug 2008 14:29:52 -0400 (EDT) Bruce Momjian [EMAIL PROTECTED] wrote: Do I think it should be pushed back to 8.3.x; no. It is a feature. I don't consider the existing behavior a bug. I consider it a limitation and we don't back patch fixes for limitations. The bottom line here is that we don't have the time to explain or justify our backpatch policy every time someone shows up with a bug that needs to be fixed. Is our backpatch policy documented? It does not appear to be in developer FAQ. Joshua D. Drake -- The PostgreSQL Company since 1997: http://www.commandprompt.com/ PostgreSQL Community Conference: http://www.postgresqlconference.org/ United States PostgreSQL Association: http://www.postgresql.us/ Donate to the PostgreSQL Project: http://www.postgresql.org/about/donate -- 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] Patch: plan invalidation vs stored procedures
Joshua Drake wrote: On Tue, 19 Aug 2008 14:29:52 -0400 (EDT) Bruce Momjian [EMAIL PROTECTED] wrote: Do I think it should be pushed back to 8.3.x; no. It is a feature. I don't consider the existing behavior a bug. I consider it a limitation and we don't back patch fixes for limitations. The bottom line here is that we don't have the time to explain or justify our backpatch policy every time someone shows up with a bug that needs to be fixed. Is our backpatch policy documented? It does not appear to be in developer FAQ. Seems we need to add it. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- 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] Patch: plan invalidation vs stored procedures
Joshua Drake [EMAIL PROTECTED] wrote: Is our backpatch policy documented? It does not appear to be in developer FAQ. It's mentioned here: http://www.postgresql.org/support/versioning PostgreSQL minor releases fix only frequently-encountered, security, and data corruption bugs to reduce the risk of upgrading. -Kevin -- 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] Patch: plan invalidation vs stored procedures
Bruce Momjian [EMAIL PROTECTED] writes: Joshua Drake wrote: Is our backpatch policy documented? It does not appear to be in developer FAQ. Seems we need to add it. I'm not sure that I *want* a formal written-down backpatch policy. Whether (and how far) to backpatch has always been a best-judgment call in the past, and we've gotten along fine with that. I think having a formal policy is just likely to lead to even more complaints: either patching or not patching could result in second-guessing by someone who feels he can construe the policy to match the result he prefers. 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] Patch: plan invalidation vs stored procedures
Tom Lane wrote: Bruce Momjian [EMAIL PROTECTED] writes: Joshua Drake wrote: Is our backpatch policy documented? It does not appear to be in developer FAQ. Seems we need to add it. I'm not sure that I *want* a formal written-down backpatch policy. Whether (and how far) to backpatch has always been a best-judgment call in the past, and we've gotten along fine with that. I think having a formal policy is just likely to lead to even more complaints: either patching or not patching could result in second-guessing by someone who feels he can construe the policy to match the result he prefers. OK, agreed. -- Bruce Momjian [EMAIL PROTECTED]http://momjian.us EnterpriseDB http://enterprisedb.com + If your life is a hard drive, Christ can be your backup. + -- 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] Patch: plan invalidation vs stored procedures
Asko Oja escribió: Another thing I do not understand well is how people are expected to work in 8.3 with a function based API, without hitting Skype problems. People are expected to use same workarounds as Skype is using. For us another unneccessary downtime week ago was what set us moving/thinking :). When you use software with limitations then you learn to live with them. Good thing about postgres you can do something yourself to get some of the limitations removed. Make sure you do not live with patches forever, i.e. that it gets into 8.4. Otherwise it's going to be a pain for everyone. -- Alvaro Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. -- 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] Patch: plan invalidation vs stored procedures
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Hi, Le 19 août 08 à 19:06, Tom Lane a écrit : Dimitri Fontaine [EMAIL PROTECTED] writes: Another thing I do not understand well is how people are expected to work in 8.3 with a function based API, without hitting Skype problems. What we've got at this point is a submitted patch for a new feature that hasn't even been accepted into HEAD yet. Lobbying to get it back-patched is entirely inappropriate IMHO. Well, there's a misunderstanding here. I certainly were lobbying for considering a backpatch as I saw it as a bugfix. You told me it's a new feature, I say ok for not backpatching, obviously. This mail was a real attempt at learning some tips to be able to push the functions usage as far as Skype is doing, in 8.3 release, and avoiding the trap which has always existed in released PostgreSQL version. This certainly was a bad attempt at it. Now, my understanding is that rolling out new versions of functions requires forcing dropping all current opened sessions as soon as PostgreSQL considers you need to drop any function. I'll think about it in next project design meetings. Regards, - -- dim -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.9 (Darwin) iEYEARECAAYFAkirHlEACgkQlBXRlnbh1bk4YQCgswDS1bu+P+N7yKJvwnRAWnL3 FYkAnRZQzqbEoahShh/Qz9mnrIm1e99y =hIBt -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] Patch: plan invalidation vs stored procedures
Le 19 août 08 à 20:47, Tom Lane a écrit : I'm not sure that I *want* a formal written-down backpatch policy. Whether (and how far) to backpatch has always been a best-judgment call in the past, and we've gotten along fine with that. I think having a formal policy is just likely to lead to even more complaints: either patching or not patching could result in second-guessing by someone who feels he can construe the policy to match the result he prefers. Agreed. The problem here (at least for me) was to understand why this (yet to be reviewed) patch is about implementing a new feature and not about bugfixing an existing one. So we're exactly in the fog around the informal backpatch policy, and as long as we're able to continue talking nicely about it, this seems the finest solution :) Keep up the amazing work, regards, -- dim -- 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] Patch: plan invalidation vs stored procedures
On Tue, 19 Aug 2008 14:47:13 -0400 Tom Lane [EMAIL PROTECTED] wrote: Bruce Momjian [EMAIL PROTECTED] writes: Joshua Drake wrote: Is our backpatch policy documented? It does not appear to be in developer FAQ. Seems we need to add it. I'm not sure that I *want* a formal written-down backpatch policy. Then we write a formal guideline. It really isn't fair to new developers to not have any idea how they are going to be able to get a patch applied to older branches. Something like: Generally speaking we adhere to the following guideline for patches. * Security fixes are applied to all applicable branches. * Bugfixes are applied to all applicable branches * Note: A patch that addresses a known limitation is generally not backpatched * New features are always applied to -HEAD only. This is not a policy as much as a legend for developers to consider before they submit their patch. If we do this, we have the opportunity to just point to the FAQ when there is no ambiguity. It also increases transparency of the process; which is always a good thing. Sincerely, Joshua D. Drake -- The PostgreSQL Company since 1997: http://www.commandprompt.com/ PostgreSQL Community Conference: http://www.postgresqlconference.org/ United States PostgreSQL Association: http://www.postgresql.us/ Donate to the PostgreSQL Project: http://www.postgresql.org/about/donate -- 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] Patch: plan invalidation vs stored procedures
On Tue, Aug 19, 2008 at 02:47:13PM -0400, Tom Lane wrote: Whether (and how far) to backpatch has always been a best-judgment call in the past, and we've gotten along fine with that. I think having a formal policy is just likely to lead to even more complaints: I completely agree with this. If you formalise the back-patch policy, then it will be necessary to invent classifications for bug severity to determine whether to back patch. This will inevitably lead to some sort of false objectivity measure, where bugs get a severity number that actually just means we have already decided to back-patch. A -- Andrew Sullivan [EMAIL PROTECTED] +1 503 667 4564 x104 http://www.commandprompt.com/ -- 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] Patch: plan invalidation vs stored procedures
Another thing I do not understand well is how people are expected to work in 8.3 with a function based API, without hitting Skype problems. I'm having a All database-driven applications have this problem. Any time you have a database on the backend and interface code on the front-end, you need to keep in mind that it won't necessarily be possible to update the two of them simultaneously, especially if you have multiple back-ends and multiple front-ends, as you almost certainly do. Even if PostgreSQL invalidated plans in the particular situation you're discussing, there would still be other problems. You could add a new, non-NULLable column to a table before updating the code that insert into that table, or drop an old column that the code still counts on being able to access. I handle these problems all the time by ordering the changes carefully. If I need to change a function API in an incompatible way, I change the NAME of the function as well as the type signature (eg. do_important_thing - do_important_thing_v2). Then I change the code. Then I remove the old function once everything that relies on it is dead. Maybe in your particular environment plan invalidation for functions will solve most of the cases you care about, but I respectfully submit that there's no substitute for good release engineering. If you don't know exactly what functions are going to be created, modified, or dropped on your production servers during each release before you actually roll that release out... you probably need to improve your internal documentation. ...Robert -- 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] Patch: plan invalidation vs stored procedures
-BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Hi, Le 19 août 08 à 22:03, Robert Haas a écrit : All database-driven applications have this problem. Any time you have a database on the backend and interface code on the front-end, you need to keep in mind that it won't necessarily be possible to update the two of them simultaneously, especially if you have multiple back-ends and multiple front-ends, as you almost certainly do. Even if PostgreSQL invalidated plans in the particular situation you're discussing, there would still be other problems. You could add a new, non-NULLable column to a table before updating the code that insert into that table, or drop an old column that the code still counts on being able to access. Using functions the way Skype uses them means not issuing a single insert, update or delete directly from your code, but calling a function which takes care about it. So you use PostgreSQL transactionnal DDL to roll-out new function versions at the same time you push the schema modifications, and commit it all in one go. Maybe in your particular environment plan invalidation for functions will solve most of the cases you care about When the code only is a client to an SQL functions API, which effectively replaces SQL as the way to interact between code and database, then I believe plan invalidation at function change is the missing piece. , but I respectfully submit that there's no substitute for good release engineering. If you don't know exactly what functions are going to be created, modified, or dropped on your production servers during each release before you actually roll that release out... you probably need to improve your internal documentation. Agreed :) - -- dim -BEGIN PGP SIGNATURE- Version: GnuPG v1.4.9 (Darwin) iEYEARECAAYFAkirK2kACgkQlBXRlnbh1bmxvQCgmowpfnZ5nFRml0mNfj2HRE+3 HJEAnR3G6Lhnb7R4+iSze8xGACwyk4D7 =of1o -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] Patch: plan invalidation vs stored procedures
On Tue, Aug 19, 2008 at 12:42:29PM -0700, Joshua Drake wrote: Generally speaking we adhere to the following guideline for patches. * Security fixes are applied to all applicable branches. * Bugfixes are applied to all applicable branches * Note: A patch that addresses a known limitation is generally not backpatched * New features are always applied to -HEAD only. This is not a policy as much as a legend for developers to consider before they submit their patch. But it's meaningless. Bugfixes are applied to all applicable branches, is either false or trivially true. It's trivially true if you interpret applicable branches to mean the ones that get the patch. It's false if you mean bugfix to mean every patch that fixes a bug. I can think of bugs that we have lived with in older releases because fixing them was too risky or because the bug was so tiny or unusual as to make the risk greater than the reward. A formal policy that's any more detailed than what's in the FAQ today is a solution in search of a problem. A -- Andrew Sullivan [EMAIL PROTECTED] +1 503 667 4564 x104 http://www.commandprompt.com/ -- 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] Patch: plan invalidation vs stored procedures
On Tue, 2008-08-19 at 21:26 +0200, Dimitri Fontaine wrote: -BEGIN PGP SIGNED MESSAGE- Hash: SHA1 Hi, Le 19 août 08 à 19:06, Tom Lane a écrit : Dimitri Fontaine [EMAIL PROTECTED] writes: Another thing I do not understand well is how people are expected to work in 8.3 with a function based API, without hitting Skype problems. What we've got at this point is a submitted patch for a new feature that hasn't even been accepted into HEAD yet. Lobbying to get it back-patched is entirely inappropriate IMHO. Well, there's a misunderstanding here. I certainly were lobbying for considering a backpatch as I saw it as a bugfix. You told me it's a new feature, I say ok for not backpatching, obviously. This mail was a real attempt at learning some tips to be able to push the functions usage as far as Skype is doing, in 8.3 release, and avoiding the trap which has always existed in released PostgreSQL version. This certainly was a bad attempt at it. Now, my understanding is that rolling out new versions of functions requires forcing dropping all current opened sessions as soon as PostgreSQL considers you need to drop any function. I'll think about it in next project design meetings. I think that another option is to manipulate pg_proc - just do a no-op update to advance xmin for all functions that may have cached plans. UPDATE pg_proc SET proname = proname; then make sure that pg_proc is vacuumed often enough. It's a bit wasteful, as it forces re-planning of all functions, but should have similar effect than the patch. It's also possible that updating pg_proc in bulk introduces some race conditions which lock up the database. -- 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] Patch: plan invalidation vs stored procedures
On Tue, 19 Aug 2008 16:22:43 -0400 Andrew Sullivan [EMAIL PROTECTED] wrote: A formal policy that's any more detailed than what's in the FAQ today is a solution in search of a problem. Odd that the problem continues to rear its head though isn't it? This certainly isn't the first time it has come up. I have however made my argument. I also tried to help solve the problem. If we aren't interested in a solution, oh well. It doesn't make my life any harder. Sincerely, Joshua D. Drake -- The PostgreSQL Company since 1997: http://www.commandprompt.com/ PostgreSQL Community Conference: http://www.postgresqlconference.org/ United States PostgreSQL Association: http://www.postgresql.us/ Donate to the PostgreSQL Project: http://www.postgresql.org/about/donate -- 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] Patch: plan invalidation vs stored procedures
Dimitri Fontaine escribió: The problem here (at least for me) was to understand why this (yet to be reviewed) patch is about implementing a new feature and not about bugfixing an existing one. So we're exactly in the fog around the informal backpatch policy, and as long as we're able to continue talking nicely about it, this seems the finest solution :) The actual criterion is not really new user-visible feature versus bug fix. It's more an attempt at measuring how large a potential impact the change has. The patch I saw was introducing a whole new message type to go through the shared invalidation queue, which is not something to be taken lightly (consider that there are three message types of messages currently.) It's possible that for the Skype usage this patch introduces the behavior they want. But for other people, perhaps this kind of invalidation causes secondary effects that are completely unforeseen -- what if it breaks their apps and they must carry out a week's work to fix it? What if a serious security problem is discovered tomorrow and they can't update because we've broken backwards compatibility for them? -- Alvaro Herrerahttp://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. -- 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] Patch: plan invalidation vs stored procedures
Alvaro Herrera [EMAIL PROTECTED] writes: The actual criterion is not really new user-visible feature versus bug fix. It's more an attempt at measuring how large a potential impact the change has. The patch I saw was introducing a whole new message type to go through the shared invalidation queue, which is not something to be taken lightly (consider that there are three message types of messages currently.) I hadn't read it yet, but that makes it wrong already. There's no need for any new inval traffic --- the existing syscache inval messages on pg_proc entries should serve fine. More generally, if we are to try to invalidate on the strength of pg_proc changes, what of other DDL changes? Operators, operator classes, maybe? How about renaming a schema? I would like to see a line drawn between things we find worth trying to track and things we don't. If there is no such line, we're going to need a patch a lot larger than this one. 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] Patch: plan invalidation vs stored procedures
On Tue, 2008-08-19 at 16:03 -0400, Robert Haas wrote: Another thing I do not understand well is how people are expected to work in 8.3 with a function based API, without hitting Skype problems. I'm having a All database-driven applications have this problem. Any time you have a database on the backend and interface code on the front-end, you need to keep in mind that it won't necessarily be possible to update the two of them simultaneously, especially if you have multiple back-ends and multiple front-ends, as you almost certainly do. Even if PostgreSQL invalidated plans in the particular situation you're discussing, there would still be other problems. You could add a new, non-NULLable column to a table before updating the code that insert into that table, or drop an old column that the code still counts on being able to access. I handle these problems all the time by ordering the changes carefully. If I need to change a function API in an incompatible way, I change the NAME of the function as well as the type signature (eg. do_important_thing - do_important_thing_v2). Then I change the code. Then I remove the old function once everything that relies on it is dead. Not having plan invalidation forces you to have do_important_thing_v2 for do_important_thing even with no changes in source code, just for the fact that do_part_of_important_thing() which it calls has changed. An example - you have functions A) caller1() to callerN() which includes call to called1() B) one of these functions, say callerM() needs one more field returned from called1(), so you either write a completely new function called1_v2() with one more field and then update callerM() to call called1_v2() C) now, to get rid of called1() you have to replace called1 with called1_v2 also in all other functions caller1() to callerN() D) then you can drop called1() if you missed one of callerx() functions (you can drop called1() even if it is used, as postgreSQL does not check dependencies in functions) then you have a non-functioning database, where even client reconnect won't help, only putting called1() back. If there is plan invalidation then you just change called1() to return one more field and that's it - no juggling with C) and D) and generally less things that can go wrong. Maybe in your particular environment plan invalidation for functions will solve most of the cases you care about, but I respectfully submit that there's no substitute for good release engineering. Nope, but the amount of release engineering (and deployment-time work) you need to do depends a lot on fragility of the system. The more arcane and fragile the system is, the more you need to rely on external systems and procedures to keep it working. Imagine how much harder it would be, if there were no transactions and you had to ensure the right ordering of all changes by release process only. You probably would end up doing several times more work and temporary hacks and you would still be out of luck doing _any_ nontrivial updates while the systems are running 24/7. If you don't know exactly what functions are going to be created, modified, or dropped on your production servers during each release before you actually roll that release out... this is not about knowing this at all - this is about needing to change less, about optimizing on work that does not need to be done if system is smarter. you probably need to improve your internal documentation. or improve the database system you use. if you need to change less functions, you also need less documentation about changes. if you can prove that select a,b from f() always returns the same data as select a,b from f_b2() then you don't need to write f_b2() at all, you just redefine f() and can also skip migrating all callers of f() to f_v2() just to satisfy your databases quirks. --- 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] Patch: plan invalidation vs stored procedures
On Tue, 2008-08-19 at 16:56 -0400, Tom Lane wrote: Alvaro Herrera [EMAIL PROTECTED] writes: The actual criterion is not really new user-visible feature versus bug fix. It's more an attempt at measuring how large a potential impact the change has. The patch I saw was introducing a whole new message type to go through the shared invalidation queue, which is not something to be taken lightly (consider that there are three message types of messages currently.) I hadn't read it yet, but that makes it wrong already. There's no need for any new inval traffic --- the existing syscache inval messages on pg_proc entries should serve fine. More generally, if we are to try to invalidate on the strength of pg_proc changes, what of other DDL changes? Operators, operator classes, maybe? How about renaming a schema? I would like to see a line drawn between things we find worth trying to track and things we don't. If there is no such line, we're going to need a patch a lot larger than this one. Or maybe a simpler and smaller patch - just invalidate everything on every schema change :) It will have a momentary impact on performance at DDL time, but otherways might be more robust and easier to check for errors. - 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] Patch: plan invalidation vs stored procedures
On Tue, 2008-08-19 at 16:56 -0400, Tom Lane wrote: Alvaro Herrera [EMAIL PROTECTED] writes: The actual criterion is not really new user-visible feature versus bug fix. It's more an attempt at measuring how large a potential impact the change has. The patch I saw was introducing a whole new message type to go through the shared invalidation queue, which is not something to be taken lightly (consider that there are three message types of messages currently.) I hadn't read it yet, but that makes it wrong already. There's no need for any new inval traffic --- the existing syscache inval messages on pg_proc entries should serve fine. I have'nt looke at the patch either, but I suspect that what goes through shared mem is the registration for invalidation, as dependent function OIDs are only learned while compiling functions so when f_caller() learns that it caches plan f_called() then it registers through shared mem message its wish to invalidate this plan if f_called() is dropped or redefined. -- 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] Patch: plan invalidation vs stored procedures
you have functions A) caller1() to callerN() which includes call to called1() B) one of these functions, say callerM() needs one more field returned from called1(), so you either write a completely new function called1_v2() with one more field and then update callerM() to call called1_v2() C) now, to get rid of called1() you have to replace called1 with called1_v2 also in all other functions caller1() to callerN() D) then you can drop called1() True. I complained about this same problem in the context of views - you can add a column to a table in place but not to a view, or even a type created via CREATE TYPE. I even went so far as to develop a patch[1] to improve the situation, which to my sadness was not met with wild enthusiasm. [1] http://archives.postgresql.org/pgsql-hackers/2008-08/msg00272.php Does it help to do CREATE OR REPLACE FUNCTION on callerX() after dropping and recreating called1()? If so, you might want to just recreate all of your functions every time you do a release. I wrote a perl script that does this and it's worked pretty well for me. Besides possibly avoiding this problem, it means that I don't really need to worry about which functions I've modified in this release quite as much, since I'm just going to push out the most-current definition for all of them. Nope, but the amount of release engineering (and deployment-time work) you need to do depends a lot on fragility of the system. Also true, but I think comparing plan invalidation to transactional semantics is quite unfair. There's basically no amount of user code which will compensate for the lack of an ACID-compliant database. On the other hand, working around the lack of plan invalidation (or the inability to add columns to views without recreating them) just requires being careful to catch all of the stray references in your DDL and testing thoroughly before you roll out to production, which are good things to do anyway. That's not to say that we shouldn't have plan invalidation, just that I don't think it's anywhere close to the same level of broken. ...Robert -- 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] Patch: plan invalidation vs stored procedures
On Wed, 2008-08-20 at 00:11 +0300, Hannu Krosing wrote: On Tue, 2008-08-19 at 16:56 -0400, Tom Lane wrote: More generally, if we are to try to invalidate on the strength of pg_proc changes, what of other DDL changes? Operators, operator classes, maybe? How about renaming a schema? I would like to see a line drawn between things we find worth trying to track and things we don't. If there is no such line, we're going to need a patch a lot larger than this one. Or maybe a simpler and smaller patch - just invalidate everything on every schema change :) It will have a momentary impact on performance at DDL time, but otherways might be more robust and easier to check for errors. I think Tom's main question is the right one: how much to invalidate? ISTM that there must be some user defined control over how this occurs. We have cascade and restrict as options in other places. Being able to force replanning of everything when you know its the right thing to do sounds sensible and useful. Being able to avoid it when you know it isn't needed also sounds sensible and useful. It would be useful to have an impact assessment tool, so we could say if I made this change, how many plans would it effect?. We can't do that because the plans aren't shared. Perhaps that is a good argument for a shared plan cache, or at least some way of defining whether some plans are shared, some not. -- Simon Riggs www.2ndQuadrant.com PostgreSQL Training, Services and Support -- 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] Patch: plan invalidation vs stored procedures
Joshua Drake [EMAIL PROTECTED] writes: Tom Lane [EMAIL PROTECTED] wrote: I'm not sure that I *want* a formal written-down backpatch policy. Then we write a formal guideline. It really isn't fair to new developers to not have any idea how they are going to be able to get a patch applied to older branches. Something like: Generally speaking we adhere to the following guideline for patches. * Security fixes are applied to all applicable branches. * Bugfixes are applied to all applicable branches * Note: A patch that addresses a known limitation is generally not backpatched * New features are always applied to -HEAD only. That just begs the question of what's the difference between a bug and a limitation. AFAICS, having such a policy/guideline/whatchacallit in place wouldn't have done a single thing to stop the current flamewar, because the people who want this thing back-patched are insisting that it's a bug, while those who don't are saying it's a long-known limitation. Also, there are a whole lot more considerations in a backpatch decision than just is it a bug. The (estimated) risk of creating new bugs and the extent to which the patch will change behavior that apps might be relying on are two big reasons why we might choose not to back-patch a bug fix. 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] Patch: plan invalidation vs stored procedures
Hannu Krosing [EMAIL PROTECTED] writes: If there is plan invalidation then you just change called1() to return one more field and that's it - no juggling with C) and D) and generally less things that can go wrong. That is a pure flight of fancy. Adjusting a function's API generally requires source-code changes on the caller side too. There might be a few limited cases where you can avoid that, but that doesn't leave you with much of an argument that this is a critical bug fix. It's a corner case and little more. FWIW, given that there will probably always be corner cases. I can see the attraction in Simon's suggestion of providing a way to manually issue a system-wide forced plan flush. 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] Patch: plan invalidation vs stored procedures
Tom Lane wrote: Also, there are a whole lot more considerations in a backpatch decision than just is it a bug. The (estimated) risk of creating new bugs and the extent to which the patch will change behavior that apps might be relying on are two big reasons why we might choose not to back-patch a bug fix. Right. And even if it is a bug the question might be what sort of bug is it? We might well be prepared to take some risks with code stability to plug security or data corruption bugs, a lot more than we would for other sorts of bugs. Even if this were considered a bug instead of a limitation, it doesn't come into the class of things we should be rushing to fix in the stable branches, unless the fix is fairly obvious and of limited impact, which is clearly not the case. cheers andrew -- 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] Patch: plan invalidation vs stored procedures
On Tue, Aug 19, 2008 at 07:45:16PM -0400, Tom Lane wrote: Hannu Krosing [EMAIL PROTECTED] writes: If there is plan invalidation then you just change called1() to return one more field and that's it - no juggling with C) and D) and generally less things that can go wrong. That is a pure flight of fancy. Adjusting a function's API generally requires source-code changes on the caller side too. There might be a few limited cases where you can avoid that, but that doesn't leave you with much of an argument that this is a critical bug fix. It's a corner case and little more. FWIW, given that there will probably always be corner cases. I can see the attraction in Simon's suggestion of providing a way to manually issue a system-wide forced plan flush. Would that require a system-wide plan cache to implement? Cheers, David. -- David Fetter [EMAIL PROTECTED] http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: [EMAIL PROTECTED] Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate -- 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] Patch: plan invalidation vs stored procedures
Andrew Dunstan [EMAIL PROTECTED] writes: Right. And even if it is a bug the question might be what sort of bug is it? We might well be prepared to take some risks with code stability to plug security or data corruption bugs, a lot more than we would for other sorts of bugs. As indeed we have done, and lost the bet more than once :-(. Rev 8.2.2 and siblings being the most recent example. A quick review of the release history will show other cases where well-intentioned, seemingly safe back-patches broke things. Now security patches are the worst-case scenario for this, because they typically go out with no significant public review. But even a regular bug-fix patch doesn't get all that much testing in the back branches before it hits the streets as a supposedly-stable update. By and large, if we commit something into REL8_3_STABLE today, it's going to appear in 8.3.4 with nothing more than buildfarm testing. That is a sobering prospect, and not one that makes me want to put nontrivial patches in there except at great need. 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] Patch: plan invalidation vs stored procedures
David Fetter [EMAIL PROTECTED] writes: On Tue, Aug 19, 2008 at 07:45:16PM -0400, Tom Lane wrote: FWIW, given that there will probably always be corner cases. I can see the attraction in Simon's suggestion of providing a way to manually issue a system-wide forced plan flush. Would that require a system-wide plan cache to implement? No, just a function that can issue a suitable sinval message. plancache.c would already respond in the desired way to a relcache inval message with OID = 0, though likely it'll be cleaner to invent an sinval message type specifically for the purpose. One thing to think about is whether the flush should be truly system-wide or just database-wide. I can see a lot more uses for the latter than the former --- I don't think there's a reason for cached plans to depend on any contents of the shared catalogs. 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] Patch: plan invalidation vs stored procedures
Every thread we are concerned in turns into something strange thing that is almost entirely differnet from the original intention. First thread we started was with the intention to discuss how we should handle the problem. Instead of discussion it was trolled into oblivion. Then we thought so what if no discussion we will submit a patch maybe people will understand we are serious. Nothing relevant came up. Spent week more to refine patch into something that looks good enough. And now we are having discusion what is bug and what s not in this thread. In the first message Martin asked There are probably a lot of details that I have overlooked. I'd be really thankful for some constructive comments and criticism. Especially, what needs to be done to have this in the core. Feedback appreciated. Can we get back to the topic? PS: We have 1+ functions (including lots of duplicates) PS: We are able to be as arrogant as any of you but we can get more things done with constructive comments. On Wed, Aug 20, 2008 at 2:53 AM, Andrew Dunstan [EMAIL PROTECTED] wrote: Tom Lane wrote: Also, there are a whole lot more considerations in a backpatch decision than just is it a bug. The (estimated) risk of creating new bugs and the extent to which the patch will change behavior that apps might be relying on are two big reasons why we might choose not to back-patch a bug fix. Right. And even if it is a bug the question might be what sort of bug is it? We might well be prepared to take some risks with code stability to plug security or data corruption bugs, a lot more than we would for other sorts of bugs. Even if this were considered a bug instead of a limitation, it doesn't come into the class of things we should be rushing to fix in the stable branches, unless the fix is fairly obvious and of limited impact, which is clearly not the case. cheers andrew
Re: [HACKERS] Patch: plan invalidation vs stored procedures
On Tue, Aug 19, 2008 at 09:50:53PM -0400, Tom Lane wrote: David Fetter [EMAIL PROTECTED] writes: On Tue, Aug 19, 2008 at 07:45:16PM -0400, Tom Lane wrote: FWIW, given that there will probably always be corner cases. I can see the attraction in Simon's suggestion of providing a way to manually issue a system-wide forced plan flush. Would that require a system-wide plan cache to implement? No, just a function that can issue a suitable sinval message. plancache.c would already respond in the desired way to a relcache inval message with OID = 0, though likely it'll be cleaner to invent an sinval message type specifically for the purpose. One thing to think about is whether the flush should be truly system-wide or just database-wide. I can see a lot more uses for the latter than the former --- I don't think there's a reason for cached plans to depend on any contents of the shared catalogs. They might during an on-line upgrade. Zdenek? Cheers, David. -- David Fetter [EMAIL PROTECTED] http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: [EMAIL PROTECTED] Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate -- 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] Patch: plan invalidation vs stored procedures
2008/8/18 Dimitri Fontaine [EMAIL PROTECTED]: Hi, Le lundi 18 août 2008, Andrew Dunstan a écrit : On Sat, Aug 16, 2008 at 09:40:19PM -0400, Tom Lane wrote: This is not the kind of patch we put into stable branches. So what? That is not the only criterion for backpatching. I fail to understand why this problem is not qualified as a bug. Does it change of result some queries? It is protection to server's hang? Regards, -- dim -- 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] Patch: plan invalidation vs stored procedures
On Mon, 2008-08-18 at 11:05 +0200, Pavel Stehule wrote: 2008/8/18 Dimitri Fontaine [EMAIL PROTECTED]: Hi, Le lundi 18 août 2008, Andrew Dunstan a écrit : On Sat, Aug 16, 2008 at 09:40:19PM -0400, Tom Lane wrote: This is not the kind of patch we put into stable branches. So what? That is not the only criterion for backpatching. I fail to understand why this problem is not qualified as a bug. Does it change of result some queries? Not in the long run, but not invalidating the functions (current behaviour) postpones seeing the results of function change until DBA manually restarts the error-producing client. It is protection to server's hang? Can't understand this question :( If you mean, does the change protect against hanging the server, then no, currently the server does not actually hang, it just becomes unusable until reconnect :( - 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] Patch: plan invalidation vs stored procedures
2008/8/18 Hannu Krosing [EMAIL PROTECTED]: On Mon, 2008-08-18 at 11:05 +0200, Pavel Stehule wrote: 2008/8/18 Dimitri Fontaine [EMAIL PROTECTED]: Hi, Le lundi 18 août 2008, Andrew Dunstan a écrit : On Sat, Aug 16, 2008 at 09:40:19PM -0400, Tom Lane wrote: This is not the kind of patch we put into stable branches. So what? That is not the only criterion for backpatching. I fail to understand why this problem is not qualified as a bug. Does it change of result some queries? Not in the long run, but not invalidating the functions (current behaviour) postpones seeing the results of function change until DBA manually restarts the error-producing client. It is protection to server's hang? Can't understand this question :( If you mean, does the change protect against hanging the server, then no, currently the server does not actually hang, it just becomes unusable until reconnect :( Hi I am sorry, but it's really new feature and not bug fix 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] Patch: plan invalidation vs stored procedures
Does it change of result some queries? Patch in itself is not changing what the queries return. It just gets rid of error condition from which Postgres itself is not able to recover. It is protection to server's hang? For users of stored procedures it is protection from downtime. For Skype it has been around 20% of databse related downtime this year. On Mon, Aug 18, 2008 at 12:05 PM, Pavel Stehule [EMAIL PROTECTED]wrote: 2008/8/18 Dimitri Fontaine [EMAIL PROTECTED]: Hi, Le lundi 18 août 2008, Andrew Dunstan a écrit : On Sat, Aug 16, 2008 at 09:40:19PM -0400, Tom Lane wrote: This is not the kind of patch we put into stable branches. So what? That is not the only criterion for backpatching. I fail to understand why this problem is not qualified as a bug. Does it change of result some queries? It is protection to server's hang? Regards, -- dim
Re: [HACKERS] Patch: plan invalidation vs stored procedures
Asko Oja [EMAIL PROTECTED] writes: For users of stored procedures it is protection from downtime. For Skype it has been around 20% of databse related downtime this year. Perhaps Skype needs to rethink how they are modifying functions. The reason that this case wasn't covered in 8.3 is that there didn't seem to be a use-case that justified doing the extra work. I still haven't seen one. Other than inline-able SQL functions there is no reason to invalidate a stored plan based on the fact that some function it called changed contents. regards, tom lane -- 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] Patch: plan invalidation vs stored procedures
On Sat, Aug 16, 2008 at 09:40:19PM -0400, Tom Lane wrote: Asko Oja [EMAIL PROTECTED] writes: Is it possible to get it into some official 8.3.x release This is not the kind of patch we put into stable branches. Does this really count as a user-visible change, except in the sense that they won't see things erroring out? It doesn't add new syntax, as far as I can tell. Cheers, David. -- David Fetter [EMAIL PROTECTED] http://fetter.org/ Phone: +1 415 235 3778 AIM: dfetter666 Yahoo!: dfetter Skype: davidfetter XMPP: [EMAIL PROTECTED] Remember to vote! Consider donating to Postgres: http://www.postgresql.org/about/donate -- 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] Patch: plan invalidation vs stored procedures
David Fetter wrote: On Sat, Aug 16, 2008 at 09:40:19PM -0400, Tom Lane wrote: Asko Oja [EMAIL PROTECTED] writes: Is it possible to get it into some official 8.3.x release This is not the kind of patch we put into stable branches. Does this really count as a user-visible change, except in the sense that they won't see things erroring out? It doesn't add new syntax, as far as I can tell. So what? That is not the only criterion for backpatching. The bigger the change the more resistance there will be to backpatching it. Code stability is a major concern. cheers andrew -- 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] Patch: plan invalidation vs stored procedures
Hi We need plan invalidation fix in 8.3 also at least it would make migrating from 8.2 to 8.3 much more attractive. Currenlty we are having problems related to plan invalidation couple of times per week (mainly we have to let developers change their code before we release it into live databases but it feels like sitting on ticking bomb after previous downtime). Is it possible to get it into some official 8.3.x release or should we do it in house? Who should add it into september commitfest? Asko On Fri, Aug 15, 2008 at 2:13 PM, Martin Pihlak [EMAIL PROTECTED]wrote: Tom Lane wrote: Martin Pihlak [EMAIL PROTECTED] writes: Changing statement result type is also currently prohibited in StorePreparedStatement. There maybe good reasons for this, How about the SQL spec says so? Admittedly, it's a bit of a jump from views to prepared statements, but the spec is perfectly clear that altering a table doesn't alter any views dependent on it: SQL99 11.11 add column definition saith As you said it is a bit of a jump ... For one thing view definitions are persistent whereas statements are bound to be replanned sooner or later - reconnects etc. Disallowing replanning after invalidation just postpones it and meanwhile the cached plans are left unusable (cached plan must not change result). IMHO the problem should be left for the application to handle. Because this is where it will end up anyway. Attached is a patch that implements plan invalidation on function DROP, REPLACE and ALTER. Function oids used by the query are collected in analyze phase and stored in PlannedStmt. Only plans that reference the altered function are invalidated. The patch also enables replanning on result set change. 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] Patch: plan invalidation vs stored procedures
Asko Oja [EMAIL PROTECTED] writes: Is it possible to get it into some official 8.3.x release This is not the kind of patch we put into stable branches. 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] Patch: plan invalidation vs stored procedures
Tom Lane wrote:
Martin Pihlak [EMAIL PROTECTED] writes:
Changing statement result type is also currently prohibited in
StorePreparedStatement. There maybe good reasons for this,
How about the SQL spec says so?
Admittedly, it's a bit of a jump from views to prepared statements,
but the spec is perfectly clear that altering a table doesn't alter
any views dependent on it: SQL99 11.11 add column definition saith
As you said it is a bit of a jump ... For one thing view definitions are
persistent whereas statements are bound to be replanned sooner or later -
reconnects etc. Disallowing replanning after invalidation just postpones
it and meanwhile the cached plans are left unusable (cached plan must not
change result). IMHO the problem should be left for the application to handle.
Because this is where it will end up anyway.
Attached is a patch that implements plan invalidation on function DROP,
REPLACE and ALTER. Function oids used by the query are collected in analyze
phase
and stored in PlannedStmt. Only plans that reference the altered function are
invalidated. The patch also enables replanning on result set change.
regards,
Martin
Index: src/backend/commands/functioncmds.c
===
RCS file: /projects/cvsroot/pgsql/src/backend/commands/functioncmds.c,v
retrieving revision 1.98
diff -c -r1.98 functioncmds.c
*** src/backend/commands/functioncmds.c 18 Jul 2008 03:32:52 - 1.98
--- src/backend/commands/functioncmds.c 15 Aug 2008 11:12:51 -
***
*** 59,64
--- 59,65
#include utils/rel.h
#include utils/syscache.h
#include utils/tqual.h
+ #include utils/inval.h
static void AlterFunctionOwner_internal(Relation rel, HeapTuple tup,
***
*** 680,685
--- 681,687
HeapTuple languageTuple;
Form_pg_language languageStruct;
List *as_clause;
+ Oid funcOid;
/* Convert list of names to a name and namespace */
namespaceId = QualifiedNameGetCreationNamespace(stmt-funcname,
***
*** 817,823
* And now that we have all the parameters, and know we're permitted to do
* so, go ahead and create the function.
*/
! ProcedureCreate(funcname,
namespaceId,
stmt-replace,
returnsSet,
--- 819,825
* And now that we have all the parameters, and know we're permitted to do
* so, go ahead and create the function.
*/
! funcOid = ProcedureCreate(funcname,
namespaceId,
stmt-replace,
returnsSet,
***
*** 837,842
--- 839,848
PointerGetDatum(proconfig),
procost,
prorows);
+
+ /* Send invalidation on REPLACE */
+ if (stmt-replace)
+ CacheInvalidateProcedure(funcOid);
}
***
*** 906,911
--- 912,920
object.objectSubId = 0;
performDeletion(object, stmt-behavior);
+
+ /* Notify that cached plans should be replanned */
+ CacheInvalidateProcedure(funcOid);
}
/*
***
*** 1029,1034
--- 1038,1046
heap_close(rel, NoLock);
heap_freetuple(tup);
+
+ /* Need plan invalidation after this */
+ CacheInvalidateProcedure(procOid);
}
/*
***
*** 1294,1299
--- 1306,1314
heap_close(rel, NoLock);
heap_freetuple(tup);
+
+ /* Invalidate plans after this */
+ CacheInvalidateProcedure(funcOid);
}
/*
Index: src/backend/commands/prepare.c
===
RCS file: /projects/cvsroot/pgsql/src/backend/commands/prepare.c,v
retrieving revision 1.89
diff -c -r1.89 prepare.c
*** src/backend/commands/prepare.c 21 Jul 2008 15:26:55 - 1.89
--- src/backend/commands/prepare.c 15 Aug 2008 11:12:52 -
***
*** 188,196
/* Shouldn't have a non-fully-planned plancache entry */
if (!entry-plansource-fully_planned)
elog(ERROR, EXECUTE does not support unplanned prepared statements);
- /* Shouldn't get any non-fixed-result cached plan, either */
- if (!entry-plansource-fixed_result)
- elog(ERROR, EXECUTE does not support variable-result cached plans);
/* Evaluate parameters, if any */
if (entry-plansource-num_params 0)
--- 188,193
***
*** 462,468
cursor_options,
stmt_list,
true,
! true);
/* Now we can add entry to hash table */
entry = (PreparedStatement *) hash_search(prepared_queries,
--- 459,465
cursor_options,
stmt_list,
true,
! false);
/* Now we can add entry to hash table */
entry = (PreparedStatement *) hash_search(prepared_queries,
***
*** 523,533
TupleDesc
FetchPreparedStatementResultDesc(PreparedStatement *stmt)
{
! /*
! * Since we don't allow prepared statements' result tupdescs to change,
! * there's no need for a revalidate call here.
! */
! Assert(stmt-plansource-fixed_result);
if
Re: [HACKERS] Patch: plan invalidation vs stored procedures
Changing statement result type is also currently prohibited in StorePreparedStatement. There maybe good reasons for this, How about the SQL spec says so? Prepare time is often also the time when you bind the result, or more generally set up the code to handle the result. Generally I argue, that a mode of operation must exist where a change in return type throws an error, so the client can readjust to the change. We are only allowed to silently replan when it is clear that the caller is agnostic to the change. e.g. because the caller only accesses explicit columns of the return type/result set, or does not supply a new parameter with a default, (or because he set some parameter that tells us he can cope). Certainly a new prepare must be able to cope with the change though, which currently does not seem to be the case when an SP calls another one that was dropped (and recreated)? Andreas -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
[HACKERS] Patch: plan invalidation vs stored procedures
This is a followup for thread plan invalidation vs stored procedures.
The background is that it is impossible to change function return type without
dropping and recreating. Unfortunately dropping a function ruins all of the
prepared statements that reference that function (including other functions).
To make matters worse the ruined plans are never invalidated and keep returning
cache lookup failed error until replanned (requires admin intervention). Also
the DBA that dropped the function probably has no clue that something is wrong
- not before looking at the server logs at least. This is NOT good, especially
if the database is supporting paid services.
I have prepared proof of concept patch to support plan invalidation on function
DROP (will add ALTER, REPLACE, etc. later). Currently the invalidation is
handled by just dropping all the plans when invalidation message is received.
The desired behaviour would be of course to drop only the affected plans. This
needs function oid list to be present in PlannedStmt -- will look into this
later.
Probably a job for the planner.
Changing statement result type is also currently prohibited in
StorePreparedStatement. There maybe good reasons for this, but for the
invalidation to be really useful, it should be enabled. Right now the attempt
to change type renders the plan unusable -- ERROR: cached plan must not
change result type. Besides that, the patch could already be useful in some
environments - if you are willing to trade the errors for some extra planning
CPU.
There are probably a lot of details that I have overlooked. I'd be really
thankful for some constructive comments and criticism. Especially, what needs
to be done to have this in the core. Feedback appreciated.
regards,
Martin
*** ./src/backend/commands/functioncmds.c.orig 2008-08-06 17:01:28.0 +0300
--- ./src/backend/commands/functioncmds.c 2008-08-06 19:10:51.0 +0300
***
*** 59,64
--- 59,65
#include utils/rel.h
#include utils/syscache.h
#include utils/tqual.h
+ #include utils/inval.h
static void AlterFunctionOwner_internal(Relation rel, HeapTuple tup,
***
*** 906,911
--- 907,915
object.objectSubId = 0;
performDeletion(object, stmt-behavior);
+
+ /* Notify that cached plans should be replanned */
+ CacheInvalidateProcedure(funcOid);
}
/*
*** ./src/backend/utils/cache/inval.c.orig 2008-08-06 16:12:17.0 +0300
--- ./src/backend/utils/cache/inval.c 2008-08-06 18:01:24.0 +0300
***
*** 115,121
typedef struct InvalidationListHeader
{
InvalidationChunk *cclist; /* list of chunks holding catcache msgs */
! InvalidationChunk *rclist; /* list of chunks holding relcache/smgr msgs */
} InvalidationListHeader;
/*
--- 115,121
typedef struct InvalidationListHeader
{
InvalidationChunk *cclist; /* list of chunks holding catcache msgs */
! InvalidationChunk *rclist; /* list of chunks holding relcache/smgr/proc msgs */
} InvalidationListHeader;
/*
***
*** 177,182
--- 177,183
#define TWOPHASE_INFO_FILE_AFTER 2 /* relcache file inval */
static void PersistInvalidationMessage(SharedInvalidationMessage *msg);
+ static void CacheRegisterCallback(int cacheid, CacheCallbackFunction func, Datum arg);
/*
***
*** 363,368
--- 364,393
}
/*
+ * Add a proc inval entry
+ */
+ static void
+ AddProcInvalidationMessage(InvalidationListHeader *hdr,
+ Oid dbId, Oid procId)
+ {
+ SharedInvalidationMessage msg;
+
+ /* Don't add a duplicate item */
+ /* We assume dbId need not be checked because it will never change */
+
+ /* XXX: for now, only keep one proc invalidation message per database */
+ ProcessMessageList(hdr-rclist,
+ if (msg-pm.id == SHAREDINVALPROC_ID)
+ return);
+
+ /* OK, add the item */
+ msg.pm.id = SHAREDINVALPROC_ID;
+ msg.pm.dbId = dbId;
+ msg.pm.procId = procId;
+ AddInvalidationMessage(hdr-rclist, msg);
+ }
+
+ /*
* Append one list of invalidation messages to another, resetting
* the source list to empty.
*/
***
*** 465,470
--- 490,512
}
/*
+ * RegisterProcInvalidation
+ *
+ * As above, but register a procedure invalidation event.
+ */
+ static void
+ RegisterProcInvalidation(Oid dbId, Oid procId)
+ {
+ AddProcInvalidationMessage(transInvalInfo-CurrentCmdInvalidMsgs,
+ dbId, procId);
+
+ /*
+ * As above, just in case there is not an associated catalog change.
+ */
+ (void) GetCurrentCommandId(true);
+ }
+
+ /*
* LocalExecuteInvalidationMessage
*
* Process a single invalidation message (which could be of any type).
***
*** 516,521
--- 558,577
*/
smgrclosenode(msg-sm.rnode);
}
+ else if (msg-id == SHAREDINVALPROC_ID)
+ {
+ if (msg-rc.dbId ==
Re: [HACKERS] Patch: plan invalidation vs stored procedures
Martin Pihlak [EMAIL PROTECTED] writes: Changing statement result type is also currently prohibited in StorePreparedStatement. There maybe good reasons for this, How about the SQL spec says so? Admittedly, it's a bit of a jump from views to prepared statements, but the spec is perfectly clear that altering a table doesn't alter any views dependent on it: SQL99 11.11 add column definition saith NOTE 189 - The addition of a column to a table has no effect on any existing query expression included in a view descriptor, triggered action included in a trigger descriptor, or search condition included in a constraint descriptor because any implicit column references in these descriptor elements are syntactically substituted by explicit column references under the Syntax Rules of Subclause 7.11, query specification. Furthermore, by implication (from the lack of any General Rules to the contrary), the meaning of a column reference is never retroactively changed by the addition of a column subsequent to the invocation of the SQL schema statement containing that column reference. and there was a comparable restriction in SQL92. You'd need to make a pretty strong argument why prepared statements should behave differently from views to convince me that changing this is a good idea. 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
