Thread: Prepared statements fail after schema changes with surprising error
A user reported an interesting issue today. After restoring a dump created with --clean on a running application in his development environment his application started complaining of missing tables despite those tables very clearly existing.
After a little thinking, we determined that this was due to the now-default behaviour of Rails to create prepared statements for most queries. The prepared statements error out because the old relation they point to is missing, but this gives a misleading report thus:
PG::Error: ERROR: relation "xxx" does not exist
--
Peter van Hardenberg
San Francisco, California
"Everything was beautiful, and nothing hurt." -- Kurt Vonnegut
I'm not sure what the best outcome here would be. A very simple solution might be to expand the error message or add a hint to make it descriptive enough that a user might be able to figure out the cause on their own without happening to have the unusual intersection of Rails and Postgres internals knowlege I (unfortunately) possess. A better solution might be to attempt to re-prepare the statement before throwing an error.
-pvh
Peter van Hardenberg
San Francisco, California
"Everything was beautiful, and nothing hurt." -- Kurt Vonnegut
Peter van Hardenberg <pvh@pvh.ca> writes: > A user reported an interesting issue today. After restoring a dump created > with --clean on a running application in his development environment his > application started complaining of missing tables despite those tables very > clearly existing. > After a little thinking, we determined that this was due to the now-default > behaviour of Rails to create prepared statements for most queries. The > prepared statements error out because the old relation they point to is > missing, but this gives a misleading report thus: > PG::Error: ERROR: relation "xxx" does not exist > I'm not sure what the best outcome here would be. A very simple solution > might be to expand the error message or add a hint to make it descriptive > enough that a user might be able to figure out the cause on their own > without happening to have the unusual intersection of Rails and Postgres > internals knowlege I (unfortunately) possess. A better solution might be to > attempt to re-prepare the statement before throwing an error. Works for me ... regression=# create table z1 (f1 int , f2 int); CREATE TABLE regression=# prepare sz1 as select * from z1; PREPARE regression=# insert into z1 values(1,2); INSERT 0 1 regression=# execute sz1;f1 | f2 ----+---- 1 | 2 (1 row) regression=# drop table z1; DROP TABLE regression=# create table z1 (f1 int , f2 int); CREATE TABLE regression=# insert into z1 values(3,4); INSERT 0 1 regression=# execute sz1;f1 | f2 ----+---- 3 | 4 (1 row) regards, tom lane
On 22 January 2013 00:00, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Works for me ... That's what I thought. But looking at RangeVarGetRelidExtended() and recomputeNamespacePath(), do you suppose that the problem could be that access privileges used by the app differed for a schema (or, more accurately, two physically distinct namespaces with the same nspname) between executions of the prepared query? -- Regards, Peter Geoghegan
Re: Prepared statements fail after schema changes with surprising error
From
"Dickson S. Guedes"
Date:
2013/1/21 Peter van Hardenberg <pvh@pvh.ca>: > A user reported an interesting issue today. After restoring a dump created > with --clean on a running application in his development environment his > application started complaining of missing tables despite those tables very > clearly existing. > > After a little thinking, we determined that this was due to the now-default > behaviour of Rails to create prepared statements for most queries. The > prepared statements error out because the old relation they point to is > missing, but this gives a misleading report thus: > > PG::Error: ERROR: relation "xxx" does not exist Isn't that something with search_path? -- Dickson S. Guedes mail/xmpp: guedes@guedesoft.net - skype: guediz http://github.com/guedes - http://guedesoft.net http://www.postgresql.org.br
Peter Geoghegan <peter.geoghegan86@gmail.com> writes: > On 22 January 2013 00:00, Tom Lane <tgl@sss.pgh.pa.us> wrote: >> Works for me ... > That's what I thought. But looking at RangeVarGetRelidExtended() and > recomputeNamespacePath(), do you suppose that the problem could be > that access privileges used by the app differed for a schema (or, more > accurately, two physically distinct namespaces with the same nspname) > between executions of the prepared query? What I'm suspicious of is that Peter is complaining about an old version, or that there's some other critical piece of information he left out. I don't plan to speculate about causes without a concrete test case. regards, tom lane
Re: Prepared statements fail after schema changes with surprising error
From
Peter van Hardenberg
Date:
Hm - I'm still able to recreate the test the user's running using pg_dump/pg_restore. I'm still working to see if I can minimize the test-case, but this is against 9.2.2. Would you prefer I test against HEAD?
--
Peter van Hardenberg
San Francisco, California
"Everything was beautiful, and nothing hurt." -- Kurt Vonnegut
regression=# create table z1 (f1 int);
CREATE TABLE
regression=# prepare sz1 as select * from z1;
PREPARE
regression=# insert into z1 values (1);
INSERT 0 1
regression=# execute sz1;
f1
----
1
(1 row)
# In another terminal window
$ pg_dump -F c regression > test.dump
$ pg_restore --clean --no-acl --no-owner -d regression test.dump
ERROR: cannot drop the currently open database
STATEMENT: DROP DATABASE regression;
pg_restore: [archiver (db)] Error while PROCESSING TOC:
pg_restore: [archiver (db)] Error from TOC entry 2185; 1262 16384 DATABASE regression pvh
pg_restore: [archiver (db)] could not execute query: ERROR: cannot drop the currently open database
Command was: DROP DATABASE regression;
WARNING: errors ignored on restore: 1
$
# back in the same backend
regression=# execute sz1;
ERROR: relation "z1" does not exist
regression=# select * from z1;
f1
----
1
(1 row)
regression=#
On Mon, Jan 21, 2013 at 5:31 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
What I'm suspicious of is that Peter is complaining about an oldPeter Geoghegan <peter.geoghegan86@gmail.com> writes:
> On 22 January 2013 00:00, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Works for me ...
> That's what I thought. But looking at RangeVarGetRelidExtended() and
> recomputeNamespacePath(), do you suppose that the problem could be
> that access privileges used by the app differed for a schema (or, more
> accurately, two physically distinct namespaces with the same nspname)
> between executions of the prepared query?
version, or that there's some other critical piece of information he
left out. I don't plan to speculate about causes without a concrete
test case.
regards, tom lane
Peter van Hardenberg
San Francisco, California
"Everything was beautiful, and nothing hurt." -- Kurt Vonnegut
Re: Prepared statements fail after schema changes with surprising error
From
Peter van Hardenberg
Date:
Okay - I've narrowed it down to an interaction with schema recreation. Here's a minimal test-case I created by paring back the restore from the pg_restore output until I only had the essence remaining:
--
Peter van Hardenberg
San Francisco, California
"Everything was beautiful, and nothing hurt." -- Kurt Vonnegut
-- setup
drop table z1;
create table z1 (f1 int);
insert into z1 values (1);
prepare sz1 as select * from z1;
select 'executing first prepared statement';
execute sz1;
-- remainder of minimized pg_restore SQL output
DROP TABLE public.z1;
DROP SCHEMA public;
CREATE SCHEMA public;
CREATE TABLE z1 (
f1 integer
);
-- proof of regression
select 'executing second prepared statement';
execute sz1;
select 'selecting from z1 to prove it exists';
select * from z1;
On Mon, Jan 21, 2013 at 10:45 PM, Peter van Hardenberg <pvh@pvh.ca> wrote:
Hm - I'm still able to recreate the test the user's running using pg_dump/pg_restore. I'm still working to see if I can minimize the test-case, but this is against 9.2.2. Would you prefer I test against HEAD?regression=# create table z1 (f1 int);CREATE TABLEregression=# prepare sz1 as select * from z1;PREPAREregression=# insert into z1 values (1);INSERT 0 1regression=# execute sz1;f1----1(1 row)# In another terminal window$ pg_dump -F c regression > test.dump$ pg_restore --clean --no-acl --no-owner -d regression test.dumpERROR: cannot drop the currently open databaseSTATEMENT: DROP DATABASE regression;pg_restore: [archiver (db)] Error while PROCESSING TOC:pg_restore: [archiver (db)] Error from TOC entry 2185; 1262 16384 DATABASE regression pvhpg_restore: [archiver (db)] could not execute query: ERROR: cannot drop the currently open databaseCommand was: DROP DATABASE regression;WARNING: errors ignored on restore: 1$# back in the same backendregression=# execute sz1;ERROR: relation "z1" does not existregression=# select * from z1;f1----1(1 row)regression=#On Mon, Jan 21, 2013 at 5:31 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:What I'm suspicious of is that Peter is complaining about an oldPeter Geoghegan <peter.geoghegan86@gmail.com> writes:
> On 22 January 2013 00:00, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Works for me ...
> That's what I thought. But looking at RangeVarGetRelidExtended() and
> recomputeNamespacePath(), do you suppose that the problem could be
> that access privileges used by the app differed for a schema (or, more
> accurately, two physically distinct namespaces with the same nspname)
> between executions of the prepared query?
version, or that there's some other critical piece of information he
left out. I don't plan to speculate about causes without a concrete
test case.
regards, tom lane--
Peter van Hardenberg
San Francisco, California
"Everything was beautiful, and nothing hurt." -- Kurt Vonnegut
Peter van Hardenberg
San Francisco, California
"Everything was beautiful, and nothing hurt." -- Kurt Vonnegut
Peter van Hardenberg <pvh@pvh.ca> writes: > Okay - I've narrowed it down to an interaction with schema recreation. > Here's a minimal test-case I created by paring back the restore from the > pg_restore output until I only had the essence remaining: Hm ... I'm too tired to trace through the code to prove this theory, but I think what's happening is that this bit: > DROP SCHEMA public; > CREATE SCHEMA public; changes the OID of schema public, whereas the search_path that's cached for the cached plan is cached in terms of OIDs. So while there is a table named public.z1 at the end of the sequence, it's not in any schema found in the cached search path. We could possibly fix that by making the path be cached as textual names not OIDs, but then people would complain (rightly, I think) that renaming a schema caused unexpected behavior. There's also the other issues that have been discussed again recently about whether we should be attempting to reinstall an old search path in the first place. We could easily dodge this issue if we redefined what the desired behavior is ... but I'm not sure if we want to risk the fallout of that. regards, tom lane
On Tue, Jan 22, 2013 at 2:17 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Peter van Hardenberg <pvh@pvh.ca> writes: >> Okay - I've narrowed it down to an interaction with schema recreation. >> Here's a minimal test-case I created by paring back the restore from the >> pg_restore output until I only had the essence remaining: > > Hm ... I'm too tired to trace through the code to prove this theory, but > I think what's happening is that this bit: > >> DROP SCHEMA public; >> CREATE SCHEMA public; > > changes the OID of schema public, whereas the search_path that's cached > for the cached plan is cached in terms of OIDs. So while there is a > table named public.z1 at the end of the sequence, it's not in any schema > found in the cached search path. > > We could possibly fix that by making the path be cached as textual names > not OIDs, but then people would complain (rightly, I think) that > renaming a schema caused unexpected behavior. What sort of unexpected behavior? I mean, search_path in its original form is stored as text, anyway. So if the unexpected behavior is merely that they're going to be referencing a different schema, that's going to happen anyway, as soon as they reconnect. I'm not sure there's any logic in trying to postpone the inevitable. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Tom Lane <tgl@sss.pgh.pa.us> writes: >> DROP SCHEMA public; >> CREATE SCHEMA public; > > changes the OID of schema public, whereas the search_path that's cached > for the cached plan is cached in terms of OIDs. So while there is a > table named public.z1 at the end of the sequence, it's not in any schema > found in the cached search path. The DROP SCHEMA should invalidate the cached plan, certainly? Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
On 2013-01-22 14:24:26 +0100, Dimitri Fontaine wrote: > > Tom Lane <tgl@sss.pgh.pa.us> writes: > >> DROP SCHEMA public; > >> CREATE SCHEMA public; > > > > changes the OID of schema public, whereas the search_path that's cached > > for the cached plan is cached in terms of OIDs. So while there is a > > table named public.z1 at the end of the sequence, it's not in any schema > > found in the cached search path. > > The DROP SCHEMA should invalidate the cached plan, certainly? Afaics the error happens during replanning of the invalidated plan. Greetings, Andres Freund -- Andres Freund http://www.2ndQuadrant.com/PostgreSQL Development, 24x7 Support, Training & Services
Re: Re: Prepared statements fail after schema changes with surprising error
From
Dimitri Fontaine
Date:
Andres Freund <andres@2ndquadrant.com> writes: > On 2013-01-22 14:24:26 +0100, Dimitri Fontaine wrote: >> Tom Lane <tgl@sss.pgh.pa.us> writes: >> >> DROP SCHEMA public; >> >> CREATE SCHEMA public; >> > >> > changes the OID of schema public, whereas the search_path that's cached >> > for the cached plan is cached in terms of OIDs. So while there is a >> > table named public.z1 at the end of the sequence, it's not in any schema >> > found in the cached search path. >> >> The DROP SCHEMA should invalidate the cached plan, certainly? > > Afaics the error happens during replanning of the invalidated plan. Oh, replaning with the cached search_path even when the invalidation came from a DROP SCHEMA, you mean? Do we have enough information to make that case appart? Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
Robert Haas <robertmhaas@gmail.com> writes: > On Tue, Jan 22, 2013 at 2:17 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: >> I think what's happening is that this bit: >>> DROP SCHEMA public; >>> CREATE SCHEMA public; >> changes the OID of schema public, whereas the search_path that's cached >> for the cached plan is cached in terms of OIDs. So while there is a >> table named public.z1 at the end of the sequence, it's not in any schema >> found in the cached search path. >> >> We could possibly fix that by making the path be cached as textual names >> not OIDs, but then people would complain (rightly, I think) that >> renaming a schema caused unexpected behavior. > What sort of unexpected behavior? After reflecting on this a bit, I think that the problem may come from drawing an unjustified analogy between views and prepared statements. The code is certainly trying to treat them as the same thing, but perhaps we shouldn't do that. Consider that once you docreate view v as select * from s.t; the view will continue to refer to the same table object no matter what. You can rename t, you can rename s, you can move t to a different schema and then drop s, but the view still knows what t is, because the reference is by OID. The one thing you can't do is drop t, because the stored dependency from v to t will prevent that (at least unless you let it cascade to drop v as well). Views therefore do not have, or need, any explicit dependencies on either specific schemas or their creation-time search_path --- they only have dependencies on individual objects. The current plancache code is trying, in a somewhat half-baked fashion, to preserve those semantics for prepared queries --- that's partly because it's reusing the dependency mechanism that was designed for views, and partly because it didn't occur to us to question that model. But it now strikes me that the model doesn't apply very well, so maybe we need a new one. The key point that seems to force a different treatment is that there are no stored (globally-visible) dependencies for prepared queries, so there's no way to guarantee that referenced objects don't get dropped. We could possibly set things up so that re-executing a prepared query that references now-dropped objects would throw an error; but what people seem to prefer is that it should be re-analyzed to see if the original source text would now refer to a different object. And we're doing that, but we haven't followed through on the logical implications. The implication, ISTM, is that we should no longer consider that referring to the same objects throughout the query's lifespan is a goal at all. Rather, what we should be trying to do is make the query preparation transparent, in the sense that you should get the same results as if you resubmitted the original query text each time. In particular, it now seems to me that this makes a good argument for changing what plancache is doing with search_path. Instead of re-establishing the original search_path in a rather vain hope that the same objects will be re-selected by parse analysis, we should consider that the prepared query has a dependency on the active search path, and thus force a replan if the effective search path changes. I'm not sure that we can make the plan caching 100% transparent, though. The existing mechanisms will force replan if any object used in the plan is modified (and fortunately, "modified" includes "renamed", even though a rename isn't interesting according to the view-centric worldview). And we can force replan if the search path changes (ie, the effective list of schema OIDs changes). But there are cases where neither of those things happens and yet the user might expect a new object to be selected. Consider for example that the search path is a, b, c, and we have a prepared query "select * from t", and that currently refers to b.t. If now someone creates a.t, or renames a.x to a.t, then a replan would cause the query to select from a.t ... but there was no invalidation event that will impinge on the stored plan, and the search_path setting didn't change either. I don't think we want to accept the overhead of saying "any DDL anywhere invalidates all cached plans", so I don't see any good way to make this case transparent. How much do we care? regards, tom lane
Dimitri Fontaine <dimitri@2ndQuadrant.fr> writes: > Tom Lane <tgl@sss.pgh.pa.us> writes: >>> DROP SCHEMA public; >>> CREATE SCHEMA public; >> changes the OID of schema public, whereas the search_path that's cached >> for the cached plan is cached in terms of OIDs. So while there is a >> table named public.z1 at the end of the sequence, it's not in any schema >> found in the cached search path. > The DROP SCHEMA should invalidate the cached plan, certainly? It does not; see my reply to Robert. regards, tom lane
* Tom Lane (tgl@sss.pgh.pa.us) wrote: > Instead of > re-establishing the original search_path in a rather vain hope that the > same objects will be re-selected by parse analysis, we should consider > that the prepared query has a dependency on the active search path, and > thus force a replan if the effective search path changes. Presuming that this flows through to SPI and in effect pl/pgsql, this is exactly what I was arguing for a while back, when we ran into cases with connection pooling where the plans generated by a pl/pgsql function remained the same, referring to the objects against which it was originally planned, even though the search_path had changed. As I recall, the same might have even been true across 'set role' actions where the text of 'search_path' wasn't actually changed, but the '$user' variable inside it was. Now, there is definitely legitimate concern about search_path rejiggery and security definer functions, so nothing done here should change how we handle that case. > Consider for example that the search path is a, b, c, > and we have a prepared query "select * from t", and that currently > refers to b.t. If now someone creates a.t, or renames a.x to a.t, > then a replan would cause the query to select from a.t ... but there > was no invalidation event that will impinge on the stored plan, and the > search_path setting didn't change either. I don't think we want to > accept the overhead of saying "any DDL anywhere invalidates all cached > plans", so I don't see any good way to make this case transparent. > How much do we care? That may simply be a trade-off that we need to make. I agree that we don't want to invalidate everything due to any DDL anywhere. I do think that what you're proposing here wrt invalidating based on search_path changes is an improvement over the current situation. Thanks, Stephen
Stephen Frost <sfrost@snowman.net> writes: > * Tom Lane (tgl@sss.pgh.pa.us) wrote: >> Instead of >> re-establishing the original search_path in a rather vain hope that the >> same objects will be re-selected by parse analysis, we should consider >> that the prepared query has a dependency on the active search path, and >> thus force a replan if the effective search path changes. > Presuming that this flows through to SPI and in effect pl/pgsql, this is > exactly what I was arguing for a while back, when we ran into cases with > connection pooling where the plans generated by a pl/pgsql function > remained the same, referring to the objects against which it was > originally planned, even though the search_path had changed. As I > recall, the same might have even been true across 'set role' actions > where the text of 'search_path' wasn't actually changed, but the '$user' > variable inside it was. The implementation I have in mind would compare the lists of schema OIDs computed from the text of search_path, so it ought to do the right thing with $user. > Now, there is definitely legitimate concern about search_path rejiggery > and security definer functions, so nothing done here should change how > we handle that case. Right offhand I see no security risk that wouldn't occur anyway given a different calling sequence (ie, if the vulnerable call had happened first). Certainly it's conceivable that somebody's app somewhere is dependent on the current behavior, but it seems relatively unlikely that that would amount to a security bug. Anyway, we're not talking about a back-patched fix I think, but something we'd change in a new major release. >> Consider for example that the search path is a, b, c, >> and we have a prepared query "select * from t", and that currently >> refers to b.t. If now someone creates a.t, or renames a.x to a.t, >> then a replan would cause the query to select from a.t ... but there >> was no invalidation event that will impinge on the stored plan, and the >> search_path setting didn't change either. I don't think we want to >> accept the overhead of saying "any DDL anywhere invalidates all cached >> plans", so I don't see any good way to make this case transparent. >> How much do we care? > That may simply be a trade-off that we need to make. I thought about the possibility of issuing an sinval message against a schema each time we create/drop/rename an object belonging to that schema, and then invalidating cached plans if an sinval is received against any schema that was in the search_path at plan time. I think that this would be a watertight fix (in combination with the other invalidation rules mentioned). However, it could still come annoyingly close to "any DDL invalidates all cached plans", at least for apps that keep most of their objects in one schema. Not entirely sure it's worth the implementation hassle and extra sinval traffic. regards, tom lane
* Tom Lane (tgl@sss.pgh.pa.us) wrote: > Anyway, we're not talking about > a back-patched fix I think, but something we'd change in a new major > release. Agreed. > However, it could still come annoyingly > close to "any DDL invalidates all cached plans", at least for apps that > keep most of their objects in one schema. Not entirely sure it's worth > the implementation hassle and extra sinval traffic. I'm really on the fence about this myself. I can certainly see value in doing the invalidations to provide an easy way for individuals to do database updates which include DDL changes without downtime or even having to pause all concurrent activity (consider a create-table, rename old-table, rename-new-into-place, drop old-table approach). That said, that use-case may end up being vanishingly small due to the planned queries themselves (or plpgsql functions) needing to be updated anyway. Perhaps we can punt to the user on this in some way? If a user needed to invalidate these plans w/o having direct access to the client connections involved (but having some appropriate access), could we provide a mechanism for them to do so? Might be a bit much as these complaints don't seem to come up terribly often and restarting backends, while annoying, isn't that bad if it isn't required very often. Thanks, Stephen
Stephen Frost <sfrost@snowman.net> writes: > * Tom Lane (tgl@sss.pgh.pa.us) wrote: >> However, it could still come annoyingly >> close to "any DDL invalidates all cached plans", at least for apps that >> keep most of their objects in one schema. Not entirely sure it's worth >> the implementation hassle and extra sinval traffic. > I'm really on the fence about this myself. I can certainly see value in > doing the invalidations to provide an easy way for individuals to do > database updates which include DDL changes without downtime or even > having to pause all concurrent activity (consider a create-table, rename > old-table, rename-new-into-place, drop old-table approach). That said, > that use-case may end up being vanishingly small due to the planned > queries themselves (or plpgsql functions) needing to be updated anyway. Even more to the point, in most scenarios like that the inval on the object previously referenced in the query would be enough to force replan. AFAICS it's only the interpose-something-new-earlier-in-the- search-path case that is problematic, and that's got to be a corner case (or even an application bug) for most people. I guess one example where it might happen routinely is if you like to create temp tables that mask regular tables, and then reuse prepared queries that originally referenced the regular tables with the expectation that they now reference the temp tables ... but that doesn't seem like great programming practice from here. I'm thinking that the main argument for trying to do this is so that we could say "plan caching is transparent", full stop, with no caveats or corner cases. But removing those caveats is going to cost a fair amount, and it looks like that cost will be wasted for most usage patterns. regards, tom lane
Tom Lane <tgl@sss.pgh.pa.us> writes: > I'm thinking that the main argument for trying to do this is so that we > could say "plan caching is transparent", full stop, with no caveats or > corner cases. But removing those caveats is going to cost a fair > amount, and it looks like that cost will be wasted for most usage > patterns. I think the right thing to do here is aim for transparent plan caching. Now, will the caveats only apply when there has been some live DDL running, or even only DDL that changes schemas (not objects therein)? Really, live DDL is not that frequent, and when you do that, you want transparent replanning. I can't see any use case where it's important to be able to run DDL in a live application yet continue to operate with the old (and in cases wrong) plans. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
On Tue, Jan 22, 2013 at 12:44 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > After reflecting on this a bit, I think that the problem may come from > drawing an unjustified analogy between views and prepared statements. > The code is certainly trying to treat them as the same thing, but > perhaps we shouldn't do that. > > Consider that once you do > create view v as select * from s.t; > the view will continue to refer to the same table object no matter what. > You can rename t, you can rename s, you can move t to a different schema > and then drop s, but the view still knows what t is, because the > reference is by OID. The one thing you can't do is drop t, because the > stored dependency from v to t will prevent that (at least unless you let > it cascade to drop v as well). Views therefore do not have, or need, > any explicit dependencies on either specific schemas or their > creation-time search_path --- they only have dependencies on individual > objects. > > The current plancache code is trying, in a somewhat half-baked fashion, > to preserve those semantics for prepared queries --- that's partly > because it's reusing the dependency mechanism that was designed for > views, and partly because it didn't occur to us to question that model. > But it now strikes me that the model doesn't apply very well, so maybe > we need a new one. The key point that seems to force a different > treatment is that there are no stored (globally-visible) dependencies > for prepared queries, so there's no way to guarantee that referenced > objects don't get dropped. > > We could possibly set things up so that re-executing a prepared query > that references now-dropped objects would throw an error; but what > people seem to prefer is that it should be re-analyzed to see if the > original source text would now refer to a different object. And we're > doing that, but we haven't followed through on the logical implications. > The implication, ISTM, is that we should no longer consider that > referring to the same objects throughout the query's lifespan is a goal > at all. Rather, what we should be trying to do is make the query > preparation transparent, in the sense that you should get the same > results as if you resubmitted the original query text each time. > > In particular, it now seems to me that this makes a good argument > for changing what plancache is doing with search_path. Instead of > re-establishing the original search_path in a rather vain hope that the > same objects will be re-selected by parse analysis, we should consider > that the prepared query has a dependency on the active search path, and > thus force a replan if the effective search path changes. I think that's exactly right, and as Stephen says, likely to be a very significant improvement over the status quo even if it's not perfect. (Basically, I agree with everything Stephen said in his followup emails down to the letter. +1 from me for everything he said.) > I'm not sure that we can make the plan caching 100% transparent, though. > The existing mechanisms will force replan if any object used in the plan > is modified (and fortunately, "modified" includes "renamed", even though > a rename isn't interesting according to the view-centric worldview). > And we can force replan if the search path changes (ie, the effective > list of schema OIDs changes). But there are cases where neither of > those things happens and yet the user might expect a new object to be > selected. Consider for example that the search path is a, b, c, > and we have a prepared query "select * from t", and that currently > refers to b.t. If now someone creates a.t, or renames a.x to a.t, > then a replan would cause the query to select from a.t ... but there > was no invalidation event that will impinge on the stored plan, and the > search_path setting didn't change either. I don't think we want to > accept the overhead of saying "any DDL anywhere invalidates all cached > plans", so I don't see any good way to make this case transparent. > How much do we care? I'd just like to mention that Noah and I left this same case unhandled when we did all of those concurrent DDL improvements for 9.2. A big part of that worked aimed at fixing tthe problem of a DML or DDL statement latching onto an object that had been concurrently dropped, as in the case where someone does BEGIN; DROP old; RENAME new TO old; COMMIT; for any sort of SQL object (table, function, etc.). That code is all now much more watertight than it was before, but the case of creating an object earlier in the search path than an existing object of the same name is still not guaranteed to work correctly - there's no relevant invalidation message, so with the right timing of events, you can still manage to latch onto the object that appears later in the search path instead of the new one added to a schema that appears earlier. So there is precedent for punting that particularly-difficult aspect of problems in this category. To make the cached-plan stuff truly safe against this case, you'd have to replan whenever an object is created in a schema which appears earlier in the search path than some object referenced by the query - except for functions, where you also need to worry about a better candidate showing up anywhere in the search path, before or after the schema where the currently-used object appears. That's a lot of replanning, but maybe not intolerable. For example, consider search_path = a, b. If somebody creates an object in b, we don't need to replan, unless it's a function. But creation of practically anything in a is enough to force a replan. I'm not sure whether we can optimize this that tightly, but if we can it could probably eliminate most of the pain here, because in most cases people are going to have a search path like $user, public, and most of the object creation and deletion is going to happen in public, which doesn't pose the hazard we're concerned about (again, except for object types that allow overloading). -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
On Wed, Jan 23, 2013 at 8:10 AM, Dimitri Fontaine <dimitri@2ndquadrant.fr> wrote: > Really, live DDL is not that frequent, and when you do that, you want > transparent replanning. I can't see any use case where it's important to > be able to run DDL in a live application yet continue to operate with > the old (and in cases wrong) plans. I agree with that, but I think Tom's concern is more with the cost of too-frequent re-planning. The most obvious case in which DDL might be frequent enough to cause an issue here is if there is heavy use of temporary objects - sessions might be rapidly creating and dropping objects in their own schemas. It would be unfortunate if that forced continual replanning of queries in other sessions. I think there could be other cases where this is an issue as well, but the temp-object case is probably the one that's most likely to matter in practice. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
2013/1/23 Robert Haas <robertmhaas@gmail.com>: > On Wed, Jan 23, 2013 at 8:10 AM, Dimitri Fontaine > <dimitri@2ndquadrant.fr> wrote: >> Really, live DDL is not that frequent, and when you do that, you want >> transparent replanning. I can't see any use case where it's important to >> be able to run DDL in a live application yet continue to operate with >> the old (and in cases wrong) plans. > > I agree with that, but I think Tom's concern is more with the cost of > too-frequent re-planning. The most obvious case in which DDL might be > frequent enough to cause an issue here is if there is heavy use of > temporary objects - sessions might be rapidly creating and dropping > objects in their own schemas. It would be unfortunate if that forced > continual replanning of queries in other sessions. I think there > could be other cases where this is an issue as well, but the > temp-object case is probably the one that's most likely to matter in > practice. probably our model is not usual, but probably not hard exception almost all queries that we send to server are CREATE TABLE cachexxx AS SELECT ... Tables are dropped, when data there are possibility so containing data are invalid. Probably any replanning based on DDL can be very problematic in our case. Number of tables in one database can be more than 100K. Regards Pavel > > -- > Robert Haas > EnterpriseDB: http://www.enterprisedb.com > The Enterprise PostgreSQL Company > > > -- > Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) > To make changes to your subscription: > http://www.postgresql.org/mailpref/pgsql-hackers
Robert Haas <robertmhaas@gmail.com> writes: > I agree with that, but I think Tom's concern is more with the cost of > too-frequent re-planning. The most obvious case in which DDL might be > frequent enough to cause an issue here is if there is heavy use of > temporary objects - sessions might be rapidly creating and dropping > objects in their own schemas. It would be unfortunate if that forced > continual replanning of queries in other sessions. I think there > could be other cases where this is an issue as well, but the > temp-object case is probably the one that's most likely to matter in > practice. Yeah, that is probably the major hazard IMO too. The designs sketched in this thread would be sufficient to ensure that DDL in one session's temp schema wouldn't have to invalidate plans in other sessions --- but is that good enough? Your point that the locking code doesn't quite cope with newly-masked objects makes me feel that we could get away with not solving the case for plan caching either. Or at least that we could put off the problem till another day. If we are willing to just change plancache's handling of search_path, that's a small patch that I think is easily doable for 9.3. If we insist on installing schema-level invalidation logic, it's not happening before 9.4. regards, tom lane
On Wed, Jan 23, 2013 at 11:40 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > Yeah, that is probably the major hazard IMO too. The designs sketched > in this thread would be sufficient to ensure that DDL in one session's > temp schema wouldn't have to invalidate plans in other sessions --- but > is that good enough? > > Your point that the locking code doesn't quite cope with newly-masked > objects makes me feel that we could get away with not solving the case > for plan caching either. Or at least that we could put off the problem > till another day. If we are willing to just change plancache's handling > of search_path, that's a small patch that I think is easily doable for > 9.3. If we insist on installing schema-level invalidation logic, it's > not happening before 9.4. I agree with that analysis. FWIW, I am pretty confident that the narrower fix will make quite a few people significantly happier than they are today, so if you're willing to take that on, +1 from me. I believe the search-path-interpolation problem is a sufficiently uncommon case that, in practice, it rarely comes up. That's not to say that we shouldn't ever fix it, but I think the simpler fix will be a 90% solution and people will be happy to have made that much progress this cycle. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
Robert Haas <robertmhaas@gmail.com> writes: > On Wed, Jan 23, 2013 at 11:40 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote: >> Your point that the locking code doesn't quite cope with newly-masked >> objects makes me feel that we could get away with not solving the case >> for plan caching either. Or at least that we could put off the problem >> till another day. If we are willing to just change plancache's handling >> of search_path, that's a small patch that I think is easily doable for >> 9.3. If we insist on installing schema-level invalidation logic, it's >> not happening before 9.4. > I agree with that analysis. FWIW, I am pretty confident that the > narrower fix will make quite a few people significantly happier than > they are today, so if you're willing to take that on, +1 from me. I > believe the search-path-interpolation problem is a sufficiently > uncommon case that, in practice, it rarely comes up. That's not to > say that we shouldn't ever fix it, but I think the simpler fix will be > a 90% solution and people will be happy to have made that much > progress this cycle. Here's a draft patch for that. I've not looked yet to see if there's any documentation that ought to be touched. With this patch, PushOverrideSearchPath/PopOverrideSearchPath are used in only one place: CreateSchemaCommand. And that's a very simple, stylized usage: temporarily push the new schema onto the front of the path. It's tempting to think about replacing that klugy code with some simpler mechanism. But that sort of cleanup should probably be a separate patch. regards, tom lane
Attachment
I wrote: > Here's a draft patch for that. I've not looked yet to see if there's > any documentation that ought to be touched. And now with the documentation. If I don't hear any objections, I plan to commit this tomorrow. regards, tom lane
Attachment
Tom Lane <tgl@sss.pgh.pa.us> writes: > I wrote: >> Here's a draft patch for that. I've not looked yet to see if there's >> any documentation that ought to be touched. > > And now with the documentation. If I don't hear any objections, I plan > to commit this tomorrow. Certainly no objections from me: I had only a cursory look at it, enough to agree with the documented behaviour changes. I think it will also make it safer to use prepared statements in pooling environments. Regards, -- Dimitri Fontaine http://2ndQuadrant.fr PostgreSQL : Expertise, Formation et Support
Re: Prepared statements fail after schema changes with surprising error
From
Peter van Hardenberg
Date:
On Thu, Jan 24, 2013 at 6:17 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
--
Peter van Hardenberg
San Francisco, California
"Everything was beautiful, and nothing hurt." -- Kurt Vonnegut
I wrote:And now with the documentation. If I don't hear any objections, I plan
> Here's a draft patch for that. I've not looked yet to see if there's
> any documentation that ought to be touched.
to commit this tomorrow.
No objections here. Thanks Tom and everyone else for setting this straight.
Peter van Hardenberg
San Francisco, California
"Everything was beautiful, and nothing hurt." -- Kurt Vonnegut