Skip to content

Commit b5565bc

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 99848ed commit b5565bc

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.170 2010/08/13 20:10:51 rhaas Exp $
11+
* $PostgreSQL: pgsql/src/backend/commands/sequence.c,v 1.171 2010/08/18 18:35:19 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -205,7 +205,7 @@ DefineSequence(CreateSeqStmt *seq)
205205
stmt->tablespacename = NULL;
206206
stmt->if_not_exists = false;
207207

208-
seqoid = DefineRelation(stmt, RELKIND_SEQUENCE);
208+
seqoid = DefineRelation(stmt, RELKIND_SEQUENCE, seq->ownerId);
209209
Assert(seqoid != InvalidOid);
210210

211211
rel = heap_open(seqoid, AccessExclusiveLock);

‎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.340 2010/08/13 20:10:51 rhaas Exp $
11+
* $PostgreSQL: pgsql/src/backend/commands/tablecmds.c,v 1.341 2010/08/18 18:35:19 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -341,11 +341,21 @@ static const char *storage_name(char c);
341341
* DefineRelation
342342
* Creates a new relation.
343343
*
344+
* stmt carries parsetree information from an ordinary CREATE TABLE statement.
345+
* The other arguments are used to extend the behavior for other cases:
346+
* relkind: relkind to assign to the new relation
347+
* ownerId: if not InvalidOid, use this as the new relation's owner.
348+
*
349+
* Note that permissions checks are done against current user regardless of
350+
* ownerId. A nonzero ownerId is used when someone is creating a relation
351+
* "on behalf of" someone else, so we still want to see that the current user
352+
* has permissions to do it.
353+
*
344354
* If successful, returns the OID of the new relation.
345355
* ----------------------------------------------------------------
346356
*/
347357
Oid
348-
DefineRelation(CreateStmt *stmt, char relkind)
358+
DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId)
349359
{
350360
char relname[NAMEDATALEN];
351361
Oid namespaceId;
@@ -440,6 +450,10 @@ DefineRelation(CreateStmt *stmt, char relkind)
440450
(errcode(ERRCODE_INVALID_PARAMETER_VALUE),
441451
errmsg("only shared relations can be placed in pg_global tablespace")));
442452

453+
/* Identify user ID that will own the table */
454+
if (!OidIsValid(ownerId))
455+
ownerId = GetUserId();
456+
443457
/*
444458
* Parse and validate reloptions, if any.
445459
*/
@@ -532,7 +546,7 @@ DefineRelation(CreateStmt *stmt, char relkind)
532546
InvalidOid,
533547
InvalidOid,
534548
ofTypeId,
535-
GetUserId(),
549+
ownerId,
536550
descriptor,
537551
list_concat(cookedDefaults,
538552
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.150 2010/07/28 05:22:24 sriggs Exp $
11+
* $PostgreSQL: pgsql/src/backend/commands/typecmds.c,v 1.151 2010/08/18 18:35:19 tgl Exp $
1212
*
1313
* DESCRIPTION
1414
* The "DefineFoo" routines take the parse tree and pick out the
@@ -1548,7 +1548,7 @@ DefineCompositeType(const RangeVar *typevar, List *coldeflist)
15481548
/*
15491549
* Finally create the relation. This also creates the type.
15501550
*/
1551-
relid = DefineRelation(createStmt, RELKIND_COMPOSITE_TYPE);
1551+
relid = DefineRelation(createStmt, RELKIND_COMPOSITE_TYPE, InvalidOid);
15521552
Assert(relid != InvalidOid);
15531553
return relid;
15541554
}

‎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.121 2010/07/25 23:21:21 rhaas Exp $
11+
* $PostgreSQL: pgsql/src/backend/commands/view.c,v 1.122 2010/08/18 18:35:19 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -242,7 +242,7 @@ DefineVirtualRelation(const RangeVar *relation, List *tlist, bool replace)
242242
* existing view, so we don't need more code to complain if "replace"
243243
* is false).
244244
*/
245-
relid = DefineRelation(createStmt, RELKIND_VIEW);
245+
relid = DefineRelation(createStmt, RELKIND_VIEW, InvalidOid);
246246
Assert(relid != InvalidOid);
247247
return relid;
248248
}

‎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.468 2010/08/18 15:21:54 tgl Exp $
18+
* $PostgreSQL: pgsql/src/backend/nodes/copyfuncs.c,v 1.469 2010/08/18 18:35:20 tgl Exp $
1919
*
2020
*-------------------------------------------------------------------------
2121
*/
@@ -3023,6 +3023,7 @@ _copyCreateSeqStmt(CreateSeqStmt *from)
30233023

30243024
COPY_NODE_FIELD(sequence);
30253025
COPY_NODE_FIELD(options);
3026+
COPY_SCALAR_FIELD(ownerId);
30263027

30273028
return newnode;
30283029
}

‎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.387 2010/08/07 02:44:06 tgl Exp $
25+
* $PostgreSQL: pgsql/src/backend/nodes/equalfuncs.c,v 1.388 2010/08/18 18:35:20 tgl Exp $
2626
*
2727
*-------------------------------------------------------------------------
2828
*/
@@ -1513,6 +1513,7 @@ _equalCreateSeqStmt(CreateSeqStmt *a, CreateSeqStmt *b)
15131513
{
15141514
COMPARE_NODE_FIELD(sequence);
15151515
COMPARE_NODE_FIELD(options);
1516+
COMPARE_SCALAR_FIELD(ownerId);
15161517

15171518
return true;
15181519
}

‎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.715 2010/08/05 04:21:53 petere Exp $
14+
* $PostgreSQL: pgsql/src/backend/parser/gram.y,v 2.716 2010/08/18 18:35:20 tgl Exp $
1515
*
1616
* HISTORY
1717
* AUTHOR DATE MAJOR EVENT
@@ -2827,6 +2827,7 @@ CreateSeqStmt:
28272827
$4->istemp = $2;
28282828
n->sequence = $4;
28292829
n->options = $5;
2830+
n->ownerId = InvalidOid;
28302831
$$ = (Node *)n;
28312832
}
28322833
;

‎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.42 2010/08/05 15:25:35 rhaas Exp $
22+
* $PostgreSQL: pgsql/src/backend/parser/parse_utilcmd.c,v 2.43 2010/08/18 18:35:20 tgl Exp $
2323
*
2424
*-------------------------------------------------------------------------
2525
*/
@@ -361,6 +361,18 @@ transformColumnDefinition(ParseState *pstate, CreateStmtContext *cxt,
361361
seqstmt->sequence = makeRangeVar(snamespace, sname, -1);
362362
seqstmt->options = NIL;
363363

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

366378
/*

‎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.336 2010/07/25 23:21:22 rhaas Exp $
13+
* $PostgreSQL: pgsql/src/backend/tcop/utility.c,v 1.337 2010/08/18 18:35:20 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
* If "IF NOT EXISTS" was specified and the relation

‎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.47 2010/07/28 05:22:24 sriggs Exp $
10+
* $PostgreSQL: pgsql/src/include/commands/tablecmds.h,v 1.48 2010/08/18 18:35:21 tgl Exp $
1111
*
1212
*-------------------------------------------------------------------------
1313
*/
@@ -19,7 +19,7 @@
1919
#include "utils/relcache.h"
2020

2121

22-
extern Oid DefineRelation(CreateStmt *stmt, char relkind);
22+
extern Oid DefineRelation(CreateStmt *stmt, char relkind, Oid ownerId);
2323

2424
extern void RemoveRelations(DropStmt *drop);
2525

‎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.434 2010/08/07 02:44:07 tgl Exp $
16+
* $PostgreSQL: pgsql/src/include/nodes/parsenodes.h,v 1.435 2010/08/18 18:35:21 tgl Exp $
1717
*
1818
*-------------------------------------------------------------------------
1919
*/
@@ -1700,6 +1700,7 @@ typedef struct CreateSeqStmt
17001700
NodeTag type;
17011701
RangeVar *sequence; /* the sequence to create */
17021702
List *options;
1703+
Oid ownerId; /* ID of owner, or InvalidOid for default */
17031704
} CreateSeqStmt;
17041705

17051706
typedef struct AlterSeqStmt

0 commit comments

Comments
 (0)