On Thu, 18 Jun 2026 at 14:36, Chao Li <[email protected]> wrote:
> I tested the fix, and it seems to work. While tracing the code, I wondered
> about this part:
> ```
> - if (att->attnullability == ATTNULLABLE_VALID &&
> - !att->atthasmissing &&
> - !att->attisdropped)
> + if (attr->attgenerated == ATTRIBUTE_GENERATED_VIRTUAL)
> + break;
> +
> + if (catt->attnullability == ATTNULLABLE_VALID &&
> + !catt->atthasmissing &&
> + !catt->attisdropped)
> guaranteed_column_number = attnum;
> ```
>
> When computing guaranteed_column_number, I think we can just skip the virtual
> generated column rather than break. Using the test from Tom’s email:
Yeah, I was confused at first as I'd done a similar optimisation in
the non-JIT deform code, but there "guaranteed" means guaranteed to be
present in the tuple data, whereas with the JIT code it means
guaranteed in the tuple data or its NULL bitmap.
I've attached v2 which includes a test that exercises deforming with
tuples which have various natts counts. I propose to backpatch
1f7dfe8c8 to v18 before applying the attached to master and v18.
David
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..a7fe8033fa3 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 gtest21c ORDER BY a;
+--INSERT INTO gtest21d (a) VALUES(20);
+--ALTER TABLE gtest21d ADD COLUMN c INT NOT NULL DEFAULT 1234;
+--SELECT * FROM gtest21c ORDER BY a;
+--ALTER TABLE gtest21d ADD COLUMN d INT;
+--INSERT INTO gtest21d (a, c, d) VALUES(30, 12345, 100);
+--SELECT * FROM gtest21c 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..6a4fddbfdf1 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 gtest21c ORDER BY a;
+--INSERT INTO gtest21d (a) VALUES(20);
+--ALTER TABLE gtest21d ADD COLUMN c INT NOT NULL DEFAULT 1234;
+--SELECT * FROM gtest21c ORDER BY a;
+--ALTER TABLE gtest21d ADD COLUMN d INT;
+--INSERT INTO gtest21d (a, c, d) VALUES(30, 12345, 100);
+--SELECT * FROM gtest21c 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,
