public inbox for [email protected]
help / color / mirror / Atom feed[BUG] ON CONFLICT DO UPDATE SET x = EXCLUDED.<virtual-generated-column> errors or silently writes NULL
4+ messages / 2 participants
[nested] [flat]
* [BUG] ON CONFLICT DO UPDATE SET x = EXCLUDED.<virtual-generated-column> errors or silently writes NULL
@ 2026-04-16 20:48 SATYANARAYANA NARLAPURAM <[email protected]>
2026-04-18 18:14 ` Re: [BUG] ON CONFLICT DO UPDATE SET x = EXCLUDED.<virtual-generated-column> errors or silently writes NULL Dean Rasheed <[email protected]>
0 siblings, 1 reply; 4+ messages in thread
From: SATYANARAYANA NARLAPURAM @ 2026-04-16 20:48 UTC (permalink / raw)
To: PostgreSQL Hackers <[email protected]>
Hi Hackers,
Virtual generated column (bgc) behavior for plain and partitioned tables is
different
when EXCLUDED.<vgc> references inside for INSERT ... ON CONFLICT DO UPDATE.
For plain table it errors out with the message "unexpected virtual
generated column reference"
and for partitioned tables, it silently writes NULL (wrong data).
Repro:
-- plan table
DROP TABLE IF EXISTS t;
CREATE TABLE t (id int PRIMARY KEY,
a int,
c int GENERATED ALWAYS AS (a * 10) VIRTUAL);
INSERT INTO t VALUES (1, 5);
INSERT INTO t VALUES (1, 7)
ON CONFLICT (id) DO UPDATE SET a = EXCLUDED.c;
-- ERROR: unexpected virtual generated column reference
-- Partitioned table:
DROP TABLE IF EXISTS tp;
CREATE TABLE tp (id int PRIMARY KEY,
a int,
c int GENERATED ALWAYS AS (a * 10) VIRTUAL)
PARTITION BY RANGE (id);
CREATE TABLE tp1 PARTITION OF tp FOR VALUES FROM (1) TO (100);
INSERT INTO tp VALUES (1, 5);
INSERT INTO tp VALUES (1, 7)
ON CONFLICT (id) DO UPDATE SET a = EXCLUDED.c;
SELECT * FROM tp
id | a | c
----+---+---
1 | |
We have two options to fix, (1) throw an error for partitioned tables
similar to plain tables or
(2) support the scenario fixing for both the cases.
I tried fixing this by replacing build_tlist_index with
build_tlist_index_other_vars . This fix
works because build_tlist_index_other_vars only indexes plain-Var TEs of
exclRelTlist and
leaves has_non_vars = false, so fix_join_expr skips whole-subtree matching
and never collapses
the VGC-expanded (EXCLUDED.a * 10) in onConflictSet back into a
Var(INNER_VAR, vgc_attno).
I am not super familiar with this area so I am not sure if this breaks
anything. Ran the existing
tests and they seem to be passing.
Thanks,
Satya
Attachments:
[application/octet-stream] vgc-excluded-fix.patch (5.0K, 3-vgc-excluded-fix.patch)
download | inline diff:
diff --git a/src/backend/optimizer/plan/setrefs.c b/src/backend/optimizer/plan/setrefs.c
index ff0e875f..bb254079 100644
--- a/src/backend/optimizer/plan/setrefs.c
+++ b/src/backend/optimizer/plan/setrefs.c
@@ -167,6 +167,7 @@ static void set_param_references(PlannerInfo *root, Plan *plan);
static Node *convert_combining_aggrefs(Node *node, void *context);
static void set_dummy_tlist_references(Plan *plan, int rtoffset);
static indexed_tlist *build_tlist_index(List *tlist);
+static indexed_tlist *build_tlist_index_other_vars(List *tlist, int ignore_rel);
static Var *search_indexed_tlist_for_var(Var *var,
indexed_tlist *itlist,
int newvarno,
@@ -1168,7 +1169,8 @@ set_plan_refs(PlannerInfo *root, Plan *plan, int rtoffset)
{
indexed_tlist *itlist;
- itlist = build_tlist_index(splan->exclRelTlist);
+ itlist = build_tlist_index_other_vars(splan->exclRelTlist,
+ 0);
splan->onConflictSet =
fix_join_expr(root, splan->onConflictSet,
diff --git a/src/test/regress/expected/generated_virtual.out b/src/test/regress/expected/generated_virtual.out
index fc41c480..8f5ad91b 100644
--- a/src/test/regress/expected/generated_virtual.out
+++ b/src/test/regress/expected/generated_virtual.out
@@ -1723,3 +1723,55 @@ select * from gtest33 where b is null;
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. Previously, the plan-time setrefs step
+-- would re-collapse the expanded expression into a Var referring to the
+-- virtual attnum of the EXCLUDED slot, producing either an "unexpected
+-- virtual generated column reference" error (plain table) or a silent NULL
+-- (partitioned table, via tuple routing's attribute remapping).
+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 9b32413e..6e958011 100644
--- a/src/test/regress/sql/generated_virtual.sql
+++ b/src/test/regress/sql/generated_virtual.sql
@@ -906,3 +906,32 @@ select * from gtest33 where b is null;
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. Previously, the plan-time setrefs step
+-- would re-collapse the expanded expression into a Var referring to the
+-- virtual attnum of the EXCLUDED slot, producing either an "unexpected
+-- virtual generated column reference" error (plain table) or a silent NULL
+-- (partitioned table, via tuple routing's attribute remapping).
+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;
^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: [BUG] ON CONFLICT DO UPDATE SET x = EXCLUDED.<virtual-generated-column> errors or silently writes NULL
2026-04-16 20:48 [BUG] ON CONFLICT DO UPDATE SET x = EXCLUDED.<virtual-generated-column> errors or silently writes NULL SATYANARAYANA NARLAPURAM <[email protected]>
@ 2026-04-18 18:14 ` Dean Rasheed <[email protected]>
2026-04-20 00:55 ` Re: [BUG] ON CONFLICT DO UPDATE SET x = EXCLUDED.<virtual-generated-column> errors or silently writes NULL SATYANARAYANA NARLAPURAM <[email protected]>
0 siblings, 1 reply; 4+ messages in thread
From: Dean Rasheed @ 2026-04-18 18:14 UTC (permalink / raw)
To: SATYANARAYANA NARLAPURAM <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>
On Thu, 16 Apr 2026 at 21:49, SATYANARAYANA NARLAPURAM
<[email protected]> wrote:
>
> Virtual generated column (bgc) behavior for plain and partitioned tables is different
> when EXCLUDED.<vgc> references inside for INSERT ... ON CONFLICT DO UPDATE.
> For plain table it errors out with the message "unexpected virtual generated column reference"
> and for partitioned tables, it silently writes NULL (wrong data).
Nice catch!
> I tried fixing this by replacing build_tlist_index with build_tlist_index_other_vars . This fix
> works because build_tlist_index_other_vars only indexes plain-Var TEs of exclRelTlist and
> leaves has_non_vars = false, so fix_join_expr skips whole-subtree matching and never collapses
> the VGC-expanded (EXCLUDED.a * 10) in onConflictSet back into a Var(INNER_VAR, vgc_attno).
This doesn't quite work in all cases -- if the generated expression is
simply a Var, then it is found in the indexed tlist without the
non_var matching code, leading to the same problem. For example,
modifying your original test case to this:
CREATE TABLE t (id int PRIMARY KEY,
c int GENERATED ALWAYS AS (a) VIRTUAL, a int);
Admittedly, that's a rather silly example, but we really ought to have
a fix that works for all cases.
Looking more closely, I think the right fix is to not expand virtual
generated columns in the targetlist of EXCLUDED (exclRelTlist), so
then they will not be found as matching expressions in the setrefs.c
code.
I also noticed that there are already a couple of places in the
planner that claim that exclRelTlist contains only Vars, so this
approach makes that claim true (though I don't think those other
places represented actual bugs).
Attached is a v2 patch doing it that way, with the same tests, which all pass.
Regards,
Dean
Attachments:
[text/x-patch] v2-0001-Fix-expansion-of-virtual-generated-columns-in-EXC.patch (6.5K, 2-v2-0001-Fix-expansion-of-virtual-generated-columns-in-EXC.patch)
download | inline diff:
From a0b352a8b98ee28a7faba0ed7b8684bb0f9d273c Mon Sep 17 00:00:00 2001
From: Dean Rasheed <[email protected]>
Date: Sat, 18 Apr 2026 18:49:38 +0100
Subject: [PATCH v2] Fix expansion of virtual generated columns in EXCLUDED.
If the SET or WHERE clause of an INSERT ... ON CONFLICT DO UPDATE
contained references to virtual generated columns of the EXCLUDED
pseudo-relation, they were not be properly expanded, leading to an
error, or wrong 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. 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
entries in the indexed tlist produced from exclRelTlist. To prevent
that from happening, do not expand virtual generated columns in the
EXCLUDED pseudo-relation's targetlist.
As a result, exclRelTlist now always contains only Vars -- something
already claimed in a couple of existing comments in the planner, which
relied on that to skip processing it (though those did not appear to
constitute active bugs).
Reported-by: Satyanarayana Narlapuram <[email protected]>
Author: Satyanarayana Narlapuram <[email protected]>
Author: Dean Rasheed <[email protected]>
Discussion: https://postgr.es/m/CAHg+QDf7wTLz_vqb1wi1EJ_4Uh+Vxm75+b4c-Ky=6P+yOAHjbQ@mail.gmail.com
Backpatch-through: 18
---
src/backend/optimizer/prep/prepjointree.c | 19 ++++++++
.../regress/expected/generated_virtual.out | 48 +++++++++++++++++++
src/test/regress/sql/generated_virtual.sql | 25 ++++++++++
3 files changed, 92 insertions(+)
diff --git a/src/backend/optimizer/prep/prepjointree.c b/src/backend/optimizer/prep/prepjointree.c
index 95bf51606cc..4424fdbe906 100644
--- 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 *save_exclRelTlist = NIL;
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.
+ *
+ * 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);
+
+ if (parse->onConflict)
+ parse->onConflict->exclRelTlist = save_exclRelTlist;
}
return parse;
diff --git a/src/test/regress/expected/generated_virtual.out b/src/test/regress/expected/generated_virtual.out
index fc41c480d40..4eb5d5376d0 100644
--- a/src/test/regress/expected/generated_virtual.out
+++ b/src/test/regress/expected/generated_virtual.out
@@ -1723,3 +1723,51 @@ select * from gtest33 where b is null;
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 9b32413e3a9..4f0c2d57c1f 100644
--- a/src/test/regress/sql/generated_virtual.sql
+++ b/src/test/regress/sql/generated_virtual.sql
@@ -906,3 +906,28 @@ select * from gtest33 where b is null;
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;
--
2.51.0
^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: [BUG] ON CONFLICT DO UPDATE SET x = EXCLUDED.<virtual-generated-column> errors or silently writes NULL
2026-04-16 20:48 [BUG] ON CONFLICT DO UPDATE SET x = EXCLUDED.<virtual-generated-column> errors or silently writes NULL SATYANARAYANA NARLAPURAM <[email protected]>
2026-04-18 18:14 ` Re: [BUG] ON CONFLICT DO UPDATE SET x = EXCLUDED.<virtual-generated-column> errors or silently writes NULL Dean Rasheed <[email protected]>
@ 2026-04-20 00:55 ` SATYANARAYANA NARLAPURAM <[email protected]>
2026-04-22 08:12 ` Re: [BUG] ON CONFLICT DO UPDATE SET x = EXCLUDED.<virtual-generated-column> errors or silently writes NULL Dean Rasheed <[email protected]>
0 siblings, 1 reply; 4+ messages in thread
From: SATYANARAYANA NARLAPURAM @ 2026-04-20 00:55 UTC (permalink / raw)
To: Dean Rasheed <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>
HI,
On Sat, Apr 18, 2026 at 11:14 AM Dean Rasheed <[email protected]>
wrote:
> On Thu, 16 Apr 2026 at 21:49, SATYANARAYANA NARLAPURAM
> <[email protected]> wrote:
> >
> > Virtual generated column (bgc) behavior for plain and partitioned tables
> is different
> > when EXCLUDED.<vgc> references inside for INSERT ... ON CONFLICT DO
> UPDATE.
> > For plain table it errors out with the message "unexpected virtual
> generated column reference"
> > and for partitioned tables, it silently writes NULL (wrong data).
>
> Nice catch!
>
> > I tried fixing this by replacing build_tlist_index with
> build_tlist_index_other_vars . This fix
> > works because build_tlist_index_other_vars only indexes plain-Var TEs of
> exclRelTlist and
> > leaves has_non_vars = false, so fix_join_expr skips whole-subtree
> matching and never collapses
> > the VGC-expanded (EXCLUDED.a * 10) in onConflictSet back into a
> Var(INNER_VAR, vgc_attno).
>
> This doesn't quite work in all cases -- if the generated expression is
> simply a Var, then it is found in the indexed tlist without the
> non_var matching code, leading to the same problem. For example,
> modifying your original test case to this:
>
> CREATE TABLE t (id int PRIMARY KEY,
> c int GENERATED ALWAYS AS (a) VIRTUAL, a int);
>
> Admittedly, that's a rather silly example, but we really ought to have
> a fix that works for all cases.
>
Agreed.
>
> Looking more closely, I think the right fix is to not expand virtual
> generated columns in the targetlist of EXCLUDED (exclRelTlist), so
> then they will not be found as matching expressions in the setrefs.c
> code.
>
> I also noticed that there are already a couple of places in the
> planner that claim that exclRelTlist contains only Vars, so this
> approach makes that claim true (though I don't think those other
> places represented actual bugs).
>
> Attached is a v2 patch doing it that way, with the same tests, which all
> pass.
>
Reran the failing tests and they all passed. Additionally ran the
regression tests.
Patch looks good to me.
>
> Regards,
> Dean
>
^ permalink raw reply [nested|flat] 4+ messages in thread
* Re: [BUG] ON CONFLICT DO UPDATE SET x = EXCLUDED.<virtual-generated-column> errors or silently writes NULL
2026-04-16 20:48 [BUG] ON CONFLICT DO UPDATE SET x = EXCLUDED.<virtual-generated-column> errors or silently writes NULL SATYANARAYANA NARLAPURAM <[email protected]>
2026-04-18 18:14 ` Re: [BUG] ON CONFLICT DO UPDATE SET x = EXCLUDED.<virtual-generated-column> errors or silently writes NULL Dean Rasheed <[email protected]>
2026-04-20 00:55 ` Re: [BUG] ON CONFLICT DO UPDATE SET x = EXCLUDED.<virtual-generated-column> errors or silently writes NULL SATYANARAYANA NARLAPURAM <[email protected]>
@ 2026-04-22 08:12 ` Dean Rasheed <[email protected]>
0 siblings, 0 replies; 4+ messages in thread
From: Dean Rasheed @ 2026-04-22 08:12 UTC (permalink / raw)
To: SATYANARAYANA NARLAPURAM <[email protected]>; +Cc: PostgreSQL Hackers <[email protected]>
On Mon, 20 Apr 2026 at 01:55, SATYANARAYANA NARLAPURAM
<[email protected]> wrote:
>
>> Attached is a v2 patch doing it that way, with the same tests, which all pass.
>
> Reran the failing tests and they all passed. Additionally ran the regression tests.
> Patch looks good to me.
>
Thanks for checking. Patch pushed and back-patched to v18.
Regards,
Dean
^ permalink raw reply [nested|flat] 4+ messages in thread
end of thread, other threads:[~2026-04-22 08:12 UTC | newest]
Thread overview: 4+ messages (download: mbox mbox.gz follow: Atom feed)
-- links below jump to the message on this page --
2026-04-16 20:48 [BUG] ON CONFLICT DO UPDATE SET x = EXCLUDED.<virtual-generated-column> errors or silently writes NULL SATYANARAYANA NARLAPURAM <[email protected]>
2026-04-18 18:14 ` Dean Rasheed <[email protected]>
2026-04-20 00:55 ` SATYANARAYANA NARLAPURAM <[email protected]>
2026-04-22 08:12 ` Dean Rasheed <[email protected]>
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox