aboutsummaryrefslogtreecommitdiff
diff options
context:
space:
mode:
authorTom Lane <tgl@sss.pgh.pa.us>2016-11-20 14:26:19 -0500
committerTom Lane <tgl@sss.pgh.pa.us>2016-11-20 14:26:19 -0500
commit71db302ecf4876f898ef49a57f9cb92b828ad736 (patch)
tree9035fa18ab7d7919194bdcea7cd46c69de52c558
parent46b6f3fff0f11b22538d63f3711b206250bc6962 (diff)
downloadpostgresql-71db302ecf4876f898ef49a57f9cb92b828ad736.tar.gz
postgresql-71db302ecf4876f898ef49a57f9cb92b828ad736.zip
Prevent multicolumn expansion of "foo.*" in an UPDATE source expression.
Because we use transformTargetList() for UPDATE as well as SELECT tlists, the code accidentally tried to expand a "*" reference into several columns. This is nonsensical, because the UPDATE syntax provides exactly one target column to put the value into. The immediate result was that transformUpdateTargetList() got confused and reported "UPDATE target count mismatch --- internal error". It seems better to treat such a reference as a plain whole-row variable, as it would be in other contexts. (This could produce useful results when the target column is of composite type.) Fix by tweaking transformTargetList() to perform *-expansion only conditionally, depending on its exprKind parameter. Back-patch to 9.3. The problem exists further back, but a fix would be much more invasive before that, because transformTargetList() wasn't told what kind of list it was working on. Doesn't seem worth the trouble given the lack of field reports. (I only noticed it because I was checking the code while trying to improve the documentation about how we handle "foo.*".) Discussion: <4308.1479595330@sss.pgh.pa.us>
-rw-r--r--src/backend/parser/parse_target.c59
-rw-r--r--src/test/regress/expected/update.out7
-rw-r--r--src/test/regress/sql/update.sql4
3 files changed, 46 insertions, 24 deletions
diff --git a/src/backend/parser/parse_target.c b/src/backend/parser/parse_target.c
index e6374c2eca5..27295291dff 100644
--- a/src/backend/parser/parse_target.c
+++ b/src/backend/parser/parse_target.c
@@ -113,17 +113,21 @@ transformTargetEntry(ParseState *pstate,
* transformTargetList()
* Turns a list of ResTarget's into a list of TargetEntry's.
*
- * At this point, we don't care whether we are doing SELECT, UPDATE,
- * or RETURNING; we just transform the given expressions (the "val" fields).
- * However, our subroutines care, so we need the exprKind parameter.
+ * This code acts mostly the same for SELECT, UPDATE, or RETURNING lists;
+ * the main thing is to transform the given expressions (the "val" fields).
+ * The exprKind parameter distinguishes these cases when necessary.
*/
List *
transformTargetList(ParseState *pstate, List *targetlist,
ParseExprKind exprKind)
{
List *p_target = NIL;
+ bool expand_star;
ListCell *o_target;
+ /* Expand "something.*" in SELECT and RETURNING, but not UPDATE */
+ expand_star = (exprKind != EXPR_KIND_UPDATE_SOURCE);
+
foreach(o_target, targetlist)
{
ResTarget *res = (ResTarget *) lfirst(o_target);
@@ -133,35 +137,42 @@ transformTargetList(ParseState *pstate, List *targetlist,
* "something", the star could appear as the last field in ColumnRef,
* or as the last indirection item in A_Indirection.
*/
- if (IsA(res->val, ColumnRef))
+ if (expand_star)
{
- ColumnRef *cref = (ColumnRef *) res->val;
-
- if (IsA(llast(cref->fields), A_Star))
+ if (IsA(res->val, ColumnRef))
{
- /* It is something.*, expand into multiple items */
- p_target = list_concat(p_target,
- ExpandColumnRefStar(pstate, cref,
- true));
- continue;
- }
- }
- else if (IsA(res->val, A_Indirection))
- {
- A_Indirection *ind = (A_Indirection *) res->val;
+ ColumnRef *cref = (ColumnRef *) res->val;
- if (IsA(llast(ind->indirection), A_Star))
+ if (IsA(llast(cref->fields), A_Star))
+ {
+ /* It is something.*, expand into multiple items */
+ p_target = list_concat(p_target,
+ ExpandColumnRefStar(pstate,
+ cref,
+ true));
+ continue;
+ }
+ }
+ else if (IsA(res->val, A_Indirection))
{
- /* It is something.*, expand into multiple items */
- p_target = list_concat(p_target,
- ExpandIndirectionStar(pstate, ind,
- true, exprKind));
- continue;
+ A_Indirection *ind = (A_Indirection *) res->val;
+
+ if (IsA(llast(ind->indirection), A_Star))
+ {
+ /* It is something.*, expand into multiple items */
+ p_target = list_concat(p_target,
+ ExpandIndirectionStar(pstate,
+ ind,
+ true,
+ exprKind));
+ continue;
+ }
}
}
/*
- * Not "something.*", so transform as a single expression
+ * Not "something.*", or we want to treat that as a plain whole-row
+ * variable, so transform as a single expression
*/
p_target = lappend(p_target,
transformTargetEntry(pstate,
diff --git a/src/test/regress/expected/update.out b/src/test/regress/expected/update.out
index 71b856f95c6..70b7f3a0b77 100644
--- a/src/test/regress/expected/update.out
+++ b/src/test/regress/expected/update.out
@@ -52,6 +52,13 @@ SELECT * FROM update_test;
100 | 20 |
(2 rows)
+-- fail, wrong data type:
+UPDATE update_test SET a = v.* FROM (VALUES(100, 20)) AS v(i, j)
+ WHERE update_test.b = v.j;
+ERROR: column "a" is of type integer but expression is of type record
+LINE 1: UPDATE update_test SET a = v.* FROM (VALUES(100, 20)) AS v(i...
+ ^
+HINT: You will need to rewrite or cast the expression.
--
-- Test multiple-set-clause syntax
--
diff --git a/src/test/regress/sql/update.sql b/src/test/regress/sql/update.sql
index a8a028f7101..a268a3bb307 100644
--- a/src/test/regress/sql/update.sql
+++ b/src/test/regress/sql/update.sql
@@ -35,6 +35,10 @@ UPDATE update_test SET a=v.i FROM (VALUES(100, 20)) AS v(i, j)
SELECT * FROM update_test;
+-- fail, wrong data type:
+UPDATE update_test SET a = v.* FROM (VALUES(100, 20)) AS v(i, j)
+ WHERE update_test.b = v.j;
+
--
-- Test multiple-set-clause syntax
--