10000 Rewriter and planner should use only resno, not resname, to identify · codeimmortal/postgres@302f1a8 · GitHub
[go: up one dir, main page]
Skip to content

Commit 302f1a8

Browse files
tglsfdctglsfdc
committed
Rewriter and planner should use only resno, not resname, to identify
target columns in INSERT and UPDATE targetlists. Don't rely on resname to be accurate in ruleutils, either. This fixes bug reported by Donald Fraser, in which renaming a column referenced in a rule did not work very well.
1 parent 730b3a1 commit 302f1a8

File tree

15 files changed

+145
-116
lines changed

15 files changed

+145
-116
lines changed

‎src/backend/access/common/tupdesc.c

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $Header: /cvsroot/pgsql/src/backend/access/common/tupdesc.c,v 1.98 2003/08/04 02:39:56 momjian Exp $
11+
* $Header: /cvsroot/pgsql/src/backend/access/common/tupdesc.c,v 1.99 2003/08/11 23:04:49 tgl Exp $
1212
*
1313
* NOTES
1414
* some of the executor utility code such as "ExecTypeFromTL" should be
@@ -357,7 +357,7 @@ equalTupleDescs(TupleDesc tupdesc1, TupleDesc tupdesc2)
357357
void
358358
TupleDescInitEntry(TupleDesc desc,
359359
AttrNumber attributeNumber,
360-
char *attributeName,
360+
const char *attributeName,
361361
Oid oidtypeid,
362362
int32 typmod,
363363
int attdim,
@@ -373,13 +373,6 @@ TupleDescInitEntry(TupleDesc desc,
373373
AssertArg(PointerIsValid(desc));
374374
AssertArg(attributeNumber >= 1);
375375
AssertArg(attributeNumber <= desc->natts);
376-
377-
/*
378-
* attributeName's are sometimes NULL, from resdom's. I don't know
379-
* why that is, though -- Jolly
380-
*/
381-
/* AssertArg(NameIsValid(attributeName));*/
382-
383376
AssertArg(!PointerIsValid(desc->attrs[attributeNumber - 1]));
384377

385378
/*
@@ -394,6 +387,11 @@ TupleDescInitEntry(TupleDesc desc,
394387
*/
395388
att->attrelid = 0; /* dummy value */
396389

390+
/*
391+
* Note: attributeName can be NULL, because the planner doesn't always
392+
* fill in valid resname values in targetlists, particularly for resjunk
393+
* attributes.
394+
*/
397395
if (attributeName != NULL)
398396
namestrcpy(&(att->attname), attributeName);
399397
else

‎src/backend/catalog/dependency.c

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
* Portions Copyright (c) 1994, Regents of the University of California
99
*
1010
* IDENTIFICATION
11-
* $Header: /cvsroot/pgsql/src/backend/catalog/dependency.c,v 1.30 2003/08/04 02:39:58 momjian Exp $
11+
* $Header: /cvsroot/pgsql/src/backend/catalog/dependency.c,v 1.31 2003/08/11 23:04:49 tgl Exp $
1212
*
1313
*-------------- F438 -----------------------------------------------------------
1414
*/
@@ -1439,8 +1439,8 @@ getObjectDescription(const ObjectAddress *object)
14391439
getRelationDescription(&buffer, object->objectId);
14401440
if (object->objectSubId != 0)
14411441
appendStringInfo(&buffer, " column %s",
1442-
get_attname(object->objectId,
1443-
object->objectSubId));
1442+
get_relid_attribute_name(object->objectId,
1443+
object->objectSubId));
14441444
break;
14451445

14461446
case OCLASS_PROC:

‎src/backend/nodes/print.c

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $Header: /cvsroot/pgsql/src/backend/nodes/print.c,v 1.63 2003/08/04 02:39:59 momjian Exp $
11+
* $Header: /cvsroot/pgsql/src/backend/nodes/print.c,v 1.64 2003/08/11 23:04:49 tgl Exp $
1212
*
1313
* HISTORY
1414
* AUTHOR DATE MAJOR EVENT
@@ -448,7 +448,8 @@ print_tl(List *tlist, List *rtable)
448448
{
449449
TargetEntry *tle = lfirst(tl);
450450

451-
printf("\t%d %s\t", tle->resdom->resno, tle->resdom->resname);
451+
printf("\t%d %s\t", tle->resdom->resno,
452+
tle->resdom->resname ? tle->resdom->resname : "<null>");
452453
if (tle->resdom->ressortgroupref != 0)
453454
printf("(%u):\t", tle->resdom->ressortgroupref);
454455
else

‎src/backend/optimizer/prep/preptlist.c

Lines changed: 1 addition & 3 deletions
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-
* $Header: /cvsroot/pgsql/src/backend/optimizer/prep/preptlist.c,v 1.64 2003/08/04 02:40:01 momjian Exp $
18+
* $Header: /cvsroot/pgsql/src/backend/optimizer/prep/preptlist.c,v 1.65 2003/08/11 23:04:49 tgl Exp $
1919
*
2020
*-------------------------------------------------------------------------
2121
*/
@@ -150,8 +150,6 @@ expand_targetlist(List *tlist, int command_type,
150150

151151
if (!resdom->resjunk && resdom->resno == attrno)
152152
{
153-
Assert(strcmp(resdom->resname,
154-
NameStr(att_tup->attname)) == 0);
155153
new_tle = old_tle;
156154
tlist = lnext(tlist);
157155
}

‎src/backend/optimizer/prep/prepunion.c

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,7 @@
1414
*
1515
*
1616
* IDENTIFICATION
17-
* $Header: /cvsroot/pgsql/src/backend/optimizer/prep/prepunion.c,v 1.103 2003/08/04 02:40:01 momjian Exp $
17+
* $Header: /cvsroot/pgsql/src/backend/optimizer/prep/prepunion.c,v 1.104 2003/08/11 23:04:49 tgl Exp $
1818
*
1919
*-------------------------------------------------------------------------
2020
*/
@@ -64,7 +64,7 @@ static bool tlist_same_datatypes(List *tlist, List *colTypes, bool junkOK);
6464
static Node *adjust_inherited_attrs_mutator(Node *node,
6565
adjust_inherited_attrs_context *context);
6666
static Relids adjust_relid_set(Relids relids, Index oldrelid, Index newrelid);
67-
static List *adjust_inherited_tlist(List *tlist, Oid new_relid);
67+
static List *adjust_inherited_tlist(List *tlist, Oid old_relid, Oid new_relid);
6868

6969

7070
/*
@@ -787,6 +787,7 @@ adjust_inherited_attrs(Node *node,
787787
if (newnode->commandType == CMD_UPDATE)
788788
newnode->targetList =
789789
adjust_inherited_tlist(newnode->targetList,
790+
old_relid,
790791
new_relid);
791792
}
792793
return (Node *) newnode;
@@ -812,9 +813,10 @@ adjust_inherited_attrs_mutator(Node *node,
812813
var->varnoold = context->new_rt_index;
813814
if (var->varattno > 0)
814815
{
815-
char *attname = get_attname(context->old_relid,
816-
var->varattno);
816+
char *attname;
817817

818+
attname = get_relid_attribute_name(context->old_relid,
819+
var->varattno);
818820
var->varattno = get_attnum(context->new_relid, attname);
819821
if (var->varattno == InvalidAttrNumber)
820822
elog(ERROR, "attribute \"%s\" of relation \"%s\" does not exist",
@@ -976,7 +978,7 @@ adjust_relid_set(Relids relids, Index oldrelid, Index newrelid)
976978
* Note that this is not needed for INSERT because INSERT isn't inheritable.
977979
*/
978980
static List *
979-
adjust_inherited_tlist(List *tlist, Oid new_relid)
981+
adjust_inherited_tlist(List *tlist, Oid old_relid, Oid new_relid)
980982
{
981983
bool changed_it = false;
982984
List *tl;
@@ -989,21 +991,26 @@ adjust_inherited_tlist(List *tlist, Oid new_relid)
989991
{
990992
TargetEntry *tle = (TargetEntry *) lfirst(tl);
991993
Resdom *resdom = tle->resdom;
994+
char *attname;
992995

993996
if (resdom->resjunk)
994997
continue; /* ignore junk items */
995998

996-
attrno = get_attnum(new_relid, resdom->resname);
999+
attname = get_relid_attribute_name(old_relid, resdom->resno);
1000+
attrno = get_attnum(new_relid, attname);
9971001
if (attrno == InvalidAttrNumber)
9981002
elog(ERROR, "attribute \"%s\" of relation \"%s\" does not exist",
999-
resdom->resname, get_rel_name(new_relid));
1003+
attname, get_rel_name(new_relid));
10001004
if (resdom->resno != attrno)
10011005
{
10021006
resdom = (Resdom *) copyObject((Node *) resdom);
10031007
resdom->resno = attrno;
1008+
resdom->resname = attname;
10041009
tle->resdom = resdom;
10051010
changed_it = true;
10061011
}
1012+
else
1013+
pfree(attname);
10071014
}
10081015

10091016
/*

‎src/backend/parser/analyze.c

Lines changed: 8 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -6,7 +6,7 @@
66
* Portions Copyright (c) 1996-2003, PostgreSQL Global Development Group
77
* Portions Copyright (c) 1994, Regents of the University of California
88
*
9-
* $Header: /cvsroot/pgsql/src/backend/parser/analyze.c,v 1.286 2003/08/08 21:41:55 momjian Exp $
9+
* $Header: /cvsroot/pgsql/src/backend/parser/analyze.c,v 1.287 2003/08/11 23:04:49 tgl Exp $
1010
*
1111
*-------------------------------------------------------------------------
1212
*/
@@ -2124,10 +2124,12 @@ transformSetOperationStmt(ParseState *pstate, SelectStmt *stmt)
21242124
{
21252125
Oid colType = lfirsto(dtlist);
21262126
Resdom *leftResdom = ((TargetEntry *) lfirst(lefttl))->resdom;
2127-
char *colName = pstrdup(leftResdom->resname);
2127+
char *colName;
21282128
Resdom *resdom;
21292129
Expr *expr;
21302130

2131+
Assert(!leftResdom->resjunk);
2132+
colName = pstrdup(leftResdom->resname);
21312133
resdom = makeResdom((AttrNumber) pstate->p_next_resno++,
21322134
colType,
21332135
-1,
@@ -2501,11 +2503,12 @@ transformUpdateStmt(ParseState *pstate, UpdateStmt *stmt)
25012503
{
25022504
/*
25032505
* Resjunk nodes need no additional processing, but be sure
2504-
* they have names and resnos that do not match any target
2505-
* columns; else rewriter or planner might get confused.
2506+
* they have resnos that do not match any target columns;
2507+
* else rewriter or planner might get confused. They don't
2508+
* need a resname either.
25062509
*/
2507-
resnode->resname = "?resjunk?";
25082510
resnode->resno = (AttrNumber) pstate->p_next_resno++;
2511+
resnode->resname = NULL;
25092512
continue;
25102513
}
25112514
if (origTargetList == NIL)

‎src/backend/parser/parse_relation.c

Lines changed: 2 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $Header: /cvsroot/pgsql/src/backend/parser/parse_relation.c,v 1.88 2003/08/11 20:46:46 tgl Exp $
11+
* $Header: /cvsroot/pgsql/src/backend/parser/parse_relation.c,v 1.89 2003/08/11 23:04:49 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -1516,8 +1516,6 @@ expandRelAttrs(ParseState *pstate, RangeTblEntry *rte)
15161516
char *
15171517
get_rte_attribute_name(RangeTblEntry *rte, AttrNumber attnum)
15181518
{
1519-
char *attname;
1520-
15211519
if (attnum == InvalidAttrNumber)
15221520
return "*";
15231521

@@ -1535,13 +1533,7 @@ get_rte_attribute_name(RangeTblEntry *rte, AttrNumber attnum)
15351533
* built (which can easily happen for rules).
15361534
*/
15371535
if (rte->rtekind == RTE_RELATION)
1538-
{
1539-
attname = get_attname(rte->relid, attnum);
1540-
if (attname == NULL)
1541-
elog(ERROR, "cache lookup failed for attribute %d of relation %u",
1542-
attnum, rte->relid);
1543-
return attname;
1544-
}
1536+
return get_relid_attribute_name(rte->relid, attnum);
15451537

15461538
/*
15471539
* Otherwise use the column name from eref. There should always be

‎src/backend/parser/parse_target.c

Lines changed: 11 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,7 +8,7 @@
88
*
99
*
1010
* IDENTIFICATION
11-
* $Header: /cvsroot/pgsql/src/backend/parser/parse_target.c,v 1.111 2003/08/11 20:46:46 tgl Exp $
11+
* $Header: /cvsroot/pgsql/src/backend/parser/parse_target.c,v 1.112 2003/08/11 23:04:49 tgl Exp $
1212
*
1313
*-------------------------------------------------------------------------
1414
*/
@@ -71,7 +71,7 @@ transformTargetEntry(ParseState *pstate,
7171
type_id = exprType(expr);
7272
type_mod = exprTypmod(expr);
7373

74-
if (colname == NULL)
74+
if (colname == NULL && !resjunk)
7575
{
7676
/*
7777
* Generate a suitable column name for a column without any
@@ -428,14 +428,19 @@ updateTargetListEntry(ParseState *pstate,
428428

429429
/*
430430
* The result of the target expression should now match the
431-
* destination column's type. Also, reset the resname and resno to
432-
* identify the destination column --- rewriter and planner depend on
433-
* that!
431+
* destination column's type.
434432
*/
435433
resnode->restype = attrtype;
436434
resnode->restypmod = attrtypmod;
437-
resnode->resname = colname;
435+
/*
436+
* Set the resno to identify the target column --- the rewriter and
437+
* planner depend on this. We also set the resname to identify the
438+
* target column, but this is only for debugging purposes; it should
439+
* not be relied on. (In particular, it might be out of date in a
440+
* stored rule.)
441+
*/
438442
resnode->resno = (AttrNumber) attrno;
443+
resnode->resname = colname;
439444
}
440445

441446

‎src/backend/rewrite/rewriteHandler.c

Lines changed: 11 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -7,7 +7,7 @@
77
* Portions Copyright (c) 1994, Regents of the University of California
88
*
99
* IDENTIFICATION
10-
* $Header: /cvsroot/pgsql/src/backend/rewrite/rewriteHandler.c,v 1.128 2003/08/08 21:41:56 momjian Exp $
10+
* $Header: /cvsroot/pgsql/src/backend/rewrite/rewriteHandler.c,v 1.129 2003/08/11 23:04:49 tgl Exp $
1111
*
1212
*-------------------------------------------------------------------------
1313
*/
@@ -48,7 +48,8 @@ static Query *rewriteRuleAction(Query *parsetree,
4848
static List *adjustJoinTreeList(Query *parsetree, bool removert, int rt_index);
4949
static void rewriteTargetList(Query *parsetree, Relation target_relation);
5050
static TargetEntry *process_matched_tle(TargetEntry *src_tle,
51-
TargetEntry *prior_tle);
51+
TargetEntry *prior_tle,
52+
const char *attrName);
5253
static void markQueryForUpdate(Query *qry, bool skipOldNew);
5354
static List *matchLocks(CmdType event, RuleLock *rulelocks,
5455
int varno, Query *parsetree);
@@ -312,8 +313,7 @@ rewriteTargetList(Query *parsetree, Relation target_relation)
312313
continue;
313314

314315
/*
315-
* Look for targetlist entries matching this attr. We match by
316-
* resno, but the resname should match too.
316+
* Look for targetlist entries matching this attr.
317317
*
318318
* Junk attributes are not candidates to be matched.
319319
*/
@@ -324,9 +324,8 @@ rewriteTargetList(Query *parsetree, Relation target_relation)
324324

325325
if (!resdom->resjunk && resdom->resno == attrno)
326326
{
327-
Assert(strcmp(resdom->resname,
328-
NameStr(att_tup->attname)) == 0);
329-
new_tle = process_matched_tle(old_tle, new_tle);
327+
new_tle = process_matched_tle(old_tle, new_tle,
328+
NameStr(att_tup->attname));
330329
/* keep scanning to detect multiple assignments to attr */
331330
}
332331
}
@@ -424,11 +423,12 @@ rewriteTargetList(Query *parsetree, Relation target_relation)
424423
* Convert a matched TLE from the original tlist into a correct new TLE.
425424
*
426425
* This routine detects and handles multiple assignments to the same target
427-
* attribute.
426+
* attribute. (The attribute name is needed only for error messages.)
428427
*/
429428
static TargetEntry *
430429
process_matched_tle(TargetEntry *src_tle,
431-
TargetEntry *prior_tle)
430+
TargetEntry *prior_tle,
431+
const char *attrName)
432432
{
433433
Resdom *resdom = src_tle->resdom;
434434
Node *priorbottom;
@@ -456,7 +456,7 @@ process_matched_tle(TargetEntry *src_tle,
456456
ereport(ERROR,
457457
(errcode(ERRCODE_SYNTAX_ERROR),
458458
errmsg("multiple assignments to same attribute \"%s\"",
459-
resdom->resname)));
459+
attrName)));
460460

461461
/*
462462
* Prior TLE could be a nest of ArrayRefs if we do this more than
@@ -470,7 +470,7 @@ process_matched_tle(TargetEntry *src_tle,
470470
ereport(ERROR,
471471
(errcode(ERRCODE_SYNTAX_ERROR),
472472
errmsg("multiple assignments to same attribute \"%s\"",
473-
resdom->resname)));
473+
attrName)));
474474

475475
/*
476476
* Looks OK to nest 'em.

0 commit comments

Comments
 (0)
0