Обсуждение: BUG #6205: ERROR: temporary tables cannot specify a schema name

The following bug has been logged online:

Bug reference:      6205
Logged by:          Abel Abraham Camarillo Ojeda
Email address:      acamari@verlet.org
PostgreSQL version: 9.1.0
Operating system:   OpenBSD-current/amd64
Description:        ERROR:  temporary tables cannot specify a schema name
Details:

Cannot create temporary tables without using CREATE TEMP,
if I hardcode 'pg_temp' as a schema _and_ the table uses
a unique constraint on a column, this does work if the table
doesn't use a unique constraint:

To reproduce:

$ psql -U postgres _n_srv
psql (9.1.0)
Type "help" for help.

_n_srv=# create table pg_temp.c (x int);
CREATE TABLE
_n_srv=# drop table pg_temp.c;
DROP TABLE
_n_srv=# create table pg_temp.c (x int unique);
ERROR:  temporary tables cannot specify a schema name

Excerpts from Abel Abraham Camarillo Ojeda's message of mié sep 14 18:33:33 -0300 2011:

> Cannot create temporary tables without using CREATE TEMP,
> if I hardcode 'pg_temp' as a schema _and_ the table uses
> a unique constraint on a column, this does work if the table
> doesn't use a unique constraint:
>
> To reproduce:
>
> $ psql -U postgres _n_srv
> psql (9.1.0)
> Type "help" for help.
>
> _n_srv=# create table pg_temp.c (x int);
> CREATE TABLE
> _n_srv=# drop table pg_temp.c;
> DROP TABLE
> _n_srv=# create table pg_temp.c (x int unique);
> ERROR:  temporary tables cannot specify a schema name

FWIW this does work in 9.0.

--
Álvaro Herrera <alvherre@commandprompt.com>
The PostgreSQL Company - Command Prompt, Inc.
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

On Wed, Sep 14, 2011 at 4:49 PM, Alvaro Herrera
<alvherre@commandprompt.com> wrote:
>
> Excerpts from Abel Abraham Camarillo Ojeda's message of mi=C3=A9 sep 14 1=
8:33:33 -0300 2011:
>
>> Cannot create temporary tables without using CREATE TEMP,
>> if I hardcode 'pg_temp' as a schema _and_ the table uses
>> a unique constraint on a column, this does work if the table
>> doesn't use a unique constraint:
>>
>> To reproduce:
>>
>> $ psql -U postgres _n_srv
>> psql (9.1.0)
>> Type "help" for help.
>>
>> _n_srv=3D# create table pg_temp.c (x int);
>> CREATE TABLE
>> _n_srv=3D# drop table pg_temp.c;
>> DROP TABLE
>> _n_srv=3D# create table pg_temp.c (x int unique);
>> ERROR: =C2=A0temporary tables cannot specify a schema name
>
> FWIW this does work in 9.0.
>
> --
> =C3=81lvaro Herrera <alvherre@commandprompt.com>
> The PostgreSQL Company - Command Prompt, Inc.
> PostgreSQL Replication, Consulting, Custom Development, 24x7 support
>

Yes, this was working in 9.0.4.

Alvaro Herrera <alvherre@commandprompt.com> writes:
> Excerpts from Abel Abraham Camarillo Ojeda's message of mié sep 14 18:33:33 -0300 2011:
>> _n_srv=# create table pg_temp.c (x int);
>> CREATE TABLE
>> _n_srv=# drop table pg_temp.c;
>> DROP TABLE
>> _n_srv=# create table pg_temp.c (x int unique);
>> ERROR:  temporary tables cannot specify a schema name

> FWIW this does work in 9.0.

In that case I'm betting Robert broke it somewhere in the unlogged-table
changes.

            regards, tom lane

On Thu, Sep 15, 2011 at 12:05 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Alvaro Herrera <alvherre@commandprompt.com> writes:
>> Excerpts from Abel Abraham Camarillo Ojeda's message of mi=E9 sep 14 18:=
33:33 -0300 2011:
>>> _n_srv=3D# create table pg_temp.c (x int);
>>> CREATE TABLE
>>> _n_srv=3D# drop table pg_temp.c;
>>> DROP TABLE
>>> _n_srv=3D# create table pg_temp.c (x int unique);
>>> ERROR: =A0temporary tables cannot specify a schema name
>
>> FWIW this does work in 9.0.
>
> In that case I'm betting Robert broke it somewhere in the unlogged-table
> changes.

Yeah, looks like it.  Will investigate.

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

On Sun, Sep 18, 2011 at 5:10 PM, Robert Haas <robertmhaas@gmail.com> wrote:
> On Thu, Sep 15, 2011 at 12:05 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>> Alvaro Herrera <alvherre@commandprompt.com> writes:
>>> Excerpts from Abel Abraham Camarillo Ojeda's message of mi=E9 sep 14 18=
:33:33 -0300 2011:
>>>> _n_srv=3D# create table pg_temp.c (x int);
>>>> CREATE TABLE
>>>> _n_srv=3D# drop table pg_temp.c;
>>>> DROP TABLE
>>>> _n_srv=3D# create table pg_temp.c (x int unique);
>>>> ERROR: =A0temporary tables cannot specify a schema name
>>
>>> FWIW this does work in 9.0.
>>
>> In that case I'm betting Robert broke it somewhere in the unlogged-table
>> changes.
>
> Yeah, looks like it. =A0Will investigate.

I committed a localized fix for this.  Basically, it now double-checks
that the schema name isn't the one that would have been forced by the
temp-ness, and doesn't throw an error if they match.

However, I'm wondering if it would be better to instead (a) jigger
things so that RangeVarAdjustRelationPersistence sets the schema name
to NULL if it changes the relpersistence, or (b) just remove the
cross-check in RangeVarGetRelid() altogether (or demote it to an elog
or assert).  It's not clear to me under what circumstances this check
can actually be hit any more, and if there are any they probably need
to be fixed further upstream.  However, I didn't feel safe doing
either of those things without discussion, especially in a back-branch
the day before we wrap 9.1.1.

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

Robert Haas <robertmhaas@gmail.com> writes:
> On Sun, Sep 18, 2011 at 5:10 PM, Robert Haas <robertmhaas@gmail.com> wrote:
>> On Thu, Sep 15, 2011 at 12:05 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>> In that case I'm betting Robert broke it somewhere in the unlogged-table
>>> changes.

>> Yeah, looks like it.  Will investigate.

> I committed a localized fix for this.  Basically, it now double-checks
> that the schema name isn't the one that would have been forced by the
> temp-ness, and doesn't throw an error if they match.

Keeping in mind that this worked before ... did you take the approach of
seeing exactly why it worked before, and why your changes broke it?
The patch you committed looked like it was introducing the functionality
from scratch, which should not have been the case.

            regards, tom lane

On Wed, Sep 21, 2011 at 11:35 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
> Robert Haas <robertmhaas@gmail.com> writes:
>> On Sun, Sep 18, 2011 at 5:10 PM, Robert Haas <robertmhaas@gmail.com> wro=
te:
>>> On Thu, Sep 15, 2011 at 12:05 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:
>>>> In that case I'm betting Robert broke it somewhere in the unlogged-tab=
le
>>>> changes.
>
>>> Yeah, looks like it. =A0Will investigate.
>
>> I committed a localized fix for this. =A0Basically, it now double-checks
>> that the schema name isn't the one that would have been forced by the
>> temp-ness, and doesn't throw an error if they match.
>
> Keeping in mind that this worked before ... did you take the approach of
> seeing exactly why it worked before, and why your changes broke it?
> The patch you committed looked like it was introducing the functionality
> from scratch, which should not have been the case.

See the commit message for 5da79169d3e9f0fab47da03318c44075b3f824c5,
and also the subsequent fix in
6f1be5a67a758499beab0082b6e63b3040913268.

I believe that the problem here is that we now need to set
relpersistence correctly much sooner than relistemp previously needed
to be hammered down.  I think that in the 9.0 code, it was fine to
have a RangeVar with istemp =3D false and schemaname =3D pg_temp, and it
all worked itself out.  For example, in 9.0,
RelationBuildLocalRelation() did this:

        rel->rd_istemp =3D isTempOrToastNamespace(relnamespace);

Well, that trick doesn't work any more.  In 9.0, you can infer the
persistence of the table from knowing what namespace it's in; and it
doesn't matter whether it ended up in that namespace because of an
explicit TEMP specification or because somebody gave a schema name of
pg_temp, or even a schema name of pg_temp_${MyBackendId}.  But in 9.1,
that's no good: there are three possible relpersistence values, and
the namespace isn't enough to know what value has to be used.  So it
must be passed in, which means that it must already be set correctly.
And setting it correctly earlier is knocking over the applecart.

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