Make INSERT-from-multiple-VALUES-rows handle targetlist indirection better.

Previously, if an INSERT with multiple rows of VALUES had indirection
(array subscripting or field selection) in its target-columns list, the
parser handled that by applying transformAssignedExpr() to each element
of each VALUES row independently.  This led to having ArrayRef assignment
nodes or FieldStore nodes in each row of the VALUES RTE.  That works for
simple cases, but in bug #14265 Nuri Boardman points out that it fails
if there are multiple assignments to elements/fields of the same target
column.  For such cases to work, rewriteTargetListIU() has to nest the
ArrayRefs or FieldStores together to produce a single expression to be
assigned to the column.  But it failed to find them in the top-level
targetlist and issued an error about "multiple assignments to same column".

We could possibly fix this by teaching the rewriter to apply
rewriteTargetListIU to each VALUES row separately, but that would be messy
(it would change the output rowtype of the VALUES RTE, for example) and
inefficient.  Instead, let's fix the parser so that the VALUES RTE outputs
are just the user-specified values, cast to the right type if necessary,
and then the ArrayRefs or FieldStores are applied in the top-level
targetlist to Vars representing the RTE's outputs.  This is the same
parsetree representation already used for similar cases with INSERT/SELECT
syntax, so it allows simplifications in ruleutils.c, which no longer needs
to treat INSERT-from-multiple-VALUES as its own special case.

This implementation works by applying transformAssignedExpr to the VALUES
entries as before, and then stripping off any ArrayRefs or FieldStores it
adds.  With lots of VALUES rows it would be noticeably more efficient to
not add those nodes in the first place.  But that's just an optimization
not a bug fix, and there doesn't seem to be any good way to do it without
significant refactoring.  (A non-invasive answer would be to apply
transformAssignedExpr + stripping to just the first VALUES row, and then
just forcibly cast remaining rows to the same data types exposed in the
first row.  But this way would lead to different, not-INSERT-specific
errors being reported in casting failure cases, so it doesn't seem very
nice.)  So leave that for later; this patch at least isn't making the
per-row parsing work worse, and it does make the finished parsetree
smaller, saving rewriter and planner work.

Catversion bump because stored rules containing such INSERTs would need
to change.  Because of that, no back-patch, even though this is a very
long-standing bug.

Report: <20160727005725.7438.26021@wrigleys.postgresql.org>
Discussion: <9578.1469645245@sss.pgh.pa.us>

2 files changed, 127 insertions, 0 deletions

diff --git a/src/test/regress/expected/insert.out b/src/test/regress/expected/insert.out
index 96c7f9e430a..70107b5bf27 100644
--- a/src/test/regress/expected/insert.out
+++ b/src/test/regress/expected/insert.out
@@ -81,3 +81,82 @@ select col1, col2, char_length(col3) from inserttest;
 (8 rows)
 
 drop table inserttest;
+--
+-- check indirection (field/array assignment), cf bug #14265
+--
+-- these tests are aware that transformInsertStmt has 3 separate code paths
+--
+create type insert_test_type as (if1 int, if2 text[]);
+create table inserttest (f1 int, f2 int[],
+                         f3 insert_test_type, f4 insert_test_type[]);
+insert into inserttest (f2[1], f2[2]) values (1,2);
+insert into inserttest (f2[1], f2[2]) values (3,4), (5,6);
+insert into inserttest (f2[1], f2[2]) select 7,8;
+insert into inserttest (f2[1], f2[2]) values (1,default);  -- not supported
+ERROR:  cannot set an array element to DEFAULT
+LINE 1: insert into inserttest (f2[1], f2[2]) values (1,default);
+                                       ^
+insert into inserttest (f3.if1, f3.if2) values (1,array['foo']);
+insert into inserttest (f3.if1, f3.if2) values (1,'{foo}'), (2,'{bar}');
+insert into inserttest (f3.if1, f3.if2) select 3, '{baz,quux}';
+insert into inserttest (f3.if1, f3.if2) values (1,default);  -- not supported
+ERROR:  cannot set a subfield to DEFAULT
+LINE 1: insert into inserttest (f3.if1, f3.if2) values (1,default);
+                                        ^
+insert into inserttest (f3.if2[1], f3.if2[2]) values ('foo', 'bar');
+insert into inserttest (f3.if2[1], f3.if2[2]) values ('foo', 'bar'), ('baz', 'quux');
+insert into inserttest (f3.if2[1], f3.if2[2]) select 'bear', 'beer';
+insert into inserttest (f4[1].if2[1], f4[1].if2[2]) values ('foo', 'bar');
+insert into inserttest (f4[1].if2[1], f4[1].if2[2]) values ('foo', 'bar'), ('baz', 'quux');
+insert into inserttest (f4[1].if2[1], f4[1].if2[2]) select 'bear', 'beer';
+select * from inserttest;
+ f1 |  f2   |        f3        |           f4           
+----+-------+------------------+------------------------
+    | {1,2} |                  | 
+    | {3,4} |                  | 
+    | {5,6} |                  | 
+    | {7,8} |                  | 
+    |       | (1,{foo})        | 
+    |       | (1,{foo})        | 
+    |       | (2,{bar})        | 
+    |       | (3,"{baz,quux}") | 
+    |       | (,"{foo,bar}")   | 
+    |       | (,"{foo,bar}")   | 
+    |       | (,"{baz,quux}")  | 
+    |       | (,"{bear,beer}") | 
+    |       |                  | {"(,\"{foo,bar}\")"}
+    |       |                  | {"(,\"{foo,bar}\")"}
+    |       |                  | {"(,\"{baz,quux}\")"}
+    |       |                  | {"(,\"{bear,beer}\")"}
+(16 rows)
+
+-- also check reverse-listing
+create table inserttest2 (f1 bigint, f2 text);
+create rule irule1 as on insert to inserttest2 do also
+  insert into inserttest (f3.if2[1], f3.if2[2])
+  values (new.f1,new.f2);
+create rule irule2 as on insert to inserttest2 do also
+  insert into inserttest (f4[1].if1, f4[1].if2[2])
+  values (1,'fool'),(new.f1,new.f2);
+create rule irule3 as on insert to inserttest2 do also
+  insert into inserttest (f4[1].if1, f4[1].if2[2])
+  select new.f1, new.f2;
+\d+ inserttest2
+                     Table "public.inserttest2"
+ Column |  Type  | Modifiers | Storage  | Stats target | Description 
+--------+--------+-----------+----------+--------------+-------------
+ f1     | bigint |           | plain    |              | 
+ f2     | text   |           | extended |              | 
+Rules:
+    irule1 AS
+    ON INSERT TO inserttest2 DO  INSERT INTO inserttest (f3.if2[1], f3.if2[2])
+  VALUES (new.f1, new.f2)
+    irule2 AS
+    ON INSERT TO inserttest2 DO  INSERT INTO inserttest (f4[1].if1, f4[1].if2[2]) VALUES (1,'fool'::text), (new.f1,new.f2)
+    irule3 AS
+    ON INSERT TO inserttest2 DO  INSERT INTO inserttest (f4[1].if1, f4[1].if2[2])  SELECT new.f1,
+            new.f2
+
+drop table inserttest2;
+drop table inserttest;
+drop type insert_test_type;
diff --git a/src/test/regress/sql/insert.sql b/src/test/regress/sql/insert.sql
index a0ae85003fb..7924d5d46de 100644
--- a/src/test/regress/sql/insert.sql
+++ b/src/test/regress/sql/insert.sql
@@ -36,3 +36,51 @@ insert into inserttest values(30, 50, repeat('x', 10000));
 select col1, col2, char_length(col3) from inserttest;
 
 drop table inserttest;
+
+--
+-- check indirection (field/array assignment), cf bug #14265
+--
+-- these tests are aware that transformInsertStmt has 3 separate code paths
+--
+
+create type insert_test_type as (if1 int, if2 text[]);
+
+create table inserttest (f1 int, f2 int[],
+                         f3 insert_test_type, f4 insert_test_type[]);
+
+insert into inserttest (f2[1], f2[2]) values (1,2);
+insert into inserttest (f2[1], f2[2]) values (3,4), (5,6);
+insert into inserttest (f2[1], f2[2]) select 7,8;
+insert into inserttest (f2[1], f2[2]) values (1,default);  -- not supported
+
+insert into inserttest (f3.if1, f3.if2) values (1,array['foo']);
+insert into inserttest (f3.if1, f3.if2) values (1,'{foo}'), (2,'{bar}');
+insert into inserttest (f3.if1, f3.if2) select 3, '{baz,quux}';
+insert into inserttest (f3.if1, f3.if2) values (1,default);  -- not supported
+
+insert into inserttest (f3.if2[1], f3.if2[2]) values ('foo', 'bar');
+insert into inserttest (f3.if2[1], f3.if2[2]) values ('foo', 'bar'), ('baz', 'quux');
+insert into inserttest (f3.if2[1], f3.if2[2]) select 'bear', 'beer';
+
+insert into inserttest (f4[1].if2[1], f4[1].if2[2]) values ('foo', 'bar');
+insert into inserttest (f4[1].if2[1], f4[1].if2[2]) values ('foo', 'bar'), ('baz', 'quux');
+insert into inserttest (f4[1].if2[1], f4[1].if2[2]) select 'bear', 'beer';
+
+select * from inserttest;
+
+-- also check reverse-listing
+create table inserttest2 (f1 bigint, f2 text);
+create rule irule1 as on insert to inserttest2 do also
+  insert into inserttest (f3.if2[1], f3.if2[2])
+  values (new.f1,new.f2);
+create rule irule2 as on insert to inserttest2 do also
+  insert into inserttest (f4[1].if1, f4[1].if2[2])
+  values (1,'fool'),(new.f1,new.f2);
+create rule irule3 as on insert to inserttest2 do also
+  insert into inserttest (f4[1].if1, f4[1].if2[2])
+  select new.f1, new.f2;
+\d+ inserttest2
+
+drop table inserttest2;
+drop table inserttest;
+drop type insert_test_type;