Re: relpersistence and temp table

From: Robert Haas <robertmhaas(at)gmail(dot)com>
To: Amit Khandekar <amit(dot)khandekar(at)enterprisedb(dot)com>
Cc: pgsql-hackers(at)postgresql(dot)org
Subject: Re: relpersistence and temp table
Date: 2011-07-01 17:59:48
Message-ID: BANLkTimDZHJJ-Ha_uUYmWV21_azxh8Cgfw@mail.gmail.com
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

On Fri, Jul 1, 2011 at 10:32 AM, Robert Haas <robertmhaas(at)gmail(dot)com> wrote:
> On Fri, Jul 1, 2011 at 8:06 AM, Amit Khandekar
> <amit(dot)khandekar(at)enterprisedb(dot)com> wrote:
>> In 9.1, if a table is created using an explicit pg_temp qualification,
>> the pg_class.relpersistence is marked 'p', not 't'.
>
> That's a bug.  Thanks for the report.

OK, so I think the problem here is that, in 9.0, it was possible to
figure out what value relistemp should take at a very late date,
because it was entirely a function of the schema name. A temporary
schema implies relistemp = true, while a non-temporary schema implies
relistemp = false. However, in 9.1, that clearly won't do, since
unlogged and permanent tables can share the same schema. Moreover, by
the time we get as far as RelationBuildLocalRelation(), we've already
made lots of other decisions based on relpersistence, so it seems that
we need to make this correct as early as possible. It's not feasible
to do that in the parser, because the creation namespace could also
come from search_path:

SET search_path = pg_temp;
CREATE TABLE foo (a int);

So it seems we can't fix this any earlier than
RangeVarGetCreationNamespace(). In the attached patch, I took
basically that approach, but created a new function
RangeVarAdjustRelationPersistence() that does the actual adjusting
(since de-constifying RangeVarGetCreationNamespace() didn't seem
smart), plus adds a bunch of additional sanity-checking that I
previously overlooked. Namely, it forbids:

- creating unlogged tables in temporary schemas
- creating relations in temporary schemas of other sessions

On the other hand, it does allow CREATE TEMP TABLE pg_temp.foo(a int),
which was somewhat pointlessly forbidden by previous releases. In
short, the code now checks directly what it used to check by
inference: that you're not creating a temporary table in a permanent
schema, or the other way around.

I also rearranged a few other bits of code to make sure that the
appropriate fixups happen BEFORE we enforce the condition that
temporary tables mustn't be created in security-restricted contexts.

--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company

Attachment Content-Type Size
fix-relpersistence-early.patch application/octet-stream 11.0 KB

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Jaime Casanova 2011-07-01 18:07:33 Re: add support for logging current role (what to review?)
Previous Message Torello Querci 2011-07-01 17:31:30 Re: pg_terminate_backend and pg_cancel_backend by not administrator user