Fix expansion of EXCLUDED virtual generated columns.

If the SET or WHERE clause of an INSERT ... ON CONFLICT command
references EXCLUDED.col, where col is a virtual generated column, the
column was not properly expanded, leading to an "unexpected virtual
generated column reference" error, or incorrect results.

The problem was that expand_virtual_generated_columns() would expand
virtual generated columns in both the SET and WHERE clauses and in the
targetlist of the EXCLUDED pseudo-relation (exclRelTlist). Then
fix_join_expr() from set_plan_refs() would turn the expanded
expressions in the SET and WHERE clauses back into Vars, because they
would be found to match the expression entries in the indexed tlist
produced from exclRelTlist.

To fix this, arrange for expand_virtual_generated_columns() to not
expand virtual generated columns in exclRelTlist. This forces
set_plan_refs() to resolve generation expressions in the query using
non-virtual columns, as required by the executor.

In addition, exclRelTlist now always contains only Vars. That was
something already claimed in a couple of existing comments in the
planner, which relied on that fact to skip some processing, though
those did not appear to constitute active bugs.

Reported-by: Satyanarayana Narlapuram <satyanarlapuram@gmail.com>
Author: Satyanarayana Narlapuram <satyanarlapuram@gmail.com>
Author: Dean Rasheed <dean.a.rasheed@gmail.com>
Discussion: https://postgr.es/m/CAHg+QDf7wTLz_vqb1wi1EJ_4Uh+Vxm75+b4c-Ky=6P+yOAHjbQ@mail.gmail.com
Backpatch-through: 18

diff --git a/src/backend/optimizer/prep/prepjointree.c b/src/backend/optimizer/prep/prepjointree.c
index 95bf51606cc6466455b4b298221e457b4ba42b9d..4424fdbe906b607ac0afa7346f5adfe21c4fcab5 100644 (file)
--- a/src/backend/optimizer/prep/prepjointree.c
+++ b/src/backend/optimizer/prep/prepjointree.c
@@ -501,6 +501,7 @@ expand_virtual_generated_columns(PlannerInfo *root, Query *parse,
        {
                List       *tlist = NIL;
                pullup_replace_vars_context rvcontext;
        {
                List       *tlist = NIL;
                pullup_replace_vars_context rvcontext;

+               List       *save_exclRelTlist = NIL;

 
                for (int i = 0; i < tupdesc->natts; i++)
                {
 
                for (int i = 0; i < tupdesc->natts; i++)
                {


@@ -568,8 +569,26 @@ expand_virtual_generated_columns(PlannerInfo *root, Query *parse,
 
                /*
                 * Apply pullup variable replacement throughout the query tree.
 
                /*
                 * Apply pullup variable replacement throughout the query tree.

+                *
+                * We intentionally do not touch the EXCLUDED pseudo-relation's
+                * targetlist here.  Various places in the planner assume that it
+                * contains only Vars, and we want that to remain the case.  More
+                * importantly, we don't want setrefs.c to turn any expanded
+                * EXCLUDED.virtual_column expressions in other parts of the query
+                * back into Vars referencing the original virtual column, which
+                * set_plan_refs() would do if exclRelTlist contained matching
+                * expressions.

                 */
                 */

+               if (parse->onConflict)
+               {
+                       save_exclRelTlist = parse->onConflict->exclRelTlist;
+                       parse->onConflict->exclRelTlist = NIL;
+               }
+

                parse = (Query *) pullup_replace_vars((Node *) parse, &rvcontext);
                parse = (Query *) pullup_replace_vars((Node *) parse, &rvcontext);

+
+               if (parse->onConflict)
+                       parse->onConflict->exclRelTlist = save_exclRelTlist;

        }
 
        return parse;
        }
 
        return parse;

diff --git a/src/test/regress/expected/generated_virtual.out b/src/test/regress/expected/generated_virtual.out
index 0d9c76c4e77b1246554c23b3b0cd7103db2da12d..262c062417dd6c22a4ca2252df316cbd01ddd128 100644 (file)
--- a/src/test/regress/expected/generated_virtual.out
+++ b/src/test/regress/expected/generated_virtual.out
@@ -1757,3 +1757,51 @@ select * from gtest33 where b is null;
 
 reset constraint_exclusion;
 drop table gtest33;
 
 reset constraint_exclusion;
 drop table gtest33;

+-- Ensure that EXCLUDED.<virtual-generated-column> in INSERT ... ON CONFLICT
+-- DO UPDATE is expanded to the generation expression, both for plain and
+-- partitioned target relations.
+create table gtest34 (id int primary key, a int,
+                      c int generated always as (a * 10) virtual);
+insert into gtest34 values (1, 5);
+insert into gtest34 values (1, 7)
+    on conflict (id) do update set a = excluded.c returning *;
+ id | a  |  c  
+----+----+-----
+  1 | 70 | 700
+(1 row)
+
+insert into gtest34 values (1, 2)
+    on conflict (id) do update set a = gtest34.c + excluded.c returning *;
+ id |  a  |  c   
+----+-----+------
+  1 | 720 | 7200
+(1 row)
+
+insert into gtest34 values (1, 3)
+    on conflict (id) do update set a = 999 where excluded.c > 20 returning *;
+ id |  a  |  c   
+----+-----+------
+  1 | 999 | 9990
+(1 row)
+
+drop table gtest34;
+create table gtest34p (id int primary key, a int,
+                       c int generated always as (a * 10) virtual)
+    partition by range (id);
+create table gtest34p_1 partition of gtest34p for values from (1) to (100);
+insert into gtest34p values (1, 5);
+insert into gtest34p values (1, 7)
+    on conflict (id) do update set a = excluded.c returning *;
+ id | a  |  c  
+----+----+-----
+  1 | 70 | 700
+(1 row)
+
+insert into gtest34p values (1, 2)
+    on conflict (id) do update set a = gtest34p.c + excluded.c returning *;
+ id |  a  |  c   
+----+-----+------
+  1 | 720 | 7200
+(1 row)
+
+drop table gtest34p;

diff --git a/src/test/regress/sql/generated_virtual.sql b/src/test/regress/sql/generated_virtual.sql
index 1bac51ea731f8d0c46ce39993ca3a9ef6ce4ca37..1746807bd3da5676a3ac3e40acc2549dcabcb3ab 100644 (file)
--- a/src/test/regress/sql/generated_virtual.sql
+++ b/src/test/regress/sql/generated_virtual.sql
@@ -929,3 +929,28 @@ select * from gtest33 where b is null;
 
 reset constraint_exclusion;
 drop table gtest33;
 
 reset constraint_exclusion;
 drop table gtest33;

+
+-- Ensure that EXCLUDED.<virtual-generated-column> in INSERT ... ON CONFLICT
+-- DO UPDATE is expanded to the generation expression, both for plain and
+-- partitioned target relations.
+create table gtest34 (id int primary key, a int,
+                      c int generated always as (a * 10) virtual);
+insert into gtest34 values (1, 5);
+insert into gtest34 values (1, 7)
+    on conflict (id) do update set a = excluded.c returning *;
+insert into gtest34 values (1, 2)
+    on conflict (id) do update set a = gtest34.c + excluded.c returning *;
+insert into gtest34 values (1, 3)
+    on conflict (id) do update set a = 999 where excluded.c > 20 returning *;
+drop table gtest34;
+
+create table gtest34p (id int primary key, a int,
+                       c int generated always as (a * 10) virtual)
+    partition by range (id);
+create table gtest34p_1 partition of gtest34p for values from (1) to (100);
+insert into gtest34p values (1, 5);
+insert into gtest34p values (1, 7)
+    on conflict (id) do update set a = excluded.c returning *;
+insert into gtest34p values (1, 2)
+    on conflict (id) do update set a = gtest34p.c + excluded.c returning *;
+drop table gtest34p;