OSDN Git Service

Code review for UPDATE SET (columnlist) patch. Make it handle as much
authorTom Lane <tgl@sss.pgh.pa.us>
Sun, 3 Sep 2006 22:37:06 +0000 (22:37 +0000)
committerTom Lane <tgl@sss.pgh.pa.us>
Sun, 3 Sep 2006 22:37:06 +0000 (22:37 +0000)
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
src/backend/parser/gram.y
src/test/regress/expected/update.out
src/test/regress/sql/update.sql

index 7b5d17a..ec2200f 100644 (file)
@@ -1,5 +1,5 @@
 <!--
-$PostgreSQL: pgsql/doc/src/sgml/ref/update.sgml,v 1.39 2006/09/02 20:34:47 momjian Exp $
+$PostgreSQL: pgsql/doc/src/sgml/ref/update.sgml,v 1.40 2006/09/03 22:37:05 tgl Exp $
 PostgreSQL documentation
 -->
 
@@ -21,8 +21,8 @@ PostgreSQL documentation
  <refsynopsisdiv>
 <synopsis>
 UPDATE [ ONLY ] <replaceable class="PARAMETER">table</replaceable> [ [ AS ] <replaceable class="parameter">alias</replaceable> ]
-    [ SET <replaceable class="PARAMETER">column</replaceable> = { <replaceable class="PARAMETER">expression</replaceable> | DEFAULT } [, ...] |
-      SET ( <replaceable class="PARAMETER">column</replaceable> [, ...] ) = ( { <replaceable class="PARAMETER">expression</replaceable> | DEFAULT } [, ...] ) [, ...] ]
+    SET { <replaceable class="PARAMETER">column</replaceable> = { <replaceable class="PARAMETER">expression</replaceable> | DEFAULT } |
+          ( <replaceable class="PARAMETER">column</replaceable> [, ...] ) = ( { <replaceable class="PARAMETER">expression</replaceable> | DEFAULT } [, ...] ) } [, ...]
     [ FROM <replaceable class="PARAMETER">fromlist</replaceable> ]
     [ WHERE <replaceable class="PARAMETER">condition</replaceable> ]
     [ RETURNING * | <replaceable class="parameter">output_expression</replaceable> [ AS <replaceable class="parameter">output_name</replaceable> ] [, ...] ]
@@ -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';
 </programlisting>
-<programlisting>
-UPDATE weather SET (temp_lo, temp_hi, prcp) = (temp_lo+1, temp_lo+15, DEFAULT)
-  WHERE city = 'San Francisco' AND date = '2003-07-03';
-</programlisting>
   </para>
 
   <para>
@@ -269,6 +265,14 @@ UPDATE weather SET temp_lo = temp_lo+1, temp_hi = temp_lo+15, prcp = DEFAULT
   </para>
 
   <para>
+   Use the alternative column-list syntax to do the same update:
+<programlisting>
+UPDATE weather SET (temp_lo, temp_hi, prcp) = (temp_lo+1, temp_lo+15, DEFAULT)
+  WHERE city = 'San Francisco' AND date = '2003-07-03';
+</programlisting>
+  </para>
+
+  <para>
    Increment the sales count of the salesperson who manages the
    account for Acme Corporation, using the <literal>FROM</literal>
    clause syntax:
@@ -317,6 +321,19 @@ COMMIT;
   </para>
 
   <para>
+   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:
+<programlisting>
+UPDATE accounts SET (contact_last_name, contact_first_name) =
+    (SELECT last_name, first_name FROM salesmen
+     WHERE salesmen.id = accounts.sales_id);
+</programlisting>
+   This is not currently implemented &mdash; the source must be a list
+   of independent expressions.
+  </para>
+
+  <para>
    Some other database systems offer a <literal>FROM</> option in which
    the target table is supposed to be listed again within <literal>FROM</>.
    That is not how <productname>PostgreSQL</productname> interprets
index 2ff2429..e0d5288 100644 (file)
@@ -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 <list>   when_clause_list
 %type <ival>   sub_type
 %type <list>   OptCreateAs CreateAsList
-%type <node>   CreateAsElement values_item
+%type <node>   CreateAsElement ctext_expr
 %type <value>  NumericOnly FloatOnly IntegerOnly
 %type <alias>  alias_clause
 %type <sortby> sortby
@@ -308,8 +308,7 @@ static void doNegateFloat(Value *v);
 %type <jexpr>  joined_table
 %type <range>  relation_expr
 %type <range>  relation_expr_opt_alias
-%type <target> target_el update_target_el update_col_list_el insert_column_item
-%type <list>   update_target_lists_list update_target_lists_el
+%type <target> target_el single_set_clause set_target insert_column_item
 
 %type <typnam> 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;
-                               }
-
-               ;
-
 
 /*****************************************************************************
  *
index 55d8262..33d52a5 100644 (file)
@@ -1,56 +1,85 @@
 --
--- UPDATE ... SET <col> = 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;
index 51007b2..2df2995 100644 (file)
@@ -1,14 +1,15 @@
 --
--- UPDATE ... SET <col> = 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;