Update JIT tuple deforming code for virtual generated columns

The JIT deforming code contains an optimization that determines which
columns are guaranteed to exist in the tuple.  That's used to allow
skipping of reading the tuple's natts when the code only needs to deform
attributes that are guaranteed to always exist in all tuples.  83ea6c540
missed updating this code to account for VIRTUAL generated columns.
These are stored as NULLs in the tuple, but may be defined as NOT NULL.
This could result in the code thinking more columns are guaranteed to
exist than actually do.

Author: David Rowley <dgrowleyml@gmail.com>
Reviewed-by: Chao Li <li.evan.chao@gmail.com>
Backpatch-through: 18
Discussion: https://postgr.es/m/1151393.1781734980@sss.pgh.pa.us

diff --git a/src/backend/jit/llvm/llvmjit_deform.c b/src/backend/jit/llvm/llvmjit_deform.c
index 12521e3e46a3bd7897c817a98da4c62eed8b306e..b28316e5b0d95ec75ed0410219f6a420f38efd26 100644 (file)
--- a/src/backend/jit/llvm/llvmjit_deform.c
+++ b/src/backend/jit/llvm/llvmjit_deform.c
@@ -104,27 +104,32 @@ slot_compile_deform(LLVMJitContext *context, TupleDesc desc,
        funcname = llvm_expand_funcname(context, "deform");
 
        /*
        funcname = llvm_expand_funcname(context, "deform");
 
        /*

-        * Check which columns have to exist, so we don't have to check the row's
-        * natts unnecessarily.
+        * Check which columns have to exist in all tuples, so we don't have to
+        * check the row's natts unnecessarily.

         */
        for (attnum = 0; attnum < desc->natts; attnum++)
        {
         */
        for (attnum = 0; attnum < desc->natts; attnum++)
        {

-               CompactAttribute *att = TupleDescCompactAttr(desc, attnum);
+               CompactAttribute *catt = TupleDescCompactAttr(desc, attnum);
+               Form_pg_attribute attr = TupleDescAttr(desc, attnum);

 
                /*
                 * If the column is declared NOT NULL then it must be present in every
                 * tuple, unless there's a "missing" entry that could provide a
                 * non-NULL value for it. That in turn guarantees that the NULL bitmap
                 * - if there are any NULLable columns - is at least long enough to
 
                /*
                 * If the column is declared NOT NULL then it must be present in every
                 * tuple, unless there's a "missing" entry that could provide a
                 * non-NULL value for it. That in turn guarantees that the NULL bitmap
                 * - if there are any NULLable columns - is at least long enough to

-                * cover columns up to attnum.
+                * cover columns up to attnum.  We treat virtual generated columns
+                * similar to atthasmissing columns, as these columns could either not
+                * be represented in the tuple or could have the column represented as
+                * a NULL in the null bitmap.

                 *
                 * Be paranoid and also check !attisdropped, even though the
                 * combination of attisdropped && attnotnull combination shouldn't
                 * exist.
                 */
                 *
                 * Be paranoid and also check !attisdropped, even though the
                 * combination of attisdropped && attnotnull combination shouldn't
                 * exist.
                 */

-               if (att->attnullability == ATTNULLABLE_VALID &&
-                       !att->atthasmissing &&
-                       !att->attisdropped)
+               if (catt->attnullability == ATTNULLABLE_VALID &&
+                       !catt->atthasmissing &&
+                       !catt->attisdropped &&
+                       attr->attgenerated != ATTRIBUTE_GENERATED_VIRTUAL)

                        guaranteed_column_number = attnum;
        }
 
                        guaranteed_column_number = attnum;
        }
 


@@ -392,6 +397,8 @@ slot_compile_deform(LLVMJitContext *context, TupleDesc desc,
        for (attnum = 0; attnum < natts; attnum++)
        {
                CompactAttribute *att = TupleDescCompactAttr(desc, attnum);
        for (attnum = 0; attnum < natts; attnum++)
        {
                CompactAttribute *att = TupleDescCompactAttr(desc, attnum);

+               Form_pg_attribute attr = TupleDescAttr(desc, attnum);
+

                LLVMValueRef v_incby;
                int                     alignto = att->attalignby;
                LLVMValueRef l_attno = l_int16_const(lc, attnum);
                LLVMValueRef v_incby;
                int                     alignto = att->attalignby;
                LLVMValueRef l_attno = l_int16_const(lc, attnum);


@@ -434,9 +441,12 @@ slot_compile_deform(LLVMJitContext *context, TupleDesc desc,
                /*
                 * Check for nulls if necessary. No need to take missing attributes
                 * into account, because if they're present the heaptuple's natts
                /*
                 * Check for nulls if necessary. No need to take missing attributes
                 * into account, because if they're present the heaptuple's natts

-                * would have indicated that a slot_getmissingattrs() is needed.
+                * would have indicated that a slot_getmissingattrs() is needed. When
+                * present in the tuple, virtual generated columns are always stored
+                * as NULL, so we must always perform NULL checks for these.

                 */
                 */

-               if (att->attnullability != ATTNULLABLE_VALID)
+               if (att->attnullability != ATTNULLABLE_VALID ||
+                       attr->attgenerated == ATTRIBUTE_GENERATED_VIRTUAL)

                {
                        LLVMBasicBlockRef b_ifnotnull;
                        LLVMBasicBlockRef b_ifnull;
                {
                        LLVMBasicBlockRef b_ifnotnull;
                        LLVMBasicBlockRef b_ifnull;


@@ -614,12 +624,14 @@ slot_compile_deform(LLVMJitContext *context, TupleDesc desc,
                        known_alignment += att->attlen;
                }
                else if (att->attnullability == ATTNULLABLE_VALID &&
                        known_alignment += att->attlen;
                }
                else if (att->attnullability == ATTNULLABLE_VALID &&

+                                attr->attgenerated != ATTRIBUTE_GENERATED_VIRTUAL &&

                                 (att->attlen % alignto) == 0)
                {
                        /*
                                 (att->attlen % alignto) == 0)
                {
                        /*

-                        * After a NOT NULL fixed-width column with a length that is a
-                        * multiple of its alignment requirement, we know the following
-                        * column is aligned to at least the current column's alignment.
+                        * After a NOT NULL (and not virtual generated) fixed-width column
+                        * with a length that is a multiple of its alignment requirement,
+                        * we know the following column is aligned to at least the current
+                        * column's alignment.

                         */
                        Assert(att->attlen > 0);
                        known_alignment = alignto;
                         */
                        Assert(att->attlen > 0);
                        known_alignment = alignto;

diff --git a/src/test/regress/expected/generated_stored.out b/src/test/regress/expected/generated_stored.out
index f87a756b231e749961f22f96f88218bc9815fe67..e17ba2f4881f5ed9dd42b8d5e9862f6acd133b37 100644 (file)
--- a/src/test/regress/expected/generated_stored.out
+++ b/src/test/regress/expected/generated_stored.out
@@ -726,6 +726,20 @@ INSERT INTO gtest21b (a) VALUES (0);  -- ok now
 --INSERT INTO gtest21c (a, c) VALUES (10, 42);
 --SELECT a, b, c FROM gtest21c;
 --DROP TABLE gtest21c;
 --INSERT INTO gtest21c (a, c) VALUES (10, 42);
 --SELECT a, b, c FROM gtest21c;
 --DROP TABLE gtest21c;

+-- try adding a virtual generated column to an existing table with tuples,
+-- then try adding an atthasmissing column before adding a normal nullable
+-- column.
+--CREATE TABLE gtest21d (a int NOT NULL);
+--INSERT INTO gtest21d (a) VALUES(10);
+--ALTER TABLE gtest21d ADD COLUMN b INT GENERATED ALWAYS AS (a * 10) VIRTUAL NOT NULL;
+--SELECT * FROM gtest21d ORDER BY a;
+--INSERT INTO gtest21d (a) VALUES(20);
+--ALTER TABLE gtest21d ADD COLUMN c INT NOT NULL DEFAULT 1234;
+--SELECT * FROM gtest21d ORDER BY a;
+--ALTER TABLE gtest21d ADD COLUMN d INT;
+--INSERT INTO gtest21d (a, c, d) VALUES(30, 12345, 100);
+--SELECT * FROM gtest21d ORDER BY a;
+--DROP TABLE gtest21d;

 -- not-null constraint with partitioned table
 CREATE TABLE gtestnn_parent (
     f1 int,
 -- not-null constraint with partitioned table
 CREATE TABLE gtestnn_parent (
     f1 int,

diff --git a/src/test/regress/expected/generated_virtual.out b/src/test/regress/expected/generated_virtual.out
index b8d5def44db0e37821edc88ad19a4b863e4350cc..01ee29fee10704dbacb7aa9c26fede69f67fe8dc 100644 (file)
--- a/src/test/regress/expected/generated_virtual.out
+++ b/src/test/regress/expected/generated_virtual.out
@@ -737,6 +737,38 @@ SELECT a, b, c FROM gtest21c;
 (1 row)
 
 DROP TABLE gtest21c;
 (1 row)
 
 DROP TABLE gtest21c;

+-- try adding a virtual generated column to an existing table with tuples,
+-- then try adding an atthasmissing column before adding a normal nullable
+-- column.
+CREATE TABLE gtest21d (a int NOT NULL);
+INSERT INTO gtest21d (a) VALUES(10);
+ALTER TABLE gtest21d ADD COLUMN b INT GENERATED ALWAYS AS (a * 10) VIRTUAL NOT NULL;
+SELECT * FROM gtest21d ORDER BY a;
+ a  |  b  
+----+-----
+ 10 | 100
+(1 row)
+
+INSERT INTO gtest21d (a) VALUES(20);
+ALTER TABLE gtest21d ADD COLUMN c INT NOT NULL DEFAULT 1234;
+SELECT * FROM gtest21d ORDER BY a;
+ a  |  b  |  c   
+----+-----+------
+ 10 | 100 | 1234
+ 20 | 200 | 1234
+(2 rows)
+
+ALTER TABLE gtest21d ADD COLUMN d INT;
+INSERT INTO gtest21d (a, c, d) VALUES(30, 12345, 100);
+SELECT * FROM gtest21d ORDER BY a;
+ a  |  b  |   c   |  d  
+----+-----+-------+-----
+ 10 | 100 |  1234 |    
+ 20 | 200 |  1234 |    
+ 30 | 300 | 12345 | 100
+(3 rows)
+
+DROP TABLE gtest21d;

 -- not-null constraint with partitioned table
 CREATE TABLE gtestnn_parent (
     f1 int,
 -- not-null constraint with partitioned table
 CREATE TABLE gtestnn_parent (
     f1 int,

diff --git a/src/test/regress/sql/generated_stored.sql b/src/test/regress/sql/generated_stored.sql
index 71b0ba6d8d7bfb94cd2bbf3b6d23ac33f00f68d5..85b6212023df01cc3cf6849f98dcb7ef73922578 100644 (file)
--- a/src/test/regress/sql/generated_stored.sql
+++ b/src/test/regress/sql/generated_stored.sql
@@ -373,6 +373,20 @@ INSERT INTO gtest21b (a) VALUES (0);  -- ok now
 --INSERT INTO gtest21c (a, c) VALUES (10, 42);
 --SELECT a, b, c FROM gtest21c;
 --DROP TABLE gtest21c;
 --INSERT INTO gtest21c (a, c) VALUES (10, 42);
 --SELECT a, b, c FROM gtest21c;
 --DROP TABLE gtest21c;

+-- try adding a virtual generated column to an existing table with tuples,
+-- then try adding an atthasmissing column before adding a normal nullable
+-- column.
+--CREATE TABLE gtest21d (a int NOT NULL);
+--INSERT INTO gtest21d (a) VALUES(10);
+--ALTER TABLE gtest21d ADD COLUMN b INT GENERATED ALWAYS AS (a * 10) VIRTUAL NOT NULL;
+--SELECT * FROM gtest21d ORDER BY a;
+--INSERT INTO gtest21d (a) VALUES(20);
+--ALTER TABLE gtest21d ADD COLUMN c INT NOT NULL DEFAULT 1234;
+--SELECT * FROM gtest21d ORDER BY a;
+--ALTER TABLE gtest21d ADD COLUMN d INT;
+--INSERT INTO gtest21d (a, c, d) VALUES(30, 12345, 100);
+--SELECT * FROM gtest21d ORDER BY a;
+--DROP TABLE gtest21d;

 -- not-null constraint with partitioned table
 CREATE TABLE gtestnn_parent (
     f1 int,
 -- not-null constraint with partitioned table
 CREATE TABLE gtestnn_parent (
     f1 int,

diff --git a/src/test/regress/sql/generated_virtual.sql b/src/test/regress/sql/generated_virtual.sql
index 9e3dc99c71d62261cd40b8e632a0629ddc791403..0cb14eb0e36f81079c66303e63735bdd1553fb76 100644 (file)
--- a/src/test/regress/sql/generated_virtual.sql
+++ b/src/test/regress/sql/generated_virtual.sql
@@ -380,6 +380,21 @@ INSERT INTO gtest21c (a, c) VALUES (10, 42);
 SELECT a, b, c FROM gtest21c;
 DROP TABLE gtest21c;
 
 SELECT a, b, c FROM gtest21c;
 DROP TABLE gtest21c;
 

+-- try adding a virtual generated column to an existing table with tuples,
+-- then try adding an atthasmissing column before adding a normal nullable
+-- column.
+CREATE TABLE gtest21d (a int NOT NULL);
+INSERT INTO gtest21d (a) VALUES(10);
+ALTER TABLE gtest21d ADD COLUMN b INT GENERATED ALWAYS AS (a * 10) VIRTUAL NOT NULL;
+SELECT * FROM gtest21d ORDER BY a;
+INSERT INTO gtest21d (a) VALUES(20);
+ALTER TABLE gtest21d ADD COLUMN c INT NOT NULL DEFAULT 1234;
+SELECT * FROM gtest21d ORDER BY a;
+ALTER TABLE gtest21d ADD COLUMN d INT;
+INSERT INTO gtest21d (a, c, d) VALUES(30, 12345, 100);
+SELECT * FROM gtest21d ORDER BY a;
+DROP TABLE gtest21d;
+

 -- not-null constraint with partitioned table
 CREATE TABLE gtestnn_parent (
     f1 int,
 -- not-null constraint with partitioned table
 CREATE TABLE gtestnn_parent (
     f1 int,