Better handling of UPDATE multiple-assignment row expressions - Mailing list pgsql-hackers
| From | Tom Lane |
|---|---|
| Subject | Better handling of UPDATE multiple-assignment row expressions |
| Date | |
| Msg-id | 8542.1479742008@sss.pgh.pa.us Whole thread Raw |
| List | pgsql-hackers |
While fooling around trying to document the behavior of *-expansion,
I got annoyed about the fact that it doesn't work in the context of
UPDATE tab SET (c1,c2,c3) = (x,y,z) ...
The righthand side of this is a row-expression according to the SQL
standard, and "foo.*" would be expanded in any other row expression,
but we don't do that here. Another oddity is that this ought to be
an abbreviated form of
UPDATE tab SET (c1,c2,c3) = ROW(x,y,z) ...
but we don't accept that.
It seemed like fixing this might be a good lazy-Sunday project ... so
here is a patch.
The reason this case doesn't work like other row-expressions is
that gram.y bursts the construct into
UPDATE tab SET c1 = x, c2 = y, c3 = z ...
before we ever do parse analysis. gram.y has no idea what "foo.*"
might expand to, so there's no way that that approach can work if
we want to fix this; the conversion has to be postponed to parse
analysis. The reason we didn't do it like that originally is
explained in gram.y:
* 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?).
But it turns out that the idea suggested in the parenthetical comment
is pretty easy to do. The attached patch allows DEFAULT in any a_expr
and then makes transformExpr() throw an error for it if it wasn't handled
by earlier processing. That allows improved error reporting: instead of
getting "syntax error" when DEFAULT appears someplace you can't put it,
you get a more specific error. For instance, this:
regression=# UPDATE update_test SET (a,b) = (default, default+1);
ERROR: syntax error at or near "+"
LINE 1: UPDATE update_test SET (a,b) = (default, default+1);
^
changes to:
regression=# UPDATE update_test SET (a,b) = (default, default+1);
ERROR: DEFAULT is not allowed in this context
LINE 1: UPDATE update_test SET (a,b) = (default, default+1);
^
As stated in the comment I just quoted, ideally we'd allow any suitable
composite-valued a_expr on the RHS of this kind of SET clause. The
attached patch doesn't really move the goal posts on that: you can still
only use a RowExpr or a sub-SELECT. But the RowExpr is now processed in
standard fashion, and we've at least gotten that restriction out of
gram.y and pushed it down one more level.
Anyway, the user-visible effects of this are:
* You can now write ROW explicitly in this construct, which you should
be able to per SQL spec.
* Expansion of "foo.*" in the row-expression now works as expected
(see regression test changes).
* Some error conditions now give errors more specific than "syntax error",
specifically misplacement of DEFAULT and attempting to use an unsupported
kind of expression on the RHS of this kind of SET clause.
I couldn't find anything that seemed worth changing in the existing
docs for any of these points, although if we don't fix the second point,
it'd require adjustments to the new docs I proposed at
https://www.postgresql.org/message-id/16288.1479610770@sss.pgh.pa.us
Comments, better ideas? Anyone want to do a formal review of this?
regards, tom lane
diff --git a/src/backend/parser/analyze.c b/src/backend/parser/analyze.c
index 6901e08..36f8c54 100644
*** a/src/backend/parser/analyze.c
--- b/src/backend/parser/analyze.c
*************** transformInsertStmt(ParseState *pstate,
*** 644,651 ****
{
List *sublist = (List *) lfirst(lc);
! /* Do basic expression transformation (same as a ROW() expr) */
! sublist = transformExpressionList(pstate, sublist, EXPR_KIND_VALUES);
/*
* All the sublists must be the same length, *after*
--- 644,655 ----
{
List *sublist = (List *) lfirst(lc);
! /*
! * 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*
*************** transformInsertStmt(ParseState *pstate,
*** 752,761 ****
Assert(list_length(valuesLists) == 1);
Assert(selectStmt->intoClause == NULL);
! /* Do basic expression transformation (same as a ROW() expr) */
exprList = transformExpressionList(pstate,
(List *) linitial(valuesLists),
! EXPR_KIND_VALUES);
/* Prepare row for assignment to target table */
exprList = transformInsertRow(pstate, exprList,
--- 756,769 ----
Assert(list_length(valuesLists) == 1);
Assert(selectStmt->intoClause == NULL);
! /*
! * Do basic expression transformation (same as a ROW() expr, but allow
! * SetToDefault at top level)
! */
exprList = transformExpressionList(pstate,
(List *) linitial(valuesLists),
! EXPR_KIND_VALUES,
! true);
/* Prepare row for assignment to target table */
exprList = transformInsertRow(pstate, exprList,
*************** transformValuesClause(ParseState *pstate
*** 1293,1301 ****
}
/*
! * 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.
*
* Note that the intermediate representation we build is column-organized
* not row-organized. That simplifies the type and collation processing
--- 1301,1307 ----
}
/*
! * 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
*************** transformValuesClause(ParseState *pstate
*** 1305,1312 ****
{
List *sublist = (List *) lfirst(lc);
! /* Do basic expression transformation (same as a ROW() expr) */
! sublist = transformExpressionList(pstate, sublist, EXPR_KIND_VALUES);
/*
* All the sublists must be the same length, *after* transformation
--- 1311,1322 ----
{
List *sublist = (List *) lfirst(lc);
! /*
! * 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
*************** transformValuesClause(ParseState *pstate
*** 1329,1345 ****
exprLocation((Node *) sublist))));
}
! /* Check for DEFAULT and 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++;
}
--- 1339,1350 ----
exprLocation((Node *) sublist))));
}
! /* Build per-column expression lists */
i = 0;
foreach(lc2, sublist)
{
Node *col = (Node *) lfirst(lc2);
colexprs[i] = lappend(colexprs[i], col);
i++;
}
diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y
index 0ec1cd3..367bc2e 100644
*** a/src/backend/parser/gram.y
--- b/src/backend/parser/gram.y
*************** static Node *makeRecursiveViewSelect(cha
*** 365,372 ****
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
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
--- 365,372 ----
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
! 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
*************** static Node *makeRecursiveViewSelect(cha
*** 454,460 ****
%type <node> case_expr case_arg when_clause case_default
%type <list> when_clause_list
%type <ival> sub_type
- %type <node> ctext_expr
%type <value> NumericOnly
%type <list> NumericOnly_list
%type <alias> alias_clause opt_alias_clause
--- 454,459 ----
*************** static Node *makeRecursiveViewSelect(cha
*** 466,472 ****
%type <range> relation_expr
%type <range> relation_expr_opt_alias
%type <node> tablesample_clause opt_repeatable_clause
! %type <target> target_el single_set_clause set_target insert_column_item
%type <str> generic_option_name
%type <node> generic_option_arg
--- 465,471 ----
%type <range> relation_expr
%type <range> relation_expr_opt_alias
%type <node> tablesample_clause opt_repeatable_clause
! %type <target> target_el set_target insert_column_item
%type <str> generic_option_name
%type <node> generic_option_arg
*************** set_clause_list:
*** 9914,9988 ****
;
set_clause:
! single_set_clause { $$ = list_make1($1); }
! | multiple_set_clause { $$ = $1; }
! ;
!
! single_set_clause:
! set_target '=' ctext_expr
! {
! $$ = $1;
! $$->val = (Node *) $3;
! }
! ;
!
! /*
! * 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
{
! 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->colno = i;
r->ncolumns = ncolumns;
res_col->val = (Node *) r;
--- 9913,9936 ----
;
set_clause:
! set_target '=' a_expr
{
! $1->val = (Node *) $3;
! $$ = list_make1($1);
}
! | '(' set_target_list ')' '=' a_expr
{
int ncolumns = list_length($2);
int i = 1;
ListCell *col_cell;
/* 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 *) $5;
r->colno = i;
r->ncolumns = ncolumns;
res_col->val = (Node *) r;
*************** locked_rels_list:
*** 10641,10657 ****
;
values_clause:
! VALUES ctext_row
{
SelectStmt *n = makeNode(SelectStmt);
! n->valuesLists = list_make1($2);
$$ = (Node *) n;
}
! | values_clause ',' ctext_row
{
SelectStmt *n = (SelectStmt *) $1;
! n->valuesLists = lappend(n->valuesLists, $3);
$$ = (Node *) n;
}
;
--- 10589,10610 ----
;
+ /*
+ * 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 '(' expr_list ')'
{
SelectStmt *n = makeNode(SelectStmt);
! n->valuesLists = list_make1($3);
$$ = (Node *) n;
}
! | values_clause ',' '(' expr_list ')'
{
SelectStmt *n = (SelectStmt *) $1;
! n->valuesLists = lappend(n->valuesLists, $4);
$$ = (Node *) n;
}
;
*************** a_expr: c_expr { $$ = $1; }
*** 12042,12047 ****
--- 11995,12014 ----
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;
+ }
;
/*
*************** opt_asymmetric: ASYMMETRIC
*** 13297,13332 ****
| /*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; }
- ;
-
/*****************************************************************************
*
--- 13264,13269 ----
diff --git a/src/backend/parser/parse_expr.c b/src/backend/parser/parse_expr.c
index 63f7965..17d1cbf 100644
*** a/src/backend/parser/parse_expr.c
--- b/src/backend/parser/parse_expr.c
*************** static Node *transformCaseExpr(ParseStat
*** 106,112 ****
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 *transformCoalesceExpr(ParseState *pstate, CoalesceExpr *c);
static Node *transformMinMaxExpr(ParseState *pstate, MinMaxExpr *m);
static Node *transformSQLValueFunction(ParseState *pstate,
--- 106,112 ----
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, bool allowDefault);
static Node *transformCoalesceExpr(ParseState *pstate, CoalesceExpr *c);
static Node *transformMinMaxExpr(ParseState *pstate, MinMaxExpr *m);
static Node *transformSQLValueFunction(ParseState *pstate,
*************** transformExprRecurse(ParseState *pstate,
*** 299,305 ****
break;
case T_RowExpr:
! result = transformRowExpr(pstate, (RowExpr *) expr);
break;
case T_CoalesceExpr:
--- 299,305 ----
break;
case T_RowExpr:
! result = transformRowExpr(pstate, (RowExpr *) expr, false);
break;
case T_CoalesceExpr:
*************** transformExprRecurse(ParseState *pstate,
*** 348,355 ****
break;
/*
! * CaseTestExpr and SetToDefault don't require any processing;
! * they are only injected into parse trees in fully-formed state.
*
* Ordinarily we should not see a Var here, but it is convenient
* for transformJoinUsingClause() to create untransformed operator
--- 348,367 ----
break;
/*
! * 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
*************** transformExprRecurse(ParseState *pstate,
*** 358,364 ****
* references, which seems expensively pointless. So allow it.
*/
case T_CaseTestExpr:
- case T_SetToDefault:
case T_Var:
{
result = (Node *) expr;
--- 370,375 ----
*************** static Node *
*** 1486,1494 ****
transformMultiAssignRef(ParseState *pstate, MultiAssignRef *maref)
{
SubLink *sublink;
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);
--- 1497,1505 ----
transformMultiAssignRef(ParseState *pstate, MultiAssignRef *maref)
{
SubLink *sublink;
+ RowExpr *rexpr;
Query *qtree;
TargetEntry *tle;
/* We should only see this in first-stage processing of UPDATE tlists */
Assert(pstate->p_expr_kind == EXPR_KIND_UPDATE_SOURCE);
*************** transformMultiAssignRef(ParseState *psta
*** 1496,1559 ****
/* 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));
! /* 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)));
! /*
! * 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);
}
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.
*/
Assert(pstate->p_multiassign_exprs != NIL);
tle = (TargetEntry *) llast(pstate->p_multiassign_exprs);
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 appropriate 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;
}
static Node *
--- 1507,1645 ----
/* We only need to transform the source if this is the first column */
if (maref->colno == 1)
{
! /*
! * 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);
! 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)));
! /*
! * 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);
! }
! 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 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(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;
}
! if (IsA(tle->expr, RowExpr))
! {
! Node *result;
! rexpr = (RowExpr *) tle->expr;
! /* 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 *
*************** transformArrayExpr(ParseState *pstate, A
*** 2081,2087 ****
}
static Node *
! transformRowExpr(ParseState *pstate, RowExpr *r)
{
RowExpr *newr;
char fname[16];
--- 2167,2173 ----
}
static Node *
! transformRowExpr(ParseState *pstate, RowExpr *r, bool allowDefault)
{
RowExpr *newr;
char fname[16];
*************** transformRowExpr(ParseState *pstate, Row
*** 2091,2097 ****
newr = makeNode(RowExpr);
/* Transform the field expressions */
! newr->args = transformExpressionList(pstate, r->args, pstate->p_expr_kind);
/* Barring later casting, we consider the type RECORD */
newr->row_typeid = RECORDOID;
--- 2177,2184 ----
newr = makeNode(RowExpr);
/* Transform the field expressions */
! 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 a76c33f..d440dec 100644
*** a/src/backend/parser/parse_target.c
--- b/src/backend/parser/parse_target.c
*************** transformTargetEntry(ParseState *pstate,
*** 91,97 ****
{
/* Transform the node if caller didn't do it already */
if (expr == NULL)
! expr = transformExpr(pstate, node, exprKind);
if (colname == NULL && !resjunk)
{
--- 91,107 ----
{
/* Transform the node if caller didn't do it already */
if (expr == NULL)
! {
! /*
! * 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)
{
*************** transformTargetList(ParseState *pstate,
*** 210,219 ****
* 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.
*/
List *
transformExpressionList(ParseState *pstate, List *exprlist,
! ParseExprKind exprKind)
{
List *result = NIL;
ListCell *lc;
--- 220,232 ----
* 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, bool allowDefault)
{
List *result = NIL;
ListCell *lc;
*************** transformExpressionList(ParseState *psta
*** 255,264 ****
}
/*
! * Not "something.*", so transform as a single expression
*/
! result = lappend(result,
! transformExpr(pstate, e, exprKind));
}
/* Shouldn't have any multiassign items here */
--- 268,284 ----
}
/*
! * 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.)
*/
! 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 8d4ad60..f85c618 100644
*** a/src/include/parser/parse_target.h
--- b/src/include/parser/parse_target.h
***************
*** 20,26 ****
extern List *transformTargetList(ParseState *pstate, List *targetlist,
ParseExprKind exprKind);
extern List *transformExpressionList(ParseState *pstate, List *exprlist,
! ParseExprKind exprKind);
extern void markTargetListOrigins(ParseState *pstate, List *targetlist);
extern TargetEntry *transformTargetEntry(ParseState *pstate,
Node *node, Node *expr, ParseExprKind exprKind,
--- 20,26 ----
extern List *transformTargetList(ParseState *pstate, List *targetlist,
ParseExprKind exprKind);
extern List *transformExpressionList(ParseState *pstate, List *exprlist,
! 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 49730ea..609899e 100644
*** a/src/test/regress/expected/update.out
--- b/src/test/regress/expected/update.out
*************** SELECT * FROM update_test;
*** 140,156 ****
| |
(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;
--- 140,154 ----
| |
(4 rows)
! -- *-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;
! -- 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: 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;
*************** UPDATE update_test SET c = repeat('x', 1
*** 163,170 ****
SELECT a, b, char_length(c) FROM update_test;
a | b | char_length
----+-----+-------------
- 21 | 101 |
| |
41 | 12 | 10000
42 | 12 | 10000
(4 rows)
--- 161,168 ----
SELECT a, b, char_length(c) FROM update_test;
a | b | char_length
----+-----+-------------
| |
+ 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 e0cf5d1..ad58273 100644
*** a/src/test/regress/sql/update.sql
--- b/src/test/regress/sql/update.sql
*************** UPDATE update_test SET (b,a) = (select a
*** 74,83 ****
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
--- 74,84 ----
UPDATE update_test SET (b,a) = (select a+1,b from update_test where a = 1000)
WHERE a = 11;
SELECT * FROM update_test;
! -- *-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;
! -- 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
pgsql-hackers by date: