Fix tuple deforming with virtual generated columns

TupleDescFinalize() failed to take into account virtual generated
columns, which are always stored as NULL in tuples.  TupleDescFinalize()
didn't check for this, and that could result in attcacheoff being set for
and beyond virtual generated columns.  Also, the TupleDesc's
firstNonGuaranteedAttr could also be set incorrectly, which could result
in the tuple deformation function deforming without checking for NULLs,
and deforming using incorrectly cached offsets.

This could result in tuples being deformed incorrectly, which could
result in incorrect results, ERRORs or possibly a crash.

This has been broken since c456e39113.

Author: Chao Li <li.evan.chao@gmail.com>
Reported-by: Chao Li <li.evan.chao@gmail.com>
Reviewed-by: ChangAo Chen <cca5507@qq.com>
Reviewed-by: David Rowley <dgrowleyml@gmail.com>
Discussion: https://postgr.es/m/A4BC563C-0CA3-4EF3-952A-EA41F9E5BF1E%40gmail.com

diff --git a/src/backend/access/common/tupdesc.c b/src/backend/access/common/tupdesc.c
index 196472c05d0664c5952665b7b24132bbe2c2b91b..36026d3ec3f1d7d96676df8d41e78f635eb28747 100644 (file)
--- a/src/backend/access/common/tupdesc.c
+++ b/src/backend/access/common/tupdesc.c
@@ -517,6 +517,7 @@ TupleDescFinalize(TupleDesc tupdesc)
        for (int i = 0; i < tupdesc->natts; i++)
        {
                CompactAttribute *cattr = TupleDescCompactAttr(tupdesc, i);
+               Form_pg_attribute attr = TupleDescAttr(tupdesc, i);
 
                /*
                 * Find the highest attnum which is guaranteed to exist in all tuples
@@ -525,10 +526,18 @@ TupleDescFinalize(TupleDesc tupdesc)
                 */
                if (firstNonGuaranteedAttr == tupdesc->natts &&
                        (cattr->attnullability != ATTNULLABLE_VALID || !cattr->attbyval ||
-                        cattr->atthasmissing || cattr->attisdropped || cattr->attlen <= 0))
+                        cattr->atthasmissing || cattr->attisdropped ||
+                        cattr->attlen <= 0 ||
+                        attr->attgenerated == ATTRIBUTE_GENERATED_VIRTUAL))
                        firstNonGuaranteedAttr = i;
 
-               if (cattr->attlen <= 0)
+               /*
+                * Don't cache offsets beyond fixed-width attributes.  Virtual
+                * generated attributes are stored as NULLs in the tuple, so we don't
+                * cache offsets beyond these.
+                */
+               if (cattr->attlen <= 0 ||
+                       attr->attgenerated == ATTRIBUTE_GENERATED_VIRTUAL)
                        break;
 
                off = att_nominal_alignby(off, cattr->attalignby);

diff --git a/src/backend/executor/execTuples.c b/src/backend/executor/execTuples.c
index b0a0028b165bd7fb449c9979e9357b6dc620dafa..7f4ebf954328436b684df1d682e5b3e11ebb3292 100644 (file)
--- a/src/backend/executor/execTuples.c
+++ b/src/backend/executor/execTuples.c
@@ -1074,6 +1074,13 @@ slot_deform_heap_tuple(TupleTableSlot *slot, HeapTuple tuple, uint32 *offp,
                {
                        /* Otherwise all required columns are guaranteed to exist */
                        firstNullAttr = natts;
+
+                       /*
+                        * Check TupleDescFinalize() didn't get confused when setting
+                        * firstNonGuaranteedAttr.  There should never be a NULL in a
+                        * guaranteed column.
+                        */
+                       Assert(first_null_attr(tup->t_bits, natts) >= firstNullAttr);
                }
        }
        else