Skip to content

Commit 2eba69a

Browse files
tglsfdctglsfdc
committed
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.
1 parent 0321d03 commit 2eba69a

File tree

11 files changed

+49
-18
lines changed

11 files changed

+49
-18
lines changed

‎src/backend/commands/sequence.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/commands/sequence.c,v 1.168 2010/02/20 21:24:02 tgl Exp $
11+
* $PostgreSQL: pgsql/src/backend/commands/sequence.c,v 1.168.4.1 2010/08/18 18:35:29 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -204,7 +204,7 @@ DefineSequence(CreateSeqStmt *seq)
204204
stmt->oncommit = ONCOMMIT_NOOP;
205205
stmt->tablespacename = NULL;
206206

207-
seqoid = DefineRelation(stmt, RELKIND_SEQUENCE);
207+
seqoid = DefineRelation(stmt, RELKIND_SEQUENCE, seq->ownerId);
208208

209209
rel = heap_open(seqoid, AccessExclusiveLock);
210210
tupDesc = RelationGetDescr(rel);

‎src/backend/commands/tablecmds.c

Lines changed: 17 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/commands/tablecmds.c,v 1.332.2.3 2010/08/03 15:47:09 rhaas Exp $
11+
* $PostgreSQL: pgsql/src/backend/commands/tablecmds.c,v 1.332.2.4 2010/08/18 18:35:29 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -340,11 +340,21 @@ static const char *storage_name(char c);
340340
* DefineRelation
341341
* Creates a new relation.
342342
*
343+
* stmt carries parsetree information from an ordinary CREATE TABLE statement.
344+
* The other arguments are used to extend the behavior for other cases:
345+
* relkind: relkind to assign to the new relation
346+
* ownerId: if not InvalidOid, use this as the new relation's owner.
347+
*
348+
* Note that permissions checks are done against current user regardless of
349+
* ownerId. A nonzero ownerId is used when someone is creating a relation
350+
* "on behalf of" someone else, so we still want to see that the current user
351+
* has permissions to do it.
352+
*
343353
* If successful, returns the OID of the new relation.
344354
* ----------------------------------------------------------------
345355
*/
346356
Oid
347-
DefineRelation(CreateStmt *stmt, char relkind)
357+
DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId)
348358
{
349359
char relname[NAMEDATALEN];
350360
Oid namespaceId;
@@ -444,6 +454,10 @@ DefineRelation(CreateStmt *stmt, char relkind)
444454
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
445455
errmsg("only shared relations can be placed in pg_global tablespace")));
446456

457+
/* Identify user ID that will own the table */
458+
if (!OidIsValid(ownerId))
459+
ownerId = GetUserId();
460+
447461
/*
448462
* Parse and validate reloptions, if any.
449463
*/
@@ -536,7 +550,7 @@ DefineRelation(CreateStmt *stmt, char relkind)
536550
InvalidOid,
537551
InvalidOid,
538552
ofTypeId,
539-
GetUserId(),
553+
ownerId,
540554
descriptor,
541555
list_concat(cookedDefaults,
542556
old_constraints),

‎src/backend/commands/typecmds.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/commands/typecmds.c,v 1.148 2010/02/26 02:00:40 momjian Exp $
11+
* $PostgreSQL: pgsql/src/backend/commands/typecmds.c,v 1.148.4.1 2010/08/18 18:35:29 tgl Exp $
1212
*
1313
* DESCRIPTION
1414
* The "DefineFoo" routines take the parse tree and pick out the
@@ -1546,7 +1546,7 @@ DefineCompositeType(const RangeVar *typevar, List *coldeflist)
15461546
/*
15471547
* Finally create the relation. This also creates the type.
15481548
*/
1549-
return DefineRelation(createStmt, RELKIND_COMPOSITE_TYPE);
1549+
return DefineRelation(createStmt, RELKIND_COMPOSITE_TYPE, InvalidOid);
15501550
}
15511551

15521552
/*

‎src/backend/commands/view.c

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $PostgreSQL: pgsql/src/backend/commands/view.c,v 1.120 2010/01/02 16:57:40 momjian Exp $
11+
* $PostgreSQL: pgsql/src/backend/commands/view.c,v 1.120.6.1 2010/08/18 18:35:29 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -239,7 +239,7 @@ DefineVirtualRelation(const RangeVar *relation, List *tlist, bool replace)
239239
* existing view, so we don't need more code to complain if "replace"
240240
* is false).
241241
*/
242-
return DefineRelation(createStmt, RELKIND_VIEW);
242+
return DefineRelation(createStmt, RELKIND_VIEW, InvalidOid);
243243
}
244244
}
245245

‎src/backend/nodes/copyfuncs.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
* Portions Copyright (c) 1994, Regents of the University of California
1616
*
1717
* IDENTIFICATION
18-
* $PostgreSQL: pgsql/src/backend/nodes/copyfuncs.c,v 1.464.4.1 2010/08/18 15:22:00 tgl Exp $
18+
* $PostgreSQL: pgsql/src/backend/nodes/copyfuncs.c,v 1.464.4.2 2010/08/18 18:35:30 tgl Exp $
1919
*
2020
*-------------------------------------------------------------------------
2121
*/
@@ -3002,6 +3002,7 @@ _copyCreateSeqStmt(CreateSeqStmt *from)
30023002

30033003
COPY_NODE_FIELD(sequence);
30043004
COPY_NODE_FIELD(options);
3005+
COPY_SCALAR_FIELD(ownerId);
30053006

30063007
return newnode;
30073008
}

‎src/backend/nodes/equalfuncs.c

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,7 @@
2222
* Portions Copyright (c) 1994, Regents of the University of California
2323
*
2424
* IDENTIFICATION
25-
* $PostgreSQL: pgsql/src/backend/nodes/equalfuncs.c,v 1.385 2010/02/26 02:00:43 momjian Exp $
25+
* $PostgreSQL: pgsql/src/backend/nodes/equalfuncs.c,v 1.385.4.1 2010/08/18 18:35:30 tgl Exp $
2626
*
2727
*-------------------------------------------------------------------------
2828
*/
@@ -1511,6 +1511,7 @@ _equalCreateSeqStmt(CreateSeqStmt *a, CreateSeqStmt *b)
15111511
{
15121512
COMPARE_NODE_FIELD(sequence);
15131513
COMPARE_NODE_FIELD(options);
1514+
COMPARE_SCALAR_FIELD(ownerId);
15141515

15151516
return true;
15161517
}

‎src/backend/parser/gram.y

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -11,7 +11,7 @@
1111
*
1212
*
1313
* IDENTIFICATION
14-
* $PostgreSQL: pgsql/src/backend/parser/gram.y,v 2.713 2010/06/13 17:43:12 rhaas Exp $
14+
* $PostgreSQL: pgsql/src/backend/parser/gram.y,v 2.713.2.1 2010/08/18 18:35:30 tgl Exp $
1515
*
1616
* HISTORY
1717
* AUTHOR DATE MAJOR EVENT
@@ -2793,6 +2793,7 @@ CreateSeqStmt:
27932793
$4->istemp = $2;
27942794
n->sequence = $4;
27952795
n->options = $5;
2796+
n->ownerId = InvalidOid;
27962797
$$ = (Node *)n;
27972798
}
27982799
;

‎src/backend/parser/parse_utilcmd.c

Lines changed: 13 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -19,7 +19,7 @@
1919
* Portions Copyright (c) 1996-2010, PostgreSQL Global Development Group
2020
* Portions Copyright (c) 1994, Regents of the University of California
2121
*
22-
* $PostgreSQL: pgsql/src/backend/parser/parse_utilcmd.c,v 2.40 2010/02/26 02:00:53 momjian Exp $
22+
* $PostgreSQL: pgsql/src/backend/parser/parse_utilcmd.c,v 2.40.4.1 2010/08/18 18:35:30 tgl Exp $
2323
*
2424
*-------------------------------------------------------------------------
2525
*/
@@ -360,6 +360,18 @@ transformColumnDefinition(ParseState *pstate, CreateStmtContext *cxt,
360360
seqstmt->sequence = makeRangeVar(snamespace, sname, -1);
361361
seqstmt->options = NIL;
362362

363+
/*
364+
* If this is ALTER ADD COLUMN, make sure the sequence will be owned
365+
* by the table's owner. The current user might be someone else
366+
* (perhaps a superuser, or someone who's only a member of the owning
367+
* role), but the SEQUENCE OWNED BY mechanisms will bleat unless
368+
* table and sequence have exactly the same owning role.
369+
*/
370+
if (cxt->rel)
371+
seqstmt->ownerId = cxt->rel->rd_rel->relowner;
372+
else
373+
seqstmt->ownerId = InvalidOid;
374+
363375
cxt->blist = lappend(cxt->blist, seqstmt);
364376

365377
/*

‎src/backend/tcop/utility.c

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,7 @@
1010
*
1111
*
1212
* IDENTIFICATION
13-
* $PostgreSQL: pgsql/src/backend/tcop/utility.c,v 1.335 2010/02/26 02:01:04 momjian Exp $
13+
* $PostgreSQL: pgsql/src/backend/tcop/utility.c,v 1.335.4.1 2010/08/18 18:35:30 tgl Exp $
1414
*
1515
*-------------------------------------------------------------------------
1616
*/
@@ -510,7 +510,8 @@ standard_ProcessUtility(Node *parsetree,
510510

511511
/* Create the table itself */
512512
relOid = DefineRelation((CreateStmt *) stmt,
513-
RELKIND_RELATION);
513+
RELKIND_RELATION,
514+
InvalidOid);
514515

515516
/*
516517
* Let AlterTableCreateToastTable decide if this one

‎src/include/commands/tablecmds.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
* Portions Copyright (c) 1996-2010, PostgreSQL Global Development Group
88
* Portions Copyright (c) 1994, Regents of the University of California
99
*
10-
* $PostgreSQL: pgsql/src/include/commands/tablecmds.h,v 1.46 2010/02/01 19:28:56 rhaas Exp $
10+
* $PostgreSQL: pgsql/src/include/commands/tablecmds.h,v 1.46.6.1 2010/08/18 18:35:30 tgl Exp $
1111
*
1212
*-------------------------------------------------------------------------
1313
*/
@@ -18,7 +18,7 @@
1818
#include "utils/relcache.h"
1919

2020

21-
extern Oid DefineRelation(CreateStmt *stmt, char relkind);
21+
extern Oid DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId);
2222

2323
extern void RemoveRelations(DropStmt *drop);
2424

‎src/include/nodes/parsenodes.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -13,7 +13,7 @@
1313
* Portions Copyright (c) 1996-2010, PostgreSQL Global Development Group
1414
* Portions Copyright (c) 1994, Regents of the University of California
1515
*
16-
* $PostgreSQL: pgsql/src/include/nodes/parsenodes.h,v 1.432 2010/02/26 02:01:25 momjian Exp $
16+
* $PostgreSQL: pgsql/src/include/nodes/parsenodes.h,v 1.432.4.1 2010/08/18 18:35:30 tgl Exp $
1717
*
1818
*-------------------------------------------------------------------------
1919
*/
@@ -1696,6 +1696,7 @@ typedef struct CreateSeqStmt
16961696
NodeTag type;
16971697
RangeVar *sequence; /* the sequence to create */
16981698
List *options;
1699+
Oid ownerId; /* ID of owner, or InvalidOid for default */
16991700
} CreateSeqStmt;
17001701

17011702
typedef struct AlterSeqStmt

0 commit comments

Comments
 (0)