Skip to content

Commit 6e74a91

Browse files
tglsfdctglsfdc
committed
Fix incorrect generation of whole-row variables in planner.
A couple of places in the planner need to generate whole-row Vars, and were cutting corners by setting vartype = RECORDOID in the Vars, even in cases where there's an identifiable named composite type for the RTE being referenced. While we mostly got away with this, it failed when there was also a parser-generated whole-row reference to the same RTE, because the two Vars weren't equal() due to the difference in vartype. Fix by providing a subroutine the planner can call to generate whole-row Vars the same way the parser does. Per bug #5716 from Andrew Tipton. Back-patch to 9.0 where one of the bogus calls was introduced (the other one is new in HEAD).
1 parent 722d5be commit 6e74a91

File tree

7 files changed

+183
-88
lines changed

7 files changed

+183
-88
lines changed

‎src/backend/nodes/makefuncs.c

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -107,6 +107,93 @@ makeVarFromTargetEntry(Index varno,
107107
0);
108108
}
109109

110+
/*
111+
* makeWholeRowVar -
112+
* creates a Var node representing a whole row of the specified RTE
113+
*
114+
* A whole-row reference is a Var with varno set to the correct range
115+
* table entry, and varattno == 0 to signal that it references the whole
116+
* tuple. (Use of zero here is unclean, since it could easily be confused
117+
* with error cases, but it's not worth changing now.) The vartype indicates
118+
* a rowtype; either a named composite type, or RECORD. This function
119+
* encapsulates the logic for determining the correct rowtype OID to use.
120+
*/
121+
Var *
122+
makeWholeRowVar(RangeTblEntry *rte,
123+
Index varno,
124+
Index varlevelsup)
125+
{
126+
Var *result;
127+
Oid toid;
128+
129+
switch (rte->rtekind)
130+
{
131+
case RTE_RELATION:
132+
/* relation: the rowtype is a named composite type */
133+
toid = get_rel_type_id(rte->relid);
134+
if (!OidIsValid(toid))
135+
elog(ERROR, "could not find type OID for relation %u",
136+
rte->relid);
137+
result = makeVar(varno,
138+
InvalidAttrNumber,
139+
toid,
140+
-1,
141+
varlevelsup);
142+
break;
143+
case RTE_FUNCTION:
144+
toid = exprType(rte->funcexpr);
145+
if (type_is_rowtype(toid))
146+
{
147+
/* func returns composite; same as relation case */
148+
result = makeVar(varno,
149+
InvalidAttrNumber,
150+
toid,
151+
-1,
152+
varlevelsup);
153+
}
154+
else
155+
{
156+
/*
157+
* func returns scalar; instead of making a whole-row Var,
158+
* just reference the function's scalar output. (XXX this
159+
* seems a tad inconsistent, especially if "f.*" was
160+
* explicitly written ...)
161+
*/
162+
result = makeVar(varno,
163+
1,
164+
toid,
165+
-1,
166+
varlevelsup);
167+
}
168+
break;
169+
case RTE_VALUES:
170+
toid = RECORDOID;
171+
/* returns composite; same as relation case */
172+
result = makeVar(varno,
173+
InvalidAttrNumber,
174+
toid,
175+
-1,
176+
varlevelsup);
177+
break;
178+
default:
179+
180+
/*
181+
* RTE is a join or subselect. We represent this as a whole-row
182+
* Var of RECORD type. (Note that in most cases the Var will be
183+
* expanded to a RowExpr during planning, but that is not our
184+
* concern here.)
185+
*/
186+
result = makeVar(varno,
187+
InvalidAttrNumber,
188+
RECORDOID,
189+
-1,
190+
varlevelsup);
191+
break;
192+
}
193+
194+
return result;
195+
}
196+
110197
/*
111198
* makeTargetEntry -
112199
* creates a TargetEntry node

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

Lines changed: 3 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -139,11 +139,9 @@ preprocess_targetlist(PlannerInfo *root, List *tlist)
139139
else
140140
{
141141
/* Not a table, so we need the whole row as a junk var */
142-
var = makeVar(rc->rti,
143-
InvalidAttrNumber,
144-
RECORDOID,
145-
-1,
146-
0);
142+
var = makeWholeRowVar(rt_fetch(rc->rti, range_table),
143+
rc->rti,
144+
0);
147145
snprintf(resname, sizeof(resname), "wholerow%u", rc->rti);
148146
tle = makeTargetEntry((Expr *) var,
149147
list_length(tlist) + 1,

‎src/backend/parser/parse_expr.c

Lines changed: 2 additions & 74 deletions
Original file line numberDiff line numberDiff line change
@@ -2015,93 +2015,21 @@ transformCurrentOfExpr(ParseState *pstate, CurrentOfExpr *cexpr)
20152015

20162016
/*
20172017
* Construct a whole-row reference to represent the notation "relation.*".
2018-
*
2019-
* A whole-row reference is a Var with varno set to the correct range
2020-
* table entry, and varattno == 0 to signal that it references the whole
2021-
* tuple. (Use of zero here is unclean, since it could easily be confused
2022-
* with error cases, but it's not worth changing now.) The vartype indicates
2023-
* a rowtype; either a named composite type, or RECORD.
20242018
*/
20252019
static Node *
20262020
transformWholeRowRef(ParseState *pstate, RangeTblEntry *rte, int location)
20272021
{
20282022
Var *result;
20292023
int vnum;
20302024
int sublevels_up;
2031-
Oid toid;
20322025

20332026
/* Find the RTE's rangetable location */
2034-
20352027
vnum = RTERangeTablePosn(pstate, rte, &sublevels_up);
20362028

20372029
/* Build the appropriate referencing node */
2030+
result = makeWholeRowVar(rte, vnum, sublevels_up);
20382031

2039-
switch (rte->rtekind)
2040-
{
2041-
case RTE_RELATION:
2042-
/* relation: the rowtype is a named composite type */
2043-
toid = get_rel_type_id(rte->relid);
2044-
if (!OidIsValid(toid))
2045-
elog(ERROR, "could not find type OID for relation %u",
2046-
rte->relid);
2047-
result = makeVar(vnum,
2048-
InvalidAttrNumber,
2049-
toid,
2050-
-1,
2051-
sublevels_up);
2052-
break;
2053-
case RTE_FUNCTION:
2054-
toid = exprType(rte->funcexpr);
2055-
if (type_is_rowtype(toid))
2056-
{
2057-
/* func returns composite; same as relation case */
2058-
result = makeVar(vnum,
2059-
InvalidAttrNumber,
2060-
toid,
2061-
-1,
2062-
sublevels_up);
2063-
}
2064-
else
2065-
{
2066-
/*
2067-
* func returns scalar; instead of making a whole-row Var,
2068-
* just reference the function's scalar output. (XXX this
2069-
* seems a tad inconsistent, especially if "f.*" was
2070-
* explicitly written ...)
2071-
*/
2072-
result = makeVar(vnum,
2073-
1,
2074-
toid,
2075-
-1,
2076-
sublevels_up);
2077-
}
2078-
break;
2079-
case RTE_VALUES:
2080-
toid = RECORDOID;
2081-
/* returns composite; same as relation case */
2082-
result = makeVar(vnum,
2083-
InvalidAttrNumber,
2084-
toid,
2085-
-1,
2086-
sublevels_up);
2087-
break;
2088-
default:
2089-
2090-
/*
2091-
* RTE is a join or subselect. We represent this as a whole-row
2092-
* Var of RECORD type. (Note that in most cases the Var will be
2093-
* expanded to a RowExpr during planning, but that is not our
2094-
* concern here.)
2095-
*/
2096-
result = makeVar(vnum,
2097-
InvalidAttrNumber,
2098-
RECORDOID,
2099-
-1,
2100-
sublevels_up);
2101-
break;
2102-
}
2103-
2104-
/* location is not filled in by makeVar */
2032+
/* location is not filled in by makeWholeRowVar */
21052033
result->location = location;
21062034

21072035
/* mark relation as requiring whole-row SELECT access */

‎src/backend/rewrite/rewriteHandler.c

Lines changed: 9 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -52,7 +52,8 @@ static TargetEntry *process_matched_tle(TargetEntry *src_tle,
5252
static Node *get_assignment_input(Node *node);
5353
static void rewriteValuesRTE(RangeTblEntry *rte, Relation target_relation,
5454
List *attrnos);
55-
static void rewriteTargetListUD(Query *parsetree, Relation target_relation);
55+
static void rewriteTargetListUD(Query *parsetree, RangeTblEntry *target_rte,
56+
Relation target_relation);
5657
static void markQueryForLocking(Query *qry, Node *jtnode,
5758
bool forUpdate, bool noWait, bool pushedDown);
5859
static List *matchLocks(CmdType event, RuleLock *rulelocks,
@@ -1110,7 +1111,8 @@ rewriteValuesRTE(RangeTblEntry *rte, Relation target_relation, List *attrnos)
11101111
* ordering isn't actually critical at the moment.
11111112
*/
11121113
static void
1113-
rewriteTargetListUD(Query *parsetree, Relation target_relation)
1114+
rewriteTargetListUD(Query *parsetree, RangeTblEntry *target_rte,
1115+
Relation target_relation)
11141116
{
11151117
Var *var;
11161118
const char *attrname;
@@ -1135,11 +1137,9 @@ rewriteTargetListUD(Query *parsetree, Relation target_relation)
11351137
* Emit whole-row Var so that executor will have the "old" view row
11361138
* to pass to the INSTEAD OF trigger.
11371139
*/
1138-
var = makeVar(parsetree->resultRelation,
1139-
InvalidAttrNumber,
1140-
RECORDOID,
1141-
-1,
1142-
0);
1140+
var = makeWholeRowVar(target_rte,
1141+
parsetree->resultRelation,
1142+
0);
11431143

11441144
attrname = "wholerow";
11451145
}
@@ -1858,11 +1858,11 @@ RewriteQuery(Query *parsetree, List *rewrite_events)
18581858
else if (event == CMD_UPDATE)
18591859
{
18601860
rewriteTargetListIU(parsetree, rt_entry_relation, NULL);
1861-
rewriteTargetListUD(parsetree, rt_entry_relation);
1861+
rewriteTargetListUD(parsetree, rt_entry, rt_entry_relation);
18621862
}
18631863
else if (event == CMD_DELETE)
18641864
{
1865-
rewriteTargetListUD(parsetree, rt_entry_relation);
1865+
rewriteTargetListUD(parsetree, rt_entry, rt_entry_relation);
18661866
}
18671867
else
18681868
elog(ERROR, "unrecognized commandType: %d", (int) event);

‎src/include/nodes/makefuncs.h

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,10 @@ extern Var *makeVar(Index varno,
3232
extern Var *makeVarFromTargetEntry(Index varno,
3333
TargetEntry *tle);
3434

35+
extern Var *makeWholeRowVar(RangeTblEntry *rte,
36+
Index varno,
37+
Index varlevelsup);
38+
3539
extern TargetEntry *makeTargetEntry(Expr *expr,
3640
AttrNumber resno,
3741
char *resname,

‎src/test/regress/expected/rowtypes.out

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -286,3 +286,41 @@ select row(1,1.1) = any (array[ row(7,7.7), row(1,1.0), row(0,0.0) ]);
286286
f
287287
(1 row)
288288

289+
--
290+
-- Test case derived from bug #5716: check multiple uses of a rowtype result
291+
--
292+
BEGIN;
293+
CREATE TABLE price (
294+
id SERIAL PRIMARY KEY,
295+
active BOOLEAN NOT NULL,
296+
price NUMERIC
297+
);
298+
NOTICE: CREATE TABLE will create implicit sequence "price_id_seq" for serial column "price.id"
299+
NOTICE: CREATE TABLE / PRIMARY KEY will create implicit index "price_pkey" for table "price"
300+
CREATE TYPE price_input AS (
301+
id INTEGER,
302+
price NUMERIC
303+
);
304+
CREATE TYPE price_key AS (
305+
id INTEGER
306+
);
307+
CREATE FUNCTION price_key_from_table(price) RETURNS price_key AS $$
308+
SELECT $1.id
309+
$$ LANGUAGE SQL;
310+
CREATE FUNCTION price_key_from_input(price_input) RETURNS price_key AS $$
311+
SELECT $1.id
312+
$$ LANGUAGE SQL;
313+
insert into price values (1,false,42), (10,false,100), (11,true,17.99);
314+
UPDATE price
315+
SET active = true, price = input_prices.price
316+
FROM unnest(ARRAY[(10, 123.00), (11, 99.99)]::price_input[]) input_prices
317+
WHERE price_key_from_table(price.*) = price_key_from_input(input_prices.*);
318+
select * from price;
319+
id | active | price
320+
----+--------+--------
321+
1 | f | 42
322+
10 | t | 123.00
323+
11 | t | 99.99
324+
(3 rows)
325+
326+
rollback;

‎src/test/regress/sql/rowtypes.sql

Lines changed: 40 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -117,3 +117,43 @@ select array[ row(1,2), row(3,4), row(5,6) ];
117117
-- Check ability to compare an anonymous row to elements of an array
118118
select row(1,1.1) = any (array[ row(7,7.7), row(1,1.1), row(0,0.0) ]);
119119
select row(1,1.1) = any (array[ row(7,7.7), row(1,1.0), row(0,0.0) ]);
120+
121+
--
122+
-- Test case derived from bug #5716: check multiple uses of a rowtype result
123+
--
124+
125+
BEGIN;
126+
127+
CREATE TABLE price (
128+
id SERIAL PRIMARY KEY,
129+
active BOOLEAN NOT NULL,
130+
price NUMERIC
131+
);
132+
133+
CREATE TYPE price_input AS (
134+
id INTEGER,
135+
price NUMERIC
136+
);
137+
138+
CREATE TYPE price_key AS (
139+
id INTEGER
140+
);
141+
142+
CREATE FUNCTION price_key_from_table(price) RETURNS price_key AS $$
143+
SELECT $1.id
144+
$$ LANGUAGE SQL;
145+
146+
CREATE FUNCTION price_key_from_input(price_input) RETURNS price_key AS $$
147+
SELECT $1.id
148+
$$ LANGUAGE SQL;
149+
150+
insert into price values (1,false,42), (10,false,100), (11,true,17.99);
151+
152+
UPDATE price
153+
SET active = true, price = input_prices.price
154+
FROM unnest(ARRAY[(10, 123.00), (11, 99.99)]::price_input[]) input_prices
155+
WHERE price_key_from_table(price.*) = price_key_from_input(input_prices.*);
156+
157+
select * from price;
158+
159+
rollback;

0 commit comments

Comments
 (0)