Assertion failure from plan cache invalidation

This leads to assertion failure, on versions 8.3 onwards where plan
cache invalidation was introduced:

postgres=# CREATE FUNCTION ttfunc() RETURNS VOID AS $$
begin
PERFORM * FROM temptable;
END;
$$ LANGUAGE plpgsql;
CREATE FUNCTION
postgres=# begin;
BEGIN
postgres=# CREATE TEMPORARY TABLE temptable (id int4);
CREATE TABLE
postgres=# SELECT ttfunc();
ttfunc
--------

(1 row)

postgres=# rollback;
ROLLBACK
postgres=# SELECT ttfunc();
server closed the connection unexpectedly
This probably means the server terminated abnormally
before or while processing the request.

TRAP: FailedAssertion("!(((bool) ((myTempNamespace) != ((Oid) 0))))",
File: "namespace.c", Line: 2705)

PushOverrideSearchPath() assumes that if the temporary namespace existed
when an override search path was memorized with GetOverrideSearchPath(),
it must still exist. That's not true in the above example, rolling back
the transaction that the temporary namespace was created in drops it.

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

Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com> writes:
> PushOverrideSearchPath() assumes that if the temporary namespace existed
> when an override search path was memorized with GetOverrideSearchPath(),
> it must still exist. That's not true in the above example, rolling back
> the transaction that the temporary namespace was created in drops it.

Hm ... seems like there are two possibilities here. We could forcibly
recreate the temp schema, or we could just ignore the useTemp flag.

The former would more nearly approximate the situation that prevailed
at GetOverrideSearchPath() time, but on the other hand it's not clear
that it's a good idea for PushOverrideSearchPath() to have side-effects
like that. I *think* that it'd be safe, at least for the two existing
callers, but ...

In the plancache case it could be argued that there's no real reason
to recreate the temp schema: it would necessarily be empty, so it
couldn't affect the results of planning anyhow. So the second solution
would work just fine for the current usage.

Thoughts?

regards, tom lane

On 13/08/10 18:18, Tom Lane wrote:
> Heikki Linnakangas<heikki(dot)linnakangas(at)enterprisedb(dot)com> writes:
>> PushOverrideSearchPath() assumes that if the temporary namespace existed
>> when an override search path was memorized with GetOverrideSearchPath(),
>> it must still exist. That's not true in the above example, rolling back
>> the transaction that the temporary namespace was created in drops it.
>
> Hm ... seems like there are two possibilities here. We could forcibly
> recreate the temp schema, or we could just ignore the useTemp flag.

Yeah, I was undecided on that too.

> The former would more nearly approximate the situation that prevailed
> at GetOverrideSearchPath() time, but on the other hand it's not clear
> that it's a good idea for PushOverrideSearchPath() to have side-effects
> like that. I *think* that it'd be safe, at least for the two existing
> callers, but ...
>
> In the plancache case it could be argued that there's no real reason
> to recreate the temp schema: it would necessarily be empty, so it
> couldn't affect the results of planning anyhow. So the second solution
> would work just fine for the current usage.
>
> Thoughts?

Let's do the latter, add a comment noting that, and extend it later if
necessary.

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

Heikki Linnakangas <heikki(dot)linnakangas(at)enterprisedb(dot)com> writes:
> On 13/08/10 18:18, Tom Lane wrote:
>> Heikki Linnakangas<heikki(dot)linnakangas(at)enterprisedb(dot)com> writes:
>>> PushOverrideSearchPath() assumes that if the temporary namespace existed
>>> when an override search path was memorized with GetOverrideSearchPath(),
>>> it must still exist. That's not true in the above example, rolling back
>>> the transaction that the temporary namespace was created in drops it.
>>
>> Hm ... seems like there are two possibilities here. We could forcibly
>> recreate the temp schema, or we could just ignore the useTemp flag.

> Yeah, I was undecided on that too.

>> The former would more nearly approximate the situation that prevailed
>> at GetOverrideSearchPath() time, but on the other hand it's not clear
>> that it's a good idea for PushOverrideSearchPath() to have side-effects
>> like that. I *think* that it'd be safe, at least for the two existing
>> callers, but ...
>>
>> In the plancache case it could be argued that there's no real reason
>> to recreate the temp schema: it would necessarily be empty, so it
>> couldn't affect the results of planning anyhow. So the second solution
>> would work just fine for the current usage.
>>
>> Thoughts?

> Let's do the latter, add a comment noting that, and extend it later if
> necessary.

That's the way I was leaning, too. Will take care of it.

regards, tom lane