| From: | Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> |
|---|---|
| To: | Postgres hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org> |
| Cc: | David Rowley <dgrowleyml(at)gmail(dot)com> |
| Subject: | Fix tuple deformation with virtual generated NOT NULL columns |
| Date: | 2026-06-04 05:57:05 |
| Message-ID: | A4BC563C-0CA3-4EF3-952A-EA41F9E5BF1E@gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
Hi,
While testing "Optimize tuple deformation”, I found a bug:
```
evantest=# create table t (a int not null,
evantest(# g int generated always as (a+1) virtual not null,
evantest(# b int not null);
CREATE TABLE
evantest=# insert into t (a, b) values (10, 20);
INSERT 0 1
evantest=# select a, g, b from t;
a | g | b
----+----+---
10 | 11 | 0
(1 row)
```
Here, b was inserted as 20, but select only returned 0.
I think the problem is in finding the first non-guaranteed attribute where virtual generated attributes are not considered:
```
for (int i = 0; i < tupdesc->natts; i++)
{
CompactAttribute *cattr = TupleDescCompactAttr(tupdesc, i);
/*
* Find the highest attnum which is guaranteed to exist in all tuples
* in the table. We currently only pay attention to byval attributes
* to allow additional optimizations during tuple deformation.
*/
if (firstNonGuaranteedAttr == tupdesc->natts &&
(cattr->attnullability != ATTNULLABLE_VALID || !cattr->attbyval ||
cattr->atthasmissing || cattr->attisdropped || cattr->attlen <= 0))
firstNonGuaranteedAttr = i;
```
To fix this, we should consider virtual generated attributes as non-guaranteed. The tricky part is that cattr->attgenerated is only a boolean and cannot distinguish virtual generated from stored. So we have to further check TupleDescAttr(tupdesc, i)->attgenerated. In the patch, I changed the check as follows:
```
if (firstNonGuaranteedAttr == tupdesc->natts &&
(cattr->attnullability != ATTNULLABLE_VALID || !cattr->attbyval ||
cattr->atthasmissing || cattr->attisdropped || cattr->attlen <= 0 ||
(cattr->attgenerated &&
TupleDescAttr(tupdesc, i)->attgenerated == ATTRIBUTE_GENERATED_VIRTUAL)))
firstNonGuaranteedAttr = i;
```
This way, we only check TupleDescAttr(tupdesc, i)->attgenerated when needed.
See the attached patch for details. I also added a regression test case to cover this fix. With the fix, select now returns correct values:
```
evantest=# select a, g, b from t;
a | g | b
----+----+----
10 | 11 | 20
(1 row)
```
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
| Attachment | Content-Type | Size |
|---|---|---|
| v1-0001-Fix-tuple-deformation-with-virtual-generated-NOT-.patch | application/octet-stream | 4.0 KB |
| From | Date | Subject | |
|---|---|---|---|
| Next Message | shveta malik | 2026-06-04 06:01:36 | Re: Proposal: Conflict log history table for Logical Replication |
| Previous Message | Chao Li | 2026-06-04 05:47:04 | Re: SERVICEFILE shows wrong file after servicefile fallback |