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: v1-0001-Fix-tuple-deformation-with-virtual-generated-NOT-.patch
Description: Binary data

Reply via email to