Обсуждение: Prepared statements fail after schema changes with surprising error

Поиск
Список
Период
Сортировка

Prepared statements fail after schema changes with surprising error

От
Peter van Hardenberg
Дата:
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.

-pvh

--
Peter van Hardenberg
San Francisco, California
"Everything was beautiful, and nothing hurt." -- Kurt Vonnegut

Re: Prepared statements fail after schema changes with surprising error

От
Tom Lane
Дата:
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



Re: Prepared statements fail after schema changes with surprising error

От
Peter Geoghegan
Дата:
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

От
"Dickson S. Guedes"
Дата:
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



Re: Prepared statements fail after schema changes with surprising error

От
Tom Lane
Дата:
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

От
Peter van Hardenberg
Дата:
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 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:
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



--
Peter van Hardenberg
San Francisco, California
"Everything was beautiful, and nothing hurt." -- Kurt Vonnegut

Re: Prepared statements fail after schema changes with surprising error

От
Peter van Hardenberg
Дата:
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:

-- 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 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:
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



--
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

Re: Prepared statements fail after schema changes with surprising error

От
Tom Lane
Дата:
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



Re: Prepared statements fail after schema changes with surprising error

От
Robert Haas
Дата:
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



Re: Prepared statements fail after schema changes with surprising error

От
Dimitri Fontaine
Дата:
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



Re: Prepared statements fail after schema changes with surprising error

От
Andres Freund
Дата:
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

От
Dimitri Fontaine
Дата:
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



Re: Prepared statements fail after schema changes with surprising error

От
Tom Lane
Дата:
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



Re: Prepared statements fail after schema changes with surprising error

От
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



Re: Prepared statements fail after schema changes with surprising error

От
Stephen Frost
Дата:
* 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

Re: Prepared statements fail after schema changes with surprising error

От
Tom Lane
Дата:
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



Re: Prepared statements fail after schema changes with surprising error

От
Stephen Frost
Дата:
* 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

Re: Prepared statements fail after schema changes with surprising error

От
Tom Lane
Дата:
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



Re: Prepared statements fail after schema changes with surprising error

От
Dimitri Fontaine
Дата:
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



Re: Prepared statements fail after schema changes with surprising error

От
Robert Haas
Дата:
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



Re: Prepared statements fail after schema changes with surprising error

От
Robert Haas
Дата:
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



Re: Prepared statements fail after schema changes with surprising error

От
Pavel Stehule
Дата:
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



Re: Prepared statements fail after schema changes with surprising error

От
Tom Lane
Дата:
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



Re: Prepared statements fail after schema changes with surprising error

От
Robert Haas
Дата:
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



Re: Prepared statements fail after schema changes with surprising error

От
Tom Lane
Дата:
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


Вложения

Re: Prepared statements fail after schema changes with surprising error

От
Tom Lane
Дата:
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


Вложения

Re: Prepared statements fail after schema changes with surprising error

От
Dimitri Fontaine
Дата:
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

От
Peter van Hardenberg
Дата:
On Thu, Jan 24, 2013 at 6:17 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
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.


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