Fix failure of "ALTER TABLE t ADD COLUMN c serial" when done by non-owner.

The implicitly created sequence was created as owned by the current user,
who could be different from the table owner, eg if current user is a
superuser or some member of the table's owning role.  This caused sanity
checks in the SEQUENCE OWNED BY code to spit up.  Although possibly we
don't need those sanity checks, the safest fix seems to be to make sure
the implicit sequence is assigned the same owner role as the table has.
(We still do all permissions checks as the current user, however.)
Per report from Josh Berkus.

Back-patch to 9.0.  The bug goes back to the invention of SEQUENCE OWNED BY
in 8.2, but the fix requires an API change for DefineRelation(), which seems
to have potential for breaking third-party code if done in a minor release.
Given the lack of prior complaints, it's probably not worth fixing in the
stable branches.

diff --git a/src/backend/commands/sequence.c b/src/backend/commands/sequence.c
index fc2446997a7fcd27dbd2922c82536146f0b025f4..292a427449321bd4bb282ca3f56b5816b6799a80 100644 (file)
--- a/src/backend/commands/sequence.c
+++ b/src/backend/commands/sequence.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *   $PostgreSQL: pgsql/src/backend/commands/sequence.c,v 1.168 2010/02/20 21:24:02 tgl Exp $
+ *   $PostgreSQL: pgsql/src/backend/commands/sequence.c,v 1.168.4.1 2010/08/18 18:35:29 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -204,7 +204,7 @@ DefineSequence(CreateSeqStmt *seq)
    stmt->oncommit = ONCOMMIT_NOOP;
    stmt->tablespacename = NULL;
 
-   seqoid = DefineRelation(stmt, RELKIND_SEQUENCE);
+   seqoid = DefineRelation(stmt, RELKIND_SEQUENCE, seq->ownerId);
 
    rel = heap_open(seqoid, AccessExclusiveLock);
    tupDesc = RelationGetDescr(rel);

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 4c79f8f7e4de3a3b02b79c4777368e6d343aaafb..c4d81bb4fecb6c1edb58ca8eebaaa5478028e22d 100644 (file)
--- a/src/backend/commands/tablecmds.c
+++ b/src/backend/commands/tablecmds.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *   $PostgreSQL: pgsql/src/backend/commands/tablecmds.c,v 1.332.2.3 2010/08/03 15:47:09 rhaas Exp $
+ *   $PostgreSQL: pgsql/src/backend/commands/tablecmds.c,v 1.332.2.4 2010/08/18 18:35:29 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -340,11 +340,21 @@ static const char *storage_name(char c);
  *     DefineRelation
  *             Creates a new relation.
  *
+ * stmt carries parsetree information from an ordinary CREATE TABLE statement.
+ * The other arguments are used to extend the behavior for other cases:
+ * relkind: relkind to assign to the new relation
+ * ownerId: if not InvalidOid, use this as the new relation's owner.
+ *
+ * Note that permissions checks are done against current user regardless of
+ * ownerId.  A nonzero ownerId is used when someone is creating a relation
+ * "on behalf of" someone else, so we still want to see that the current user
+ * has permissions to do it.
+ *
  * If successful, returns the OID of the new relation.
  * ----------------------------------------------------------------
  */
 Oid
-DefineRelation(CreateStmt *stmt, char relkind)
+DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId)
 {
    char        relname[NAMEDATALEN];
    Oid         namespaceId;
@@ -444,6 +454,10 @@ DefineRelation(CreateStmt *stmt, char relkind)
                (errcode(ERRCODE_INVALID_PARAMETER_VALUE),
                 errmsg("only shared relations can be placed in pg_global tablespace")));
 
+   /* Identify user ID that will own the table */
+   if (!OidIsValid(ownerId))
+       ownerId = GetUserId();
+
    /*
     * Parse and validate reloptions, if any.
     */
@@ -536,7 +550,7 @@ DefineRelation(CreateStmt *stmt, char relkind)
                                          InvalidOid,
                                          InvalidOid,
                                          ofTypeId,
-                                         GetUserId(),
+                                         ownerId,
                                          descriptor,
                                          list_concat(cookedDefaults,
                                                      old_constraints),

diff --git a/src/backend/commands/typecmds.c b/src/backend/commands/typecmds.c
index 8a85e79ea659481295fb744b3d83ab82f832ecb7..abde562fe30a487553f71361a2b73c6f740218bd 100644 (file)
--- a/src/backend/commands/typecmds.c
+++ b/src/backend/commands/typecmds.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *   $PostgreSQL: pgsql/src/backend/commands/typecmds.c,v 1.148 2010/02/26 02:00:40 momjian Exp $
+ *   $PostgreSQL: pgsql/src/backend/commands/typecmds.c,v 1.148.4.1 2010/08/18 18:35:29 tgl Exp $
  *
  * DESCRIPTION
  *   The "DefineFoo" routines take the parse tree and pick out the
@@ -1546,7 +1546,7 @@ DefineCompositeType(const RangeVar *typevar, List *coldeflist)
    /*
     * Finally create the relation.  This also creates the type.
     */
-   return DefineRelation(createStmt, RELKIND_COMPOSITE_TYPE);
+   return DefineRelation(createStmt, RELKIND_COMPOSITE_TYPE, InvalidOid);
 }
 
 /*

diff --git a/src/backend/commands/view.c b/src/backend/commands/view.c
index 900def0148060ee331d38990226811fe84364198..c291529f7d0fb8c012e9064d3e8ead74a5d24ff4 100644 (file)
--- a/src/backend/commands/view.c
+++ b/src/backend/commands/view.c
@@ -8,7 +8,7 @@
  *
  *
  * IDENTIFICATION
- *   $PostgreSQL: pgsql/src/backend/commands/view.c,v 1.120 2010/01/02 16:57:40 momjian Exp $
+ *   $PostgreSQL: pgsql/src/backend/commands/view.c,v 1.120.6.1 2010/08/18 18:35:29 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -239,7 +239,7 @@ DefineVirtualRelation(const RangeVar *relation, List *tlist, bool replace)
         * existing view, so we don't need more code to complain if "replace"
         * is false).
         */
-       return DefineRelation(createStmt, RELKIND_VIEW);
+       return DefineRelation(createStmt, RELKIND_VIEW, InvalidOid);
    }
 }
 

diff --git a/src/backend/nodes/copyfuncs.c b/src/backend/nodes/copyfuncs.c
index 85a8fca97a9e6b4db4d1bca5ec20d40463724fca..3e237c11d9bd95734d05548abdcf831543c54a18 100644 (file)
--- a/src/backend/nodes/copyfuncs.c
+++ b/src/backend/nodes/copyfuncs.c
@@ -15,7 +15,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *   $PostgreSQL: pgsql/src/backend/nodes/copyfuncs.c,v 1.464.4.1 2010/08/18 15:22:00 tgl Exp $
+ *   $PostgreSQL: pgsql/src/backend/nodes/copyfuncs.c,v 1.464.4.2 2010/08/18 18:35:30 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -3002,6 +3002,7 @@ _copyCreateSeqStmt(CreateSeqStmt *from)
 
    COPY_NODE_FIELD(sequence);
    COPY_NODE_FIELD(options);
+   COPY_SCALAR_FIELD(ownerId);
 
    return newnode;
 }

diff --git a/src/backend/nodes/equalfuncs.c b/src/backend/nodes/equalfuncs.c
index e97a3ea9daaaf5c98c4a5a302b4932e2e63a6847..dc5f88375aa2f98901e9579119fbbed674a65a86 100644 (file)
--- a/src/backend/nodes/equalfuncs.c
+++ b/src/backend/nodes/equalfuncs.c
@@ -22,7 +22,7 @@
  * Portions Copyright (c) 1994, Regents of the University of California
  *
  * IDENTIFICATION
- *   $PostgreSQL: pgsql/src/backend/nodes/equalfuncs.c,v 1.385 2010/02/26 02:00:43 momjian Exp $
+ *   $PostgreSQL: pgsql/src/backend/nodes/equalfuncs.c,v 1.385.4.1 2010/08/18 18:35:30 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -1511,6 +1511,7 @@ _equalCreateSeqStmt(CreateSeqStmt *a, CreateSeqStmt *b)
 {
    COMPARE_NODE_FIELD(sequence);
    COMPARE_NODE_FIELD(options);
+   COMPARE_SCALAR_FIELD(ownerId);
 
    return true;
 }

diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 122213a134f907c3faf2ee8f35e18f3455ed5b0e..50ed8716ca863f96001ff943945989150f97e0a9 100644 (file)
--- a/src/backend/parser/gram.y
+++ b/src/backend/parser/gram.y
@@ -11,7 +11,7 @@
  *
  *
  * IDENTIFICATION
- *   $PostgreSQL: pgsql/src/backend/parser/gram.y,v 2.713 2010/06/13 17:43:12 rhaas Exp $
+ *   $PostgreSQL: pgsql/src/backend/parser/gram.y,v 2.713.2.1 2010/08/18 18:35:30 tgl Exp $
  *
  * HISTORY
  *   AUTHOR            DATE            MAJOR EVENT
@@ -2793,6 +2793,7 @@ CreateSeqStmt:
                    $4->istemp = $2;
                    n->sequence = $4;
                    n->options = $5;
+                   n->ownerId = InvalidOid;
                    $$ = (Node *)n;
                }
        ;

diff --git a/src/backend/parser/parse_utilcmd.c b/src/backend/parser/parse_utilcmd.c
index 1657096c6d83c47bfddd56cb6f8b0ecead97ced5..d1e1dac3a1e95dbb872d60c7da6476bdf63ae586 100644 (file)
--- a/src/backend/parser/parse_utilcmd.c
+++ b/src/backend/parser/parse_utilcmd.c
@@ -19,7 +19,7 @@
  * Portions Copyright (c) 1996-2010, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/backend/parser/parse_utilcmd.c,v 2.40 2010/02/26 02:00:53 momjian Exp $
+ * $PostgreSQL: pgsql/src/backend/parser/parse_utilcmd.c,v 2.40.4.1 2010/08/18 18:35:30 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -360,6 +360,18 @@ transformColumnDefinition(ParseState *pstate, CreateStmtContext *cxt,
        seqstmt->sequence = makeRangeVar(snamespace, sname, -1);
        seqstmt->options = NIL;
 
+       /*
+        * If this is ALTER ADD COLUMN, make sure the sequence will be owned
+        * by the table's owner.  The current user might be someone else
+        * (perhaps a superuser, or someone who's only a member of the owning
+        * role), but the SEQUENCE OWNED BY mechanisms will bleat unless
+        * table and sequence have exactly the same owning role.
+        */
+       if (cxt->rel)
+           seqstmt->ownerId = cxt->rel->rd_rel->relowner;
+       else
+           seqstmt->ownerId = InvalidOid;
+
        cxt->blist = lappend(cxt->blist, seqstmt);
 
        /*

diff --git a/src/backend/tcop/utility.c b/src/backend/tcop/utility.c
index 33b1aca72dc14e2fa5a4776117b053d7984d3bcd..ec36644a492ab69d5306b52294daab0599f332fe 100644 (file)
--- a/src/backend/tcop/utility.c
+++ b/src/backend/tcop/utility.c
@@ -10,7 +10,7 @@
  *
  *
  * IDENTIFICATION
- *   $PostgreSQL: pgsql/src/backend/tcop/utility.c,v 1.335 2010/02/26 02:01:04 momjian Exp $
+ *   $PostgreSQL: pgsql/src/backend/tcop/utility.c,v 1.335.4.1 2010/08/18 18:35:30 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -510,7 +510,8 @@ standard_ProcessUtility(Node *parsetree,
 
                        /* Create the table itself */
                        relOid = DefineRelation((CreateStmt *) stmt,
-                                               RELKIND_RELATION);
+                                               RELKIND_RELATION,
+                                               InvalidOid);
 
                        /*
                         * Let AlterTableCreateToastTable decide if this one

diff --git a/src/include/commands/tablecmds.h b/src/include/commands/tablecmds.h
index 45d1835548abad8937239f7b7f0aa3887b4638e5..c1774a2b379a1156ee8cbe8d333d44326014f7d3 100644 (file)
--- a/src/include/commands/tablecmds.h
+++ b/src/include/commands/tablecmds.h
@@ -7,7 +7,7 @@
  * Portions Copyright (c) 1996-2010, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/commands/tablecmds.h,v 1.46 2010/02/01 19:28:56 rhaas Exp $
+ * $PostgreSQL: pgsql/src/include/commands/tablecmds.h,v 1.46.6.1 2010/08/18 18:35:30 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -18,7 +18,7 @@
 #include "utils/relcache.h"
 
 
-extern Oid DefineRelation(CreateStmt *stmt, char relkind);
+extern Oid DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId);
 
 extern void RemoveRelations(DropStmt *drop);
 

diff --git a/src/include/nodes/parsenodes.h b/src/include/nodes/parsenodes.h
index 5325f7e924b20d0fae39d56b8c8aedbba6d025a9..54eebbc4a5ea84a779e653141e741f8276e70b91 100644 (file)
--- a/src/include/nodes/parsenodes.h
+++ b/src/include/nodes/parsenodes.h
@@ -13,7 +13,7 @@
  * Portions Copyright (c) 1996-2010, PostgreSQL Global Development Group
  * Portions Copyright (c) 1994, Regents of the University of California
  *
- * $PostgreSQL: pgsql/src/include/nodes/parsenodes.h,v 1.432 2010/02/26 02:01:25 momjian Exp $
+ * $PostgreSQL: pgsql/src/include/nodes/parsenodes.h,v 1.432.4.1 2010/08/18 18:35:30 tgl Exp $
  *
  *-------------------------------------------------------------------------
  */
@@ -1696,6 +1696,7 @@ typedef struct CreateSeqStmt
    NodeTag     type;
    RangeVar   *sequence;       /* the sequence to create */
    List       *options;
+   Oid         ownerId;        /* ID of owner, or InvalidOid for default */
 } CreateSeqStmt;
 
 typedef struct AlterSeqStmt