Re: BUG #3403: ver 8.2 can't add serial column to temp table,but 8.1 can - Mailing list pgsql-bugs

From Heikki Linnakangas
Subject Re: BUG #3403: ver 8.2 can't add serial column to temp table,but 8.1 can
Date
Msg-id 467BA205.6090709@enterprisedb.com
Whole thread Raw
In response to Re: BUG #3403: ver 8.2 can't add serial column to temp table,but 8.1 can  (Zdenek Kotala <Zdenek.Kotala@Sun.COM>)
Responses Re: BUG #3403: ver 8.2 can't add serial column to temp table,but 8.1 can  (Zdenek Kotala <Zdenek.Kotala@Sun.COM>)
Re: BUG #3403: ver 8.2 can't add serial column to temp table,but 8.1 can  (Tom Lane <tgl@sss.pgh.pa.us>)
List pgsql-bugs
Zdenek Kotala wrote:
> Jasen Betts wrote:
>> template1=# create temp table foo ( x text);
>> CREATE TABLE
>> template1=# alter table foo add column y text ;
>> ALTER TABLE
>> template1=# alter table foo add column id serial;
>> NOTICE:  ALTER TABLE will create implicit sequence "foo_id_seq" for
>> serial
>> colum
>> n "foo.id"
>> ERROR:  relation "public.foo" does not exist
>> template1=#
>
> It does not work on 8.2.4 as well. It seems PG lost information about
> schema and try to use default schema. Following command works well:
>
> alter table pg_temp.foo add column id serial;
>
> It could be use as workaround.

8.1 creates the sequence in wrong schema:

postgres=# create temp table foo ( x text);
CREATE TABLE
postgres=# alter table foo add column id serial;
NOTICE:  ALTER TABLE will create implicit sequence "foo_id_seq" for
serial column "foo.id"
ALTER TABLE
postgres=# \d
               List of relations
   Schema   |    Name    |   Type   |  Owner
-----------+------------+----------+----------
  pg_temp_1 | foo        | table    | hlinnaka
  public    | foo_id_seq | sequence | hlinnaka
(2 rows)

The problem seems to be in transformColumnDefinition, where the schema
of the to-be-created sequence is determined from the relation name
given. The default creation schema is used, if the user didn't specify
the schame of the table explicitly, but since it's an ALTER TABLE, it
really should use the schema of the existing table.

Patch against 8.2 attached, seems to apply to 8.1 and CVS head though I
haven't tested them.. This is not my area of expertise, so I'm not 100%
sure this is the right way to fix it.

--
   Heikki Linnakangas
   EnterpriseDB   http://www.enterprisedb.com
Index: src/backend/parser/analyze.c
===================================================================
RCS file: /home/hlinnaka/pgcvsrepository/pgsql/src/backend/parser/analyze.c,v
retrieving revision 1.353
diff -c -r1.353 analyze.c
*** src/backend/parser/analyze.c    11 Oct 2006 16:42:59 -0000    1.353
--- src/backend/parser/analyze.c    22 Jun 2007 10:16:20 -0000
***************
*** 69,74 ****
--- 69,75 ----
  {
      const char *stmtType;        /* "CREATE TABLE" or "ALTER TABLE" */
      RangeVar   *relation;        /* relation to create */
+     Oid            namespace;        /* namespace of relation to alter */
      List       *inhRelations;    /* relations to inherit from */
      bool        hasoids;        /* does relation have an OID column? */
      bool        isalter;        /* true if altering existing table */
***************
*** 945,950 ****
--- 946,952 ----

      cxt.stmtType = "CREATE TABLE";
      cxt.relation = stmt->relation;
+     cxt.namespace = InvalidOid;
      cxt.inhRelations = stmt->inhRelations;
      cxt.isalter = false;
      cxt.columns = NIL;
***************
*** 1087,1093 ****
           * quite unlikely to be a problem, especially since few people would
           * need two serial columns in one table.
           */
!         snamespaceid = RangeVarGetCreationNamespace(cxt->relation);
          snamespace = get_namespace_name(snamespaceid);
          sname = ChooseRelationName(cxt->relation->relname,
                                     column->colname,
--- 1089,1098 ----
           * quite unlikely to be a problem, especially since few people would
           * need two serial columns in one table.
           */
!         if (OidIsValid(cxt->namespace))
!            snamespaceid = cxt->namespace;
!         else
!            snamespaceid = RangeVarGetCreationNamespace(cxt->relation);
          snamespace = get_namespace_name(snamespaceid);
          sname = ChooseRelationName(cxt->relation->relname,
                                     column->colname,
***************
*** 3010,3015 ****
--- 3015,3021 ----
      List       *newcmds = NIL;
      bool        skipValidation = true;
      AlterTableCmd *newcmd;
+     Relation    relation;

      cxt.stmtType = "ALTER TABLE";
      cxt.relation = stmt->relation;
***************
*** 3024,3029 ****
--- 3030,3045 ----
      cxt.alist = NIL;
      cxt.pkey = NULL;

+     relation = heap_openrv(stmt->relation, AccessShareLock);
+     if (relation->rd_rel->relkind != RELKIND_RELATION)
+         ereport(ERROR,
+                 (errcode(ERRCODE_WRONG_OBJECT_TYPE),
+                  errmsg("relation \"%s\" is not a table",
+                         stmt->relation->relname)));
+
+     cxt.namespace = RelationGetNamespace(relation);
+
+
      /*
       * The only subtypes that currently require parse transformation handling
       * are ADD COLUMN and ADD CONSTRAINT.  These largely re-use code from
***************
*** 3166,3171 ****
--- 3182,3194 ----
      *extras_before = list_concat(*extras_before, cxt.blist);
      *extras_after = list_concat(cxt.alist, *extras_after);

+     /*
+      * Close the parent rel, but keep our AccessShareLock on it until xact
+      * commit.    That will prevent someone else from deleting or ALTERing the
+      * table before we get to execute the changes.
+      */
+     heap_close(relation, NoLock);
+
      return qry;
  }


pgsql-bugs by date:

Previous
From: Katsuhiko Okano
Date:
Subject: Re: BUG #3401: PITR does not work in the case ofrecovery_target_xid = 'SELECT_only_transaction_ID'
Next
From: Heikki Linnakangas
Date:
Subject: Temp table woes