Fix tuple deformation with virtual generated NOT NULL columns - Mailing list pgsql-hackers

From Chao Li
Subject Fix tuple deformation with virtual generated NOT NULL columns
Date
Msg-id A4BC563C-0CA3-4EF3-952A-EA41F9E5BF1E@gmail.com
Whole thread
Responses Re: Fix tuple deformation with virtual generated NOT NULL columns
Re: Fix tuple deformation with virtual generated NOT NULL columns
List 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->attgeneratedis 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
returnscorrect 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

pgsql-hackers by date:

Previous
From: Chao Li
Date:
Subject: Re: SERVICEFILE shows wrong file after servicefile fallback
Next
From: shveta malik
Date:
Subject: Re: Proposal: Conflict log history table for Logical Replication