public inbox for [email protected]
help / color / mirror / Atom feedFrom: David Rowley <[email protected]>
To: Chao Li <[email protected]>
Cc: Tom Lane <[email protected]>
Cc: Peter Eisentraut <[email protected]>
Cc: Andres Freund <[email protected]>
Cc: Postgres hackers <[email protected]>
Subject: Re: Fix tuple deformation with virtual generated NOT NULL columns
Date: Thu, 18 Jun 2026 17:18:47 +1200
Message-ID: <CAApHDvqrPnvJcTCT6631OHwMbXmP66uOmhrc7-0KG_8kwJ3wPA@mail.gmail.com> (raw)
In-Reply-To: <[email protected]>
References: <[email protected]>
<CAApHDvoQKY8zHb1LZBuYZBRszi0qVmTaV_zFup=A9xqRpJWMRQ@mail.gmail.com>
<pse3eru75b5skbvc7jjrf7origavqojc6nqtwdrr7z6sjkyxfo@siklvj643v4f>
<[email protected]>
<CAApHDvogTSD_G1nkJqOvO3gZFACQUCbKg6U7yBB48r5RMaR7-Q@mail.gmail.com>
<CAApHDvrJBXEhet4=Es_wHBKdv5PCV5OGCaJOSmJexeaFqfmUHA@mail.gmail.com>
<[email protected]>
<[email protected]>
<CAApHDvpE1Jx=NrwUCihrfP+Dmkfn6kUkpCZYC10Yo+wkMu3ssQ@mail.gmail.com>
<CAApHDvq6FaC441C=G4Q9fNZQ-OKmaeW8yP0YjHpjyjBeWhj3+g@mail.gmail.com>
<[email protected]>
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,
Attachments:
[text/plain] fix_jit_deform_for_virtual_generated_cols_v2.patch (8.3K, 2-fix_jit_deform_for_virtual_generated_cols_v2.patch)
download | inline diff:
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,
view thread (22+ messages) latest in thread
reply
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: [email protected]
Cc: [email protected], [email protected], [email protected], [email protected], [email protected], [email protected]
Subject: Re: Fix tuple deformation with virtual generated NOT NULL columns
In-Reply-To: <CAApHDvqrPnvJcTCT6631OHwMbXmP66uOmhrc7-0KG_8kwJ3wPA@mail.gmail.com>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by agora; see mirroring instructions
for how to clone and mirror all data and code used for this inbox