Thread: Plan invalidation vs temp sequences

Plan invalidation vs temp sequences

From
Tom Lane
Date:
In bug #3662
http://archives.postgresql.org/pgsql-bugs/2007-10/msg00047.php

we see that it doesn't work to do nextval('seq') on a temp sequence
in a plpgsql function except via EXECUTE, because the sequence OID gets
embedded into the cached plan, same as any other temp table.  This is to
be expected in existing releases, but I notice it still doesn't work in
CVS HEAD, which is a tad annoying given the existence of the plan inval
mechanism.  The reason is that plancache.c only searches the plan's
RangeTable for references to a just-invalidated relation; and the
argument of nextval() isn't mentioned in the RangeTable (it is in fact
just a Const of type regclass).

There doesn't seem to be any very nice way to fix this.  There is
not any existing support mechanism (comparable to query_tree_walker)
for scanning whole plan trees, which means that searching a cached plan
for regclass Consts is going to involve a chunk of new code no matter
how we approach it.  We might want to do that someday --- in particular,
if we ever try to extend the plan inval mechanism to react to
redefinitions of non-table objects, we'd likely need some such thing
anyway.  I'm disinclined to try to do it for 8.3 though.  The use-case
for temp sequences seems a bit narrow and there are several workarounds
(see followups to bug report), so I'm feeling this is a
fix-some-other-day kind of issue.

Comments?
        regards, tom lane


Re: Plan invalidation vs temp sequences

From
"Heikki Linnakangas"
Date:
Tom Lane wrote:
> ... We might want to do that someday --- in particular,
> if we ever try to extend the plan inval mechanism to react to
> redefinitions of non-table objects, we'd likely need some such thing
> anyway.  I'm disinclined to try to do it for 8.3 though.  The use-case
> for temp sequences seems a bit narrow and there are several workarounds
> (see followups to bug report), so I'm feeling this is a
> fix-some-other-day kind of issue.

Agreed. I was a bit worried about this kind of usage:

CREATE OR REPLACE FUNCTION testfunc(val int) RETURNS int AS $$
DECLARE
BEGIN CREATE TEMPORARY SEQUENCE tempseq; CREATE TEMPORARY TABLE inttable (key integer DEFAULT
nextval('tempseq'), data text); INSERT INTO inttable (data) VALUES ('foo'); DROP TABLE inttable; DROP SEQUENCE tempseq;
return1;
 
END;
$$ LANGUAGE plpgsql;

but that seems to work, because creating/dropping the temp table
triggers the plan invalidation.

--  Heikki Linnakangas EnterpriseDB   http://www.enterprisedb.com


Re: Plan invalidation vs temp sequences

From
Gregory Stark
Date:
"Tom Lane" <tgl@sss.pgh.pa.us> writes:

> There doesn't seem to be any very nice way to fix this.  There is
> not any existing support mechanism (comparable to query_tree_walker)
> for scanning whole plan trees, which means that searching a cached plan
> for regclass Consts is going to involve a chunk of new code no matter
> how we approach it.  We might want to do that someday --- in particular,
> if we ever try to extend the plan inval mechanism to react to
> redefinitions of non-table objects, we'd likely need some such thing
> anyway.  I'm disinclined to try to do it for 8.3 though.  The use-case
> for temp sequences seems a bit narrow and there are several workarounds
> (see followups to bug report), so I'm feeling this is a
> fix-some-other-day kind of issue.

Given that sequences are in fact relations is there some way to work around
the issue at least in this case by stuffing the sequence's relid someplace
which the plan invalldation code can check for it?


--  Gregory Stark EnterpriseDB          http://www.enterprisedb.com


Re: Plan invalidation vs temp sequences

From
"Florian G. Pflug"
Date:
Gregory Stark wrote:
> "Tom Lane" <tgl@sss.pgh.pa.us> writes:
> 
>> There doesn't seem to be any very nice way to fix this.  There is
>> not any existing support mechanism (comparable to query_tree_walker)
>> for scanning whole plan trees, which means that searching a cached plan
>> for regclass Consts is going to involve a chunk of new code no matter
>> how we approach it.  We might want to do that someday --- in particular,
>> if we ever try to extend the plan inval mechanism to react to
>> redefinitions of non-table objects, we'd likely need some such thing
>> anyway.  I'm disinclined to try to do it for 8.3 though.  The use-case
>> for temp sequences seems a bit narrow and there are several workarounds
>> (see followups to bug report), so I'm feeling this is a
>> fix-some-other-day kind of issue.
> 
> Given that sequences are in fact relations is there some way to work around
> the issue at least in this case by stuffing the sequence's relid someplace
> which the plan invalldation code can check for it?

Hm... couldn't this be worked around by doing
create or replace function dynamic_oid(text) returning regclass as
'select $1::regclass' language 'pl/pgsql' stable;
And then writing
nextval(dynamic_oid('mysequence')).

I didn't test this, but it it actually works, maybe we should just stick this
into the docs somewhere. It's probably too late to add that function to the 
backend, though...

As long as mysequence is really a temporary sequence, this wont even have
searchpath issues I think, because those are always on top of the searchpatch
anyway, aren't they?

greetings, Florian Pflug



Re: Plan invalidation vs temp sequences

From
Tom Lane
Date:
"Florian G. Pflug" <fgp@phlo.org> writes:
> Gregory Stark wrote:
>> Given that sequences are in fact relations is there some way to work around
>> the issue at least in this case by stuffing the sequence's relid someplace
>> which the plan invalldation code can check for it?

Well, we *have* the sequence's Oid in the regclass constant, the problem
is the difficulty of digging through the plan tree to find it.  I did
consider having the planner extract it and save it aside somewhere, but
there doesn't seem to be any very convenient place to do that, short of
an extra traversal of the query tree, which is pretty annoying/expensive
for data that will probably never be needed for most queries.

> Hm... couldn't this be worked around by doing
> create or replace function dynamic_oid(text) returning regclass as
> 'select $1::regclass' language 'pl/pgsql' stable;
> And then writing
> nextval(dynamic_oid('mysequence')).

The cast-to-text workaround that I suggested does exactly that.
        regards, tom lane


Re: Plan invalidation vs temp sequences

From
Tom Lane
Date:
I wrote:
> Well, we *have* the sequence's Oid in the regclass constant, the problem
> is the difficulty of digging through the plan tree to find it.  I did
> consider having the planner extract it and save it aside somewhere, but
> there doesn't seem to be any very convenient place to do that, short of
> an extra traversal of the query tree, which is pretty annoying/expensive
> for data that will probably never be needed for most queries.

Actually ... now that I've consumed a bit more caffeine, it seems this
could be done relatively cheaply in setrefs.c.  We could add a
list-of-relation-OIDs to PlannedStmt, and charge setrefs.c with creating
the list, and simplify plancache.c to just use list_member_oid() instead
of groveling over the rangetable for itself.  I'll go try that out.
        regards, tom lane


Re: Plan invalidation vs temp sequences

From
"Florian G. Pflug"
Date:
Gregory Stark wrote:
> "Tom Lane" <tgl@sss.pgh.pa.us> writes:
> 
>> There doesn't seem to be any very nice way to fix this.  There is
>> not any existing support mechanism (comparable to query_tree_walker)
>> for scanning whole plan trees, which means that searching a cached plan
>> for regclass Consts is going to involve a chunk of new code no matter
>> how we approach it.  We might want to do that someday --- in particular,
>> if we ever try to extend the plan inval mechanism to react to
>> redefinitions of non-table objects, we'd likely need some such thing
>> anyway.  I'm disinclined to try to do it for 8.3 though.  The use-case
>> for temp sequences seems a bit narrow and there are several workarounds
>> (see followups to bug report), so I'm feeling this is a
>> fix-some-other-day kind of issue.
> 
> Given that sequences are in fact relations is there some way to work around
> the issue at least in this case by stuffing the sequence's relid someplace
> which the plan invalldation code can check for it?

Hm... couldn't this be worked around by doing
create or replace function dynamic_oid(text) returning regclass as
'select $1::regclass' language 'pl/pgsql' stable;
And then writing
nextval(dynamic_oid('mysequence')).

I didn't test this, but it it actually works, maybe we should just stick this
into the docs somewhere. It's probably too late to add that function to the 
backend, though...

As long as mysequence is really a temporary sequence, this wont even have
searchpath issues I think, because those are always on top of the searchpatch
anyway, aren't they?

greetings, Florian Pflug


---------------------------(end of broadcast)---------------------------
TIP 1: if posting/reading through Usenet, please send an appropriate      subscribe-nomail command to
majordomo@postgresql.orgso that your      message can get through to the mailing list cleanly