On Thu, 18 Jun 2026 at 17:56, Chao Li <[email protected]> wrote:
> I don’t know why you added these commented SQL statements, I guess you have 
> your reason. The problem is, in the 3 SELECTs, gtest21c should probably be 
> gtest21d.

This relates to the "-- keep these tests aligned with
generated_stored.sql" at the top of the file.

I mentioned in [1];
> I pushed the patch to add the new test after modifying it to also
> include the change in the generated_stored test, but with it commented
> out. This is the method that 83ea6c540 introduced.

and this is meant to follow the same pattern.

I've attached a version with the commented table names fixed. Thanks
for looking.

David

[1] 
https://postgr.es/m/CAApHDvrJBXEhet4=es_whbkdv5pcv5ogcajosmjexeafqfm...@mail.gmail.com
diff --git a/src/backend/jit/llvm/llvmjit_deform.c 
b/src/backend/jit/llvm/llvmjit_deform.c
index 12521e3e46a..3e4ecda7b00 100644
--- 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");
 
        /*
-        * 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++)
        {
-               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
-                * 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.
                 */
-               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;
        }
 
@@ -392,6 +397,8 @@ slot_compile_deform(LLVMJitContext *context, TupleDesc desc,
        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);
@@ -435,8 +442,11 @@ 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
                 * 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;
@@ -614,12 +624,14 @@ slot_compile_deform(LLVMJitContext *context, TupleDesc 
desc,
                        known_alignment += att->attlen;
                }
                else if (att->attnullability == ATTNULLABLE_VALID &&
+                                attr->attgenerated != 
ATTRIBUTE_GENERATED_VIRTUAL &&
                                 (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;
diff --git a/src/test/regress/expected/generated_stored.out 
b/src/test/regress/expected/generated_stored.out
index f87a756b231..e17ba2f4881 100644
--- 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;
+-- 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,
diff --git a/src/test/regress/expected/generated_virtual.out 
b/src/test/regress/expected/generated_virtual.out
index b8d5def44db..01ee29fee10 100644
--- 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;
+-- 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,
diff --git a/src/test/regress/sql/generated_stored.sql 
b/src/test/regress/sql/generated_stored.sql
index 71b0ba6d8d7..85b6212023d 100644
--- 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;
+-- 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,
diff --git a/src/test/regress/sql/generated_virtual.sql 
b/src/test/regress/sql/generated_virtual.sql
index 9e3dc99c71d..0cb14eb0e36 100644
--- 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;
 
+-- 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,

Reply via email to