From 091fe037757abbecd6994daea0ae4eaa87f7217e Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Sun, 3 Sep 2006 22:37:06 +0000 Subject: [PATCH] Code review for UPDATE SET (columnlist) patch. Make it handle as much of the syntax as this fundamentally dead-end approach can, in particular combinations of single and multi column assignments. Improve rather inadequate documentation and provide some regression tests. --- doc/src/sgml/ref/update.sgml | 31 ++++-- src/backend/parser/gram.y | 200 +++++++++++++++++------------------ src/test/regress/expected/update.out | 79 +++++++++----- src/test/regress/sql/update.sql | 26 ++++- 4 files changed, 195 insertions(+), 141 deletions(-) diff --git a/doc/src/sgml/ref/update.sgml b/doc/src/sgml/ref/update.sgml index 7b5d17a1fc..ec2200f3bc 100644 --- a/doc/src/sgml/ref/update.sgml +++ b/doc/src/sgml/ref/update.sgml @@ -1,5 +1,5 @@ @@ -21,8 +21,8 @@ PostgreSQL documentation UPDATE [ ONLY ] table [ [ AS ] alias ] - [ SET column = { expression | DEFAULT } [, ...] | - SET ( column [, ...] ) = ( { expression | DEFAULT } [, ...] ) [, ...] ] + SET { column = { expression | DEFAULT } | + ( column [, ...] ) = ( { expression | DEFAULT } [, ...] ) } [, ...] [ FROM fromlist ] [ WHERE condition ] [ RETURNING * | output_expression [ AS output_name ] [, ...] ] @@ -252,10 +252,6 @@ UPDATE films SET kind = 'Dramatic' WHERE kind = 'Drama'; UPDATE weather SET temp_lo = temp_lo+1, temp_hi = temp_lo+15, prcp = DEFAULT WHERE city = 'San Francisco' AND date = '2003-07-03'; - -UPDATE weather SET (temp_lo, temp_hi, prcp) = (temp_lo+1, temp_lo+15, DEFAULT) - WHERE city = 'San Francisco' AND date = '2003-07-03'; - @@ -269,6 +265,14 @@ UPDATE weather SET temp_lo = temp_lo+1, temp_hi = temp_lo+15, prcp = DEFAULT + Use the alternative column-list syntax to do the same update: + +UPDATE weather SET (temp_lo, temp_hi, prcp) = (temp_lo+1, temp_lo+15, DEFAULT) + WHERE city = 'San Francisco' AND date = '2003-07-03'; + + + + Increment the sales count of the salesperson who manages the account for Acme Corporation, using the FROM clause syntax: @@ -317,6 +321,19 @@ COMMIT; + According to the standard, the column-list syntax should allow a list + of columns to be assigned from a single row-valued expression, such + as a sub-select: + +UPDATE accounts SET (contact_last_name, contact_first_name) = + (SELECT last_name, first_name FROM salesmen + WHERE salesmen.id = accounts.sales_id); + + This is not currently implemented — the source must be a list + of independent expressions. + + + Some other database systems offer a FROM option in which the target table is supposed to be listed again within FROM. That is not how PostgreSQL interprets diff --git a/src/backend/parser/gram.y b/src/backend/parser/gram.y index 2ff24296e9..e0d5288795 100644 --- a/src/backend/parser/gram.y +++ b/src/backend/parser/gram.y @@ -11,7 +11,7 @@ * * * IDENTIFICATION - * $PostgreSQL: pgsql/src/backend/parser/gram.y,v 2.564 2006/09/03 03:19:44 momjian Exp $ + * $PostgreSQL: pgsql/src/backend/parser/gram.y,v 2.565 2006/09/03 22:37:05 tgl Exp $ * * HISTORY * AUTHOR DATE MAJOR EVENT @@ -236,9 +236,9 @@ static void doNegateFloat(Value *v); name_list from_clause from_list opt_array_bounds qualified_name_list any_name any_name_list any_operator expr_list attrs - target_list update_col_list update_target_list - update_value_list set_opt insert_column_list - values_list def_list indirection opt_indirection + target_list insert_column_list set_target_list + set_clause_list set_clause multiple_set_clause + ctext_expr_list ctext_row def_list indirection opt_indirection group_clause TriggerFuncArgs select_limit opt_select_limit opclass_item_list transaction_mode_list_or_empty @@ -299,7 +299,7 @@ static void doNegateFloat(Value *v); %type when_clause_list %type sub_type %type OptCreateAs CreateAsList -%type CreateAsElement values_item +%type CreateAsElement ctext_expr %type NumericOnly FloatOnly IntegerOnly %type alias_clause %type sortby @@ -308,8 +308,7 @@ static void doNegateFloat(Value *v); %type joined_table %type relation_expr %type relation_expr_opt_alias -%type target_el update_target_el update_col_list_el insert_column_item -%type update_target_lists_list update_target_lists_el +%type target_el single_set_clause set_target insert_column_item %type Typename SimpleTypename ConstTypename GenericType Numeric opt_float @@ -5488,7 +5487,7 @@ opt_nowait: NOWAIT { $$ = TRUE; } *****************************************************************************/ UpdateStmt: UPDATE relation_expr_opt_alias - SET set_opt + SET set_clause_list from_clause where_clause returning_clause @@ -5503,9 +5502,65 @@ UpdateStmt: UPDATE relation_expr_opt_alias } ; -set_opt: - update_target_list { $$ = $1; } - | update_target_lists_list { $$ = $1; } +set_clause_list: + set_clause { $$ = $1; } + | set_clause_list ',' set_clause { $$ = list_concat($1,$3); } + ; + +set_clause: + single_set_clause { $$ = list_make1($1); } + | multiple_set_clause { $$ = $1; } + ; + +single_set_clause: + set_target '=' ctext_expr + { + $$ = $1; + $$->val = (Node *) $3; + } + ; + +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. XXX this approach + * cannot work for general row expressions as sources. + */ + if (list_length($2) != list_length($5)) + ereport(ERROR, + (errcode(ERRCODE_SYNTAX_ERROR), + errmsg("number of columns does not match number of values"))); + 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: + ColId opt_indirection + { + $$ = makeNode(ResTarget); + $$->name = $1; + $$->indirection = $2; + $$->val = NULL; /* upper production sets this */ + $$->location = @1; + } + ; + +set_target_list: + set_target { $$ = list_make1($1); } + | set_target_list ',' set_target { $$ = lappend($1,$3); } ; @@ -5887,83 +5942,20 @@ locked_rels_list: values_clause: - VALUES '(' values_list ')' + VALUES ctext_row { SelectStmt *n = makeNode(SelectStmt); - n->valuesLists = list_make1($3); + n->valuesLists = list_make1($2); $$ = (Node *) n; } - | values_clause ',' '(' values_list ')' + | values_clause ',' ctext_row { SelectStmt *n = (SelectStmt *) $1; - n->valuesLists = lappend(n->valuesLists, $4); + n->valuesLists = lappend(n->valuesLists, $3); $$ = (Node *) n; } ; -values_list: values_item { $$ = list_make1($1); } - | values_list ',' values_item { $$ = lappend($1, $3); } - ; - -values_item: - a_expr { $$ = (Node *) $1; } - | DEFAULT { $$ = (Node *) makeNode(SetToDefault); } - ; - -update_target_lists_list: - update_target_lists_el { $$ = $1; } - | update_target_lists_list ',' update_target_lists_el { $$ = list_concat($1, $3); } - ; - -update_target_lists_el: - '(' update_col_list ')' '=' '(' update_value_list ')' - { - ListCell *col_cell; - ListCell *val_cell; - - if (list_length($2) != list_length($6)) - { - ereport(ERROR, - (errcode(ERRCODE_SYNTAX_ERROR), - errmsg("number of columns does not match to number of values"))); - } - - for (col_cell = list_head($2), val_cell = list_head($6); - col_cell != NULL && val_cell != NULL; - col_cell = lnext(col_cell), val_cell = lnext(val_cell)) - { - /* merge update_value_list with update_col_list */ - ResTarget *res_col = (ResTarget *) lfirst(col_cell); - Node *res_val = (Node *) lfirst(val_cell); - - res_col->val = res_val; - } - - $$ = $2; - } - ; - -update_col_list: - update_col_list_el { $$ = list_make1($1); } - | update_col_list ',' update_col_list_el { $$ = lappend($1, $3); } - ; - -update_col_list_el: - ColId opt_indirection - { - $$ = makeNode(ResTarget); - $$->name = $1; - $$->indirection = $2; - $$->val = NULL; - $$->location = @1; - } - ; - -update_value_list: - values_item { $$ = list_make1($1); } - | update_value_list ',' values_item { $$ = lappend($1, $3); } - ; - /***************************************************************************** * @@ -8232,10 +8224,35 @@ 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 { $$ = (Node *) makeNode(SetToDefault); } + ; + +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; } + ; + /***************************************************************************** * - * target lists for SELECT, UPDATE, INSERT + * target list for SELECT * *****************************************************************************/ @@ -8275,31 +8292,6 @@ target_el: a_expr AS ColLabel } ; -update_target_list: - update_target_el { $$ = list_make1($1); } - | update_target_list ',' update_target_el { $$ = lappend($1,$3); } - ; - -update_target_el: - ColId opt_indirection '=' a_expr - { - $$ = makeNode(ResTarget); - $$->name = $1; - $$->indirection = $2; - $$->val = (Node *) $4; - $$->location = @1; - } - | ColId opt_indirection '=' DEFAULT - { - $$ = makeNode(ResTarget); - $$->name = $1; - $$->indirection = $2; - $$->val = (Node *) makeNode(SetToDefault); - $$->location = @1; - } - - ; - /***************************************************************************** * diff --git a/src/test/regress/expected/update.out b/src/test/regress/expected/update.out index 55d82628e2..33d52a55a0 100644 --- a/src/test/regress/expected/update.out +++ b/src/test/regress/expected/update.out @@ -1,56 +1,85 @@ -- --- UPDATE ... SET = DEFAULT; +-- UPDATE syntax tests -- CREATE TABLE update_test ( a INT DEFAULT 10, - b INT + b INT, + c TEXT ); -INSERT INTO update_test VALUES (5, 10); -INSERT INTO update_test VALUES (10, 15); +INSERT INTO update_test VALUES (5, 10, 'foo'); +INSERT INTO update_test(b, a) VALUES (15, 10); SELECT * FROM update_test; - a | b -----+---- - 5 | 10 - 10 | 15 + a | b | c +----+----+----- + 5 | 10 | foo + 10 | 15 | (2 rows) UPDATE update_test SET a = DEFAULT, b = DEFAULT; SELECT * FROM update_test; - a | b -----+--- - 10 | - 10 | + a | b | c +----+---+----- + 10 | | foo + 10 | | (2 rows) -- aliases for the UPDATE target table UPDATE update_test AS t SET b = 10 WHERE t.a = 10; SELECT * FROM update_test; - a | b -----+---- - 10 | 10 - 10 | 10 + a | b | c +----+----+----- + 10 | 10 | foo + 10 | 10 | (2 rows) UPDATE update_test t SET b = t.b + 10 WHERE t.a = 10; SELECT * FROM update_test; - a | b -----+---- - 10 | 20 - 10 | 20 + a | b | c +----+----+----- + 10 | 20 | foo + 10 | 20 | (2 rows) -- -- Test VALUES in FROM -- UPDATE update_test SET a=v.i FROM (VALUES(100, 20)) AS v(i, j) - WHERE update_test.b = v.j; + WHERE update_test.b = v.j; SELECT * FROM update_test; - a | b ------+---- - 100 | 20 - 100 | 20 + a | b | c +-----+----+----- + 100 | 20 | foo + 100 | 20 | (2 rows) +-- +-- Test multiple-set-clause syntax +-- +UPDATE update_test SET (c,b,a) = ('bugle', b+11, DEFAULT) WHERE c = 'foo'; +SELECT * FROM update_test; + a | b | c +-----+----+------- + 100 | 20 | + 10 | 31 | bugle +(2 rows) + +UPDATE update_test SET (c,b) = ('car', a+b), a = a + 1 WHERE a = 10; +SELECT * FROM update_test; + a | b | c +-----+----+----- + 100 | 20 | + 11 | 41 | car +(2 rows) + +-- fail, multi assignment to same column: +UPDATE update_test SET (c,b) = ('car', a+b), b = a + 1 WHERE a = 10; +ERROR: multiple assignments to same column "b" +-- XXX this should work, but doesn't yet: +UPDATE update_test SET (a,b) = (select a,b FROM update_test where c = 'foo') + WHERE a = 10; +ERROR: syntax error at or near "select" +LINE 1: UPDATE update_test SET (a,b) = (select a,b FROM update_test ... + ^ -- if an alias for the target table is specified, don't allow references -- to the original table name BEGIN; diff --git a/src/test/regress/sql/update.sql b/src/test/regress/sql/update.sql index 51007b2ff1..2df2995810 100644 --- a/src/test/regress/sql/update.sql +++ b/src/test/regress/sql/update.sql @@ -1,14 +1,15 @@ -- --- UPDATE ... SET = DEFAULT; +-- UPDATE syntax tests -- CREATE TABLE update_test ( a INT DEFAULT 10, - b INT + b INT, + c TEXT ); -INSERT INTO update_test VALUES (5, 10); -INSERT INTO update_test VALUES (10, 15); +INSERT INTO update_test VALUES (5, 10, 'foo'); +INSERT INTO update_test(b, a) VALUES (15, 10); SELECT * FROM update_test; @@ -30,10 +31,25 @@ SELECT * FROM update_test; -- UPDATE update_test SET a=v.i FROM (VALUES(100, 20)) AS v(i, j) - WHERE update_test.b = v.j; + WHERE update_test.b = v.j; SELECT * FROM update_test; +-- +-- Test multiple-set-clause syntax +-- + +UPDATE update_test SET (c,b,a) = ('bugle', b+11, DEFAULT) WHERE c = 'foo'; +SELECT * FROM update_test; +UPDATE update_test SET (c,b) = ('car', a+b), a = a + 1 WHERE a = 10; +SELECT * FROM update_test; +-- fail, multi assignment to same column: +UPDATE update_test SET (c,b) = ('car', a+b), b = a + 1 WHERE a = 10; + +-- XXX this should work, but doesn't yet: +UPDATE update_test SET (a,b) = (select a,b FROM update_test where c = 'foo') + WHERE a = 10; + -- if an alias for the target table is specified, don't allow references -- to the original table name BEGIN; -- 2.11.0