From 906bfcad7ba7cb3863fe0e2a7810be8e3cd84fbd Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Tue, 22 Nov 2016 15:19:57 -0500 Subject: [PATCH] Improve handling of "UPDATE ... SET (column_list) = row_constructor". Previously, the right-hand side of a multiple-column assignment, if it wasn't a sub-SELECT, had to be a simple parenthesized expression list, because gram.y was responsible for "bursting" the construct into independent column assignments. This had the minor defect that you couldn't write ROW (though you should be able to, since the standard says this is a row constructor), and the rather larger defect that unlike other uses of row constructors, we would not expand a "foo.*" item into multiple columns. Fix that by changing the RHS to be just "a_expr" in the grammar, leaving it to transformMultiAssignRef to separate the elements of a RowExpr; which it will do only after performing standard transformation of the RowExpr, so that "foo.*" behaves as expected. The key reason we didn't do that before was the hard-wired handling of DEFAULT tokens (SetToDefault nodes). This patch deals with that issue by allowing DEFAULT in any a_expr and having parse analysis throw an error if SetToDefault is found in an unexpected place. That's an improvement anyway since the error can be more specific than just "syntax error". The SQL standard suggests that the RHS could be any a_expr yielding a suitable row value. This patch doesn't really move the goal posts in that respect --- you're still limited to RowExpr or a sub-SELECT --- but it does fix the grammar restriction, so it provides some tangible progress towards a full implementation. And the limitation is now documented by an explicit error message rather than an unhelpful "syntax error". Discussion: <8542.1479742008@sss.pgh.pa.us> --- doc/src/sgml/ref/update.sgml | 14 +-- src/backend/parser/analyze.c | 35 +++--- src/backend/parser/gram.y | 125 +++++-------------- src/backend/parser/parse_expr.c | 179 ++++++++++++++++++++------- src/backend/parser/parse_target.c | 30 ++++- src/include/parser/parse_target.h | 2 +- src/test/regress/expected/update.out | 18 ++- src/test/regress/sql/update.sql | 7 +- 8 files changed, 229 insertions(+), 181 deletions(-) diff --git a/doc/src/sgml/ref/update.sgml b/doc/src/sgml/ref/update.sgml index c50434f85f..2de0f4aad1 100644 --- a/doc/src/sgml/ref/update.sgml +++ b/doc/src/sgml/ref/update.sgml @@ -24,7 +24,7 @@ PostgreSQL documentation [ WITH [ RECURSIVE ] with_query [, ...] ] UPDATE [ ONLY ] table_name [ * ] [ [ AS ] alias ] SET { column_name = { expression | DEFAULT } | - ( column_name [, ...] ) = ( { expression | DEFAULT } [, ...] ) | + ( column_name [, ...] ) = [ ROW ] ( { expression | DEFAULT } [, ...] ) | ( column_name [, ...] ) = ( sub-SELECT ) } [, ...] [ FROM from_list ] @@ -420,12 +420,12 @@ UPDATE films SET kind = 'Dramatic' WHERE CURRENT OF c_films; According to the standard, the source value for a parenthesized sub-list of - column names can be any row-valued expression yielding the correct number - of columns. PostgreSQL only allows the source - value to be a parenthesized list of expressions (a row constructor) or a - sub-SELECT. An individual column's updated value can be - specified as DEFAULT in the row-constructor case, but not - inside a sub-SELECT. + target column names can be any row-valued expression yielding the correct + number of columns. PostgreSQL only allows the + source value to be a row + constructor or a sub-SELECT. An individual column's + updated value can be specified as DEFAULT in the + row-constructor case, but not inside a sub-SELECT. diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c index 6901e08fd9..36f8c548eb 100644 --- a/src/backend/parser/analyze.c +++ b/src/backend/parser/analyze.c @@ -644,8 +644,12 @@ transformInsertStmt(ParseState *pstate, InsertStmt *stmt) { List *sublist = (List *) lfirst(lc); - /* Do basic expression transformation (same as a ROW() expr) */ - sublist = transformExpressionList(pstate, sublist, EXPR_KIND_VALUES); + /* + * Do basic expression transformation (same as a ROW() expr, but + * allow SetToDefault at top level) + */ + sublist = transformExpressionList(pstate, sublist, + EXPR_KIND_VALUES, true); /* * All the sublists must be the same length, *after* @@ -752,10 +756,14 @@ transformInsertStmt(ParseState *pstate, InsertStmt *stmt) Assert(list_length(valuesLists) == 1); Assert(selectStmt->intoClause == NULL); - /* Do basic expression transformation (same as a ROW() expr) */ + /* + * Do basic expression transformation (same as a ROW() expr, but allow + * SetToDefault at top level) + */ exprList = transformExpressionList(pstate, (List *) linitial(valuesLists), - EXPR_KIND_VALUES); + EXPR_KIND_VALUES, + true); /* Prepare row for assignment to target table */ exprList = transformInsertRow(pstate, exprList, @@ -1293,9 +1301,7 @@ transformValuesClause(ParseState *pstate, SelectStmt *stmt) } /* - * For each row of VALUES, transform the raw expressions. This is also a - * handy place to reject DEFAULT nodes, which the grammar allows for - * simplicity. + * For each row of VALUES, transform the raw expressions. * * Note that the intermediate representation we build is column-organized * not row-organized. That simplifies the type and collation processing @@ -1305,8 +1311,12 @@ transformValuesClause(ParseState *pstate, SelectStmt *stmt) { List *sublist = (List *) lfirst(lc); - /* Do basic expression transformation (same as a ROW() expr) */ - sublist = transformExpressionList(pstate, sublist, EXPR_KIND_VALUES); + /* + * Do basic expression transformation (same as a ROW() expr, but here + * we disallow SetToDefault) + */ + sublist = transformExpressionList(pstate, sublist, + EXPR_KIND_VALUES, false); /* * All the sublists must be the same length, *after* transformation @@ -1329,17 +1339,12 @@ transformValuesClause(ParseState *pstate, SelectStmt *stmt) exprLocation((Node *) sublist)))); } - /* Check for DEFAULT and build per-column expression lists */ + /* Build per-column expression lists */ i = 0; foreach(lc2, sublist) { Node *col = (Node *) lfirst(lc2); - if (IsA(col, SetToDefault)) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("DEFAULT can only appear in a VALUES list within INSERT"), - parser_errposition(pstate, exprLocation(col)))); colexprs[i] = lappend(colexprs[i], col); i++; } diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index 0ec1cd345b..367bc2ecff 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -365,8 +365,8 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query); qualified_name_list any_name any_name_list type_name_list any_operator expr_list attrs target_list opt_target_list insert_column_list set_target_list - set_clause_list set_clause multiple_set_clause - ctext_expr_list ctext_row def_list operator_def_list indirection opt_indirection + set_clause_list set_clause + def_list operator_def_list indirection opt_indirection reloption_list group_clause TriggerFuncArgs select_limit opt_select_limit opclass_item_list opclass_drop_list opclass_purpose opt_opfamily transaction_mode_list_or_empty @@ -454,7 +454,6 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query); %type case_expr case_arg when_clause case_default %type when_clause_list %type sub_type -%type ctext_expr %type NumericOnly %type NumericOnly_list %type alias_clause opt_alias_clause @@ -466,7 +465,7 @@ static Node *makeRecursiveViewSelect(char *relname, List *aliases, Node *query); %type relation_expr %type relation_expr_opt_alias %type tablesample_clause opt_repeatable_clause -%type target_el single_set_clause set_target insert_column_item +%type target_el set_target insert_column_item %type generic_option_name %type generic_option_arg @@ -9914,75 +9913,24 @@ set_clause_list: ; set_clause: - single_set_clause { $$ = list_make1($1); } - | multiple_set_clause { $$ = $1; } - ; - -single_set_clause: - set_target '=' ctext_expr + set_target '=' a_expr { - $$ = $1; - $$->val = (Node *) $3; + $1->val = (Node *) $3; + $$ = list_make1($1); } - ; - -/* - * Ideally, we'd accept any row-valued a_expr as RHS of a multiple_set_clause. - * However, per SQL spec the row-constructor case must allow DEFAULT as a row - * member, and it's pretty unclear how to do that (unless perhaps we allow - * DEFAULT in any a_expr and let parse analysis sort it out later?). For the - * moment, the planner/executor only support a subquery as a multiassignment - * source anyhow, so we need only accept ctext_row and subqueries here. - */ -multiple_set_clause: - '(' set_target_list ')' '=' ctext_row + | '(' set_target_list ')' '=' a_expr { - ListCell *col_cell; - ListCell *val_cell; - - /* - * Break the ctext_row apart, merge individual expressions - * into the destination ResTargets. This is semantically - * equivalent to, and much cheaper to process than, the - * general case. - */ - if (list_length($2) != list_length($5)) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("number of columns does not match number of values"), - parser_errposition(@5))); - forboth(col_cell, $2, val_cell, $5) - { - ResTarget *res_col = (ResTarget *) lfirst(col_cell); - Node *res_val = (Node *) lfirst(val_cell); - - res_col->val = res_val; - } - - $$ = $2; - } - | '(' set_target_list ')' '=' select_with_parens - { - SubLink *sl = makeNode(SubLink); int ncolumns = list_length($2); int i = 1; ListCell *col_cell; - /* First, convert bare SelectStmt into a SubLink */ - sl->subLinkType = MULTIEXPR_SUBLINK; - sl->subLinkId = 0; /* will be assigned later */ - sl->testexpr = NULL; - sl->operName = NIL; - sl->subselect = $5; - sl->location = @5; - /* Create a MultiAssignRef source for each target */ foreach(col_cell, $2) { ResTarget *res_col = (ResTarget *) lfirst(col_cell); MultiAssignRef *r = makeNode(MultiAssignRef); - r->source = (Node *) sl; + r->source = (Node *) $5; r->colno = i; r->ncolumns = ncolumns; res_col->val = (Node *) r; @@ -10641,17 +10589,22 @@ locked_rels_list: ; +/* + * We should allow ROW '(' expr_list ')' too, but that seems to require + * making VALUES a fully reserved word, which will probably break more apps + * than allowing the noise-word is worth. + */ values_clause: - VALUES ctext_row + VALUES '(' expr_list ')' { SelectStmt *n = makeNode(SelectStmt); - n->valuesLists = list_make1($2); + n->valuesLists = list_make1($3); $$ = (Node *) n; } - | values_clause ',' ctext_row + | values_clause ',' '(' expr_list ')' { SelectStmt *n = (SelectStmt *) $1; - n->valuesLists = lappend(n->valuesLists, $3); + n->valuesLists = lappend(n->valuesLists, $4); $$ = (Node *) n; } ; @@ -12042,6 +11995,20 @@ a_expr: c_expr { $$ = $1; } list_make1($1), @2), @2); } + | DEFAULT + { + /* + * The SQL spec only allows DEFAULT in "contextually typed + * expressions", but for us, it's easier to allow it in + * any a_expr and then throw error during parse analysis + * if it's in an inappropriate context. This way also + * lets us say something smarter than "syntax error". + */ + SetToDefault *n = makeNode(SetToDefault); + /* parse analysis will fill in the rest */ + n->location = @1; + $$ = (Node *)n; + } ; /* @@ -13297,36 +13264,6 @@ opt_asymmetric: ASYMMETRIC | /*EMPTY*/ ; -/* - * The SQL spec defines "contextually typed value expressions" and - * "contextually typed row value constructors", which for our purposes - * are the same as "a_expr" and "row" except that DEFAULT can appear at - * the top level. - */ - -ctext_expr: - a_expr { $$ = (Node *) $1; } - | DEFAULT - { - SetToDefault *n = makeNode(SetToDefault); - n->location = @1; - $$ = (Node *) n; - } - ; - -ctext_expr_list: - ctext_expr { $$ = list_make1($1); } - | ctext_expr_list ',' ctext_expr { $$ = lappend($1, $3); } - ; - -/* - * We should allow ROW '(' ctext_expr_list ')' too, but that seems to require - * making VALUES a fully reserved word, which will probably break more apps - * than allowing the noise-word is worth. - */ -ctext_row: '(' ctext_expr_list ')' { $$ = $2; } - ; - /***************************************************************************** * diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c index 63f7965532..17d1cbf8b3 100644 --- a/src/backend/parser/parse_expr.c +++ b/src/backend/parser/parse_expr.c @@ -106,7 +106,7 @@ static Node *transformCaseExpr(ParseState *pstate, CaseExpr *c); static Node *transformSubLink(ParseState *pstate, SubLink *sublink); static Node *transformArrayExpr(ParseState *pstate, A_ArrayExpr *a, Oid array_type, Oid element_type, int32 typmod); -static Node *transformRowExpr(ParseState *pstate, RowExpr *r); +static Node *transformRowExpr(ParseState *pstate, RowExpr *r, bool allowDefault); static Node *transformCoalesceExpr(ParseState *pstate, CoalesceExpr *c); static Node *transformMinMaxExpr(ParseState *pstate, MinMaxExpr *m); static Node *transformSQLValueFunction(ParseState *pstate, @@ -299,7 +299,7 @@ transformExprRecurse(ParseState *pstate, Node *expr) break; case T_RowExpr: - result = transformRowExpr(pstate, (RowExpr *) expr); + result = transformRowExpr(pstate, (RowExpr *) expr, false); break; case T_CoalesceExpr: @@ -348,8 +348,20 @@ transformExprRecurse(ParseState *pstate, Node *expr) break; /* - * CaseTestExpr and SetToDefault don't require any processing; - * they are only injected into parse trees in fully-formed state. + * In all places where DEFAULT is legal, the caller should have + * processed it rather than passing it to transformExpr(). + */ + case T_SetToDefault: + ereport(ERROR, + (errcode(ERRCODE_SYNTAX_ERROR), + errmsg("DEFAULT is not allowed in this context"), + parser_errposition(pstate, + ((SetToDefault *) expr)->location))); + break; + + /* + * CaseTestExpr doesn't require any processing; it is only + * injected into parse trees in a fully-formed state. * * Ordinarily we should not see a Var here, but it is convenient * for transformJoinUsingClause() to create untransformed operator @@ -358,7 +370,6 @@ transformExprRecurse(ParseState *pstate, Node *expr) * references, which seems expensively pointless. So allow it. */ case T_CaseTestExpr: - case T_SetToDefault: case T_Var: { result = (Node *) expr; @@ -1486,9 +1497,9 @@ static Node * transformMultiAssignRef(ParseState *pstate, MultiAssignRef *maref) { SubLink *sublink; + RowExpr *rexpr; Query *qtree; TargetEntry *tle; - Param *param; /* We should only see this in first-stage processing of UPDATE tlists */ Assert(pstate->p_expr_kind == EXPR_KIND_UPDATE_SOURCE); @@ -1496,64 +1507,139 @@ transformMultiAssignRef(ParseState *pstate, MultiAssignRef *maref) /* We only need to transform the source if this is the first column */ if (maref->colno == 1) { - sublink = (SubLink *) transformExprRecurse(pstate, maref->source); - /* Currently, the grammar only allows a SubLink as source */ - Assert(IsA(sublink, SubLink)); - Assert(sublink->subLinkType == MULTIEXPR_SUBLINK); - qtree = (Query *) sublink->subselect; - Assert(IsA(qtree, Query)); + /* + * For now, we only allow EXPR SubLinks and RowExprs as the source of + * an UPDATE multiassignment. This is sufficient to cover interesting + * cases; at worst, someone would have to write (SELECT * FROM expr) + * to expand a composite-returning expression of another form. + */ + if (IsA(maref->source, SubLink) && + ((SubLink *) maref->source)->subLinkType == EXPR_SUBLINK) + { + /* Relabel it as a MULTIEXPR_SUBLINK */ + sublink = (SubLink *) maref->source; + sublink->subLinkType = MULTIEXPR_SUBLINK; + /* And transform it */ + sublink = (SubLink *) transformExprRecurse(pstate, + (Node *) sublink); - /* Check subquery returns required number of columns */ - if (count_nonjunk_tlist_entries(qtree->targetList) != maref->ncolumns) - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), + qtree = (Query *) sublink->subselect; + Assert(IsA(qtree, Query)); + + /* Check subquery returns required number of columns */ + if (count_nonjunk_tlist_entries(qtree->targetList) != maref->ncolumns) + ereport(ERROR, + (errcode(ERRCODE_SYNTAX_ERROR), errmsg("number of columns does not match number of values"), - parser_errposition(pstate, sublink->location))); + parser_errposition(pstate, sublink->location))); - /* - * Build a resjunk tlist item containing the MULTIEXPR SubLink, and - * add it to pstate->p_multiassign_exprs, whence it will later get - * appended to the completed targetlist. We needn't worry about - * selecting a resno for it; transformUpdateStmt will do that. - */ - tle = makeTargetEntry((Expr *) sublink, 0, NULL, true); - pstate->p_multiassign_exprs = lappend(pstate->p_multiassign_exprs, tle); + /* + * Build a resjunk tlist item containing the MULTIEXPR SubLink, + * and add it to pstate->p_multiassign_exprs, whence it will later + * get appended to the completed targetlist. We needn't worry + * about selecting a resno for it; transformUpdateStmt will do + * that. + */ + tle = makeTargetEntry((Expr *) sublink, 0, NULL, true); + pstate->p_multiassign_exprs = lappend(pstate->p_multiassign_exprs, + tle); - /* - * Assign a unique-within-this-targetlist ID to the MULTIEXPR SubLink. - * We can just use its position in the p_multiassign_exprs list. - */ - sublink->subLinkId = list_length(pstate->p_multiassign_exprs); + /* + * Assign a unique-within-this-targetlist ID to the MULTIEXPR + * SubLink. We can just use its position in the + * p_multiassign_exprs list. + */ + sublink->subLinkId = list_length(pstate->p_multiassign_exprs); + } + else if (IsA(maref->source, RowExpr)) + { + /* Transform the RowExpr, allowing SetToDefault items */ + rexpr = (RowExpr *) transformRowExpr(pstate, + (RowExpr *) maref->source, + true); + + /* Check it returns required number of columns */ + if (list_length(rexpr->args) != maref->ncolumns) + ereport(ERROR, + (errcode(ERRCODE_SYNTAX_ERROR), + errmsg("number of columns does not match number of values"), + parser_errposition(pstate, rexpr->location))); + + /* + * Temporarily append it to p_multiassign_exprs, so we can get it + * back when we come back here for additional columns. + */ + tle = makeTargetEntry((Expr *) rexpr, 0, NULL, true); + pstate->p_multiassign_exprs = lappend(pstate->p_multiassign_exprs, + tle); + } + else + ereport(ERROR, + (errcode(ERRCODE_FEATURE_NOT_SUPPORTED), + errmsg("source for a multiple-column UPDATE item must be a sub-SELECT or ROW() expression"), + parser_errposition(pstate, exprLocation(maref->source)))); } else { /* * Second or later column in a multiassignment. Re-fetch the - * transformed query, which we assume is still the last entry in - * p_multiassign_exprs. + * transformed SubLink or RowExpr, which we assume is still the last + * entry in p_multiassign_exprs. */ Assert(pstate->p_multiassign_exprs != NIL); tle = (TargetEntry *) llast(pstate->p_multiassign_exprs); + } + + /* + * Emit the appropriate output expression for the current column + */ + if (IsA(tle->expr, SubLink)) + { + Param *param; + sublink = (SubLink *) tle->expr; - Assert(IsA(sublink, SubLink)); Assert(sublink->subLinkType == MULTIEXPR_SUBLINK); qtree = (Query *) sublink->subselect; Assert(IsA(qtree, Query)); + + /* Build a Param representing the current subquery output column */ + tle = (TargetEntry *) list_nth(qtree->targetList, maref->colno - 1); + Assert(!tle->resjunk); + + param = makeNode(Param); + param->paramkind = PARAM_MULTIEXPR; + param->paramid = (sublink->subLinkId << 16) | maref->colno; + param->paramtype = exprType((Node *) tle->expr); + param->paramtypmod = exprTypmod((Node *) tle->expr); + param->paramcollid = exprCollation((Node *) tle->expr); + param->location = exprLocation((Node *) tle->expr); + + return (Node *) param; } - /* Build a Param representing the appropriate subquery output column */ - tle = (TargetEntry *) list_nth(qtree->targetList, maref->colno - 1); - Assert(!tle->resjunk); + if (IsA(tle->expr, RowExpr)) + { + Node *result; - param = makeNode(Param); - param->paramkind = PARAM_MULTIEXPR; - param->paramid = (sublink->subLinkId << 16) | maref->colno; - param->paramtype = exprType((Node *) tle->expr); - param->paramtypmod = exprTypmod((Node *) tle->expr); - param->paramcollid = exprCollation((Node *) tle->expr); - param->location = exprLocation((Node *) tle->expr); + rexpr = (RowExpr *) tle->expr; - return (Node *) param; + /* Just extract and return the next element of the RowExpr */ + result = (Node *) list_nth(rexpr->args, maref->colno - 1); + + /* + * If we're at the last column, delete the RowExpr from + * p_multiassign_exprs; we don't need it anymore, and don't want it in + * the finished UPDATE tlist. + */ + if (maref->colno == maref->ncolumns) + pstate->p_multiassign_exprs = + list_delete_ptr(pstate->p_multiassign_exprs, tle); + + return result; + } + + elog(ERROR, "unexpected expr type in multiassign list"); + return NULL; /* keep compiler quiet */ } static Node * @@ -2081,7 +2167,7 @@ transformArrayExpr(ParseState *pstate, A_ArrayExpr *a, } static Node * -transformRowExpr(ParseState *pstate, RowExpr *r) +transformRowExpr(ParseState *pstate, RowExpr *r, bool allowDefault) { RowExpr *newr; char fname[16]; @@ -2091,7 +2177,8 @@ transformRowExpr(ParseState *pstate, RowExpr *r) newr = makeNode(RowExpr); /* Transform the field expressions */ - newr->args = transformExpressionList(pstate, r->args, pstate->p_expr_kind); + newr->args = transformExpressionList(pstate, r->args, + pstate->p_expr_kind, allowDefault); /* Barring later casting, we consider the type RECORD */ newr->row_typeid = RECORDOID; diff --git a/src/backend/parser/parse_target.c b/src/backend/parser/parse_target.c index a76c33f40e..d440dec556 100644 --- a/src/backend/parser/parse_target.c +++ b/src/backend/parser/parse_target.c @@ -91,7 +91,17 @@ transformTargetEntry(ParseState *pstate, { /* Transform the node if caller didn't do it already */ if (expr == NULL) - expr = transformExpr(pstate, node, exprKind); + { + /* + * If it's a SetToDefault node and we should allow that, pass it + * through unmodified. (transformExpr will throw the appropriate + * error if we're disallowing it.) + */ + if (exprKind == EXPR_KIND_UPDATE_SOURCE && IsA(node, SetToDefault)) + expr = node; + else + expr = transformExpr(pstate, node, exprKind); + } if (colname == NULL && !resjunk) { @@ -210,10 +220,13 @@ transformTargetList(ParseState *pstate, List *targetlist, * the input list elements are bare expressions without ResTarget decoration, * and the output elements are likewise just expressions without TargetEntry * decoration. We use this for ROW() and VALUES() constructs. + * + * exprKind is not enough to tell us whether to allow SetToDefault, so + * an additional flag is needed for that. */ List * transformExpressionList(ParseState *pstate, List *exprlist, - ParseExprKind exprKind) + ParseExprKind exprKind, bool allowDefault) { List *result = NIL; ListCell *lc; @@ -255,10 +268,17 @@ transformExpressionList(ParseState *pstate, List *exprlist, } /* - * Not "something.*", so transform as a single expression + * Not "something.*", so transform as a single expression. If it's a + * SetToDefault node and we should allow that, pass it through + * unmodified. (transformExpr will throw the appropriate error if + * we're disallowing it.) */ - result = lappend(result, - transformExpr(pstate, e, exprKind)); + if (allowDefault && IsA(e, SetToDefault)) + /* do nothing */ ; + else + e = transformExpr(pstate, e, exprKind); + + result = lappend(result, e); } /* Shouldn't have any multiassign items here */ diff --git a/src/include/parser/parse_target.h b/src/include/parser/parse_target.h index 8d4ad60026..f85c618c43 100644 --- a/src/include/parser/parse_target.h +++ b/src/include/parser/parse_target.h @@ -20,7 +20,7 @@ extern List *transformTargetList(ParseState *pstate, List *targetlist, ParseExprKind exprKind); extern List *transformExpressionList(ParseState *pstate, List *exprlist, - ParseExprKind exprKind); + ParseExprKind exprKind, bool allowDefault); extern void markTargetListOrigins(ParseState *pstate, List *targetlist); extern TargetEntry *transformTargetEntry(ParseState *pstate, Node *node, Node *expr, ParseExprKind exprKind, diff --git a/src/test/regress/expected/update.out b/src/test/regress/expected/update.out index 49730ea3c5..609899e1f7 100644 --- a/src/test/regress/expected/update.out +++ b/src/test/regress/expected/update.out @@ -140,17 +140,15 @@ 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) +-- *-expansion should work in this context: +UPDATE update_test SET (a,b) = ROW(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) +-- you might expect this to work, but syntactically it's not a RowExpr: +UPDATE update_test SET (a,b) = (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... - ^ +ERROR: source for a multiple-column UPDATE item must be a sub-SELECT or ROW() expression +LINE 1: UPDATE update_test SET (a,b) = (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; @@ -163,8 +161,8 @@ UPDATE update_test SET c = repeat('x', 10000) WHERE c = 'car'; SELECT a, b, char_length(c) FROM update_test; a | b | char_length ----+-----+------------- - 21 | 101 | | | + 21 | 100 | 41 | 12 | 10000 42 | 12 | 10000 (4 rows) diff --git a/src/test/regress/sql/update.sql b/src/test/regress/sql/update.sql index e0cf5d12a9..ad58273b38 100644 --- a/src/test/regress/sql/update.sql +++ b/src/test/regress/sql/update.sql @@ -74,10 +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) +-- *-expansion should work in this context: +UPDATE update_test SET (a,b) = ROW(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) +-- you might expect this to work, but syntactically it's not a RowExpr: +UPDATE update_test SET (a,b) = (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