From c5f365f3ab21a345b7a109bc0411895536e9fa69 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Sun, 20 Nov 2016 14:26:19 -0500 Subject: [PATCH] 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> --- src/backend/parser/parse_target.c | 53 +++++++++++++++++----------- src/test/regress/expected/update.out | 18 ++++++++++ src/test/regress/sql/update.sql | 9 +++++ 3 files changed, 59 insertions(+), 21 deletions(-) diff --git a/src/backend/parser/parse_target.c b/src/backend/parser/parse_target.c index b7b82bfb6b..a76c33f40e 100644 --- a/src/backend/parser/parse_target.c +++ b/src/backend/parser/parse_target.c @@ -122,11 +122,15 @@ transformTargetList(ParseState *pstate, List *targetlist, ParseExprKind exprKind) { List *p_target = NIL; + bool expand_star; ListCell *o_target; /* Shouldn't have any leftover multiassign items at start */ Assert(pstate->p_multiassign_exprs == NIL); + /* 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); @@ -136,35 +140,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 adc1fd7c39..49730ea3c5 100644 --- a/src/test/regress/expected/update.out +++ b/src/test/regress/expected/update.out @@ -56,6 +56,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 -- @@ -133,6 +140,17 @@ SELECT * FROM update_test; | | (4 rows) +-- these should work, but don't yet: +UPDATE update_test SET (a,b) = (v.*) FROM (VALUES(21, 100)) AS v(i, j) + WHERE update_test.a = v.i; +ERROR: number of columns does not match number of values +LINE 1: UPDATE update_test SET (a,b) = (v.*) FROM (VALUES(21, 100)) ... + ^ +UPDATE update_test SET (a,b) = ROW(v.*) FROM (VALUES(21, 101)) AS v(i, j) + WHERE update_test.a = v.i; +ERROR: syntax error at or near "ROW" +LINE 1: UPDATE update_test SET (a,b) = ROW(v.*) FROM (VALUES(21, 101... + ^ -- if an alias for the target table is specified, don't allow references -- to the original table name UPDATE update_test AS t SET b = update_test.b + 10 WHERE t.a = 10; diff --git a/src/test/regress/sql/update.sql b/src/test/regress/sql/update.sql index 5637c68acf..e0cf5d12a9 100644 --- a/src/test/regress/sql/update.sql +++ b/src/test/regress/sql/update.sql @@ -40,6 +40,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 -- @@ -70,6 +74,11 @@ UPDATE update_test SET (b,a) = (select a+1,b from update_test); UPDATE update_test SET (b,a) = (select a+1,b from update_test where a = 1000) WHERE a = 11; SELECT * FROM update_test; +-- these should work, but don't yet: +UPDATE update_test SET (a,b) = (v.*) FROM (VALUES(21, 100)) AS v(i, j) + WHERE update_test.a = v.i; +UPDATE update_test SET (a,b) = ROW(v.*) FROM (VALUES(21, 101)) AS v(i, j) + WHERE update_test.a = v.i; -- if an alias for the target table is specified, don't allow references -- to the original table name -- 2.40.0