public inbox for [email protected]
help / color / mirror / Atom feedFrom: Chao Li <[email protected]>
To: Andres Freund <[email protected]>
Cc: David Rowley <[email protected]>
Cc: Postgres hackers <[email protected]>
Subject: Re: Fix tuple deformation with virtual generated NOT NULL columns
Date: Tue, 9 Jun 2026 10:19:25 +0800
Message-ID: <[email protected]> (raw)
In-Reply-To: <pse3eru75b5skbvc7jjrf7origavqojc6nqtwdrr7z6sjkyxfo@siklvj643v4f>
References: <[email protected]>
<CAApHDvoQKY8zHb1LZBuYZBRszi0qVmTaV_zFup=A9xqRpJWMRQ@mail.gmail.com>
<pse3eru75b5skbvc7jjrf7origavqojc6nqtwdrr7z6sjkyxfo@siklvj643v4f>
> On Jun 8, 2026, at 21:01, Andres Freund <[email protected]> wrote:
>
> Hi,
>
> On 2026-06-06 16:50:29 +1200, David Rowley wrote:
>> On Thu, 4 Jun 2026 at 17:57, Chao Li <[email protected]> wrote:
>>> While testing "Optimize tuple deformation”, I found a bug:
>>
>>> I think the problem is in finding the first non-guaranteed attribute where virtual generated attributes are not considered:
>>
>> Thanks for the report and fix. I pushed a slightly adjusted version.
>> Form_pg_attribute.attgenerated is '\0' for non-generated columns, so
>> there's no point in checking cattr->attgenerated as well as that.
>>
>> I also added an Assert() to help catch any other reason that the
>> guaranteed column gets set incorrectly again in TupleDescFinalize().
>
> Seems like a test for some of this would be good too?
>
> Greetings,
>
> Andres Freund
Okay, I tried to add a test matching my repro. With this test in place, I reverted the fix and ran make check; it failed by hitting the Assert David added:
```
TRAP: failed Assert("first_null_attr(tup->t_bits, natts) >= firstNullAttr"), File: "execTuples.c", Line: 1083, PID: 65804
0 postgres 0x0000000104e883b8 ExceptionalCondition + 216
1 postgres 0x00000001048e5ebc slot_deform_heap_tuple + 456
2 postgres 0x00000001048e2a18 tts_buffer_heap_getsomeattrs + 112
3 postgres 0x00000001048c5044 slot_getsomeattrs + 68
4 postgres 0x00000001048b97a4 ExecInterpExpr + 416
5 postgres 0x00000001048b8ea0 ExecInterpExprStillValid + 76
6 postgres 0x000000010492ec68 ExecEvalExprNoReturn + 44
7 postgres 0x000000010492ec28 ExecEvalExprNoReturnSwitchContext + 48
8 postgres 0x000000010492eb20 ExecProject + 72
9 postgres 0x000000010492e680 ExecScanExtended + 288
10 postgres 0x000000010492d7cc ExecSeqScanWithProject + 220
11 postgres 0x00000001048db508 ExecProcNodeFirst + 92
12 postgres 0x00000001048d1eb8 ExecProcNode + 60
```
See the attached patch for details.
Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/
Attachments:
[application/octet-stream] v1-0001-Add-regression-test-for-virtual-generated-column-.patch (2.6K, 2-v1-0001-Add-regression-test-for-virtual-generated-column-.patch)
download | inline diff:
From 944e72ad211a9ac694e7fdeb9361f9fe3e2a01d4 Mon Sep 17 00:00:00 2001
From: "Chao Li (Evan)" <[email protected]>
Date: Tue, 9 Jun 2026 09:58:44 +0800
Subject: [PATCH v1] Add regression test for virtual generated column
deformation
Add coverage for a virtual generated NOT NULL column followed by a
physically stored NOT NULL column. This exercises the tuple deformation
case fixed by 89eafad297a, where TupleDescFinalize() could incorrectly
treat a virtual generated column as part of the guaranteed physical column
prefix and compute cached offsets past it.
Without that fix, deforming the following column could read from the wrong
tuple offset.
Suggested-by: Andres Freund <[email protected]>
Author: Chao Li <[email protected]>
Discussion: https://postgr.es/m/A4BC563C-0CA3-4EF3-952A-EA41F9E5BF1E%40gmail.com
---
src/test/regress/expected/generated_virtual.out | 9 +++++++++
src/test/regress/sql/generated_virtual.sql | 5 +++++
2 files changed, 14 insertions(+)
diff --git a/src/test/regress/expected/generated_virtual.out b/src/test/regress/expected/generated_virtual.out
index 24d5dbf46ca..7a5788146f5 100644
--- a/src/test/regress/expected/generated_virtual.out
+++ b/src/test/regress/expected/generated_virtual.out
@@ -727,6 +727,15 @@ ERROR: null value in column "b" of relation "gtest21b" violates not-null constr
DETAIL: Failing row contains (null, virtual).
ALTER TABLE gtest21b ALTER COLUMN b DROP NOT NULL;
INSERT INTO gtest21b (a) VALUES (0); -- ok now
+-- virtual generated columns are not physically stored, even when not null
+CREATE TABLE gtest21c (a int NOT NULL, b int GENERATED ALWAYS AS (a * 2) VIRTUAL NOT NULL, c int NOT NULL);
+INSERT INTO gtest21c (a, c) VALUES (10, 42);
+SELECT a, b, c FROM gtest21c;
+ a | b | c
+----+----+----
+ 10 | 20 | 42
+(1 row)
+
-- 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 9c2bb6590b3..126ae3ecda9 100644
--- a/src/test/regress/sql/generated_virtual.sql
+++ b/src/test/regress/sql/generated_virtual.sql
@@ -374,6 +374,11 @@ INSERT INTO gtest21b (a) VALUES (NULL); -- error
ALTER TABLE gtest21b ALTER COLUMN b DROP NOT NULL;
INSERT INTO gtest21b (a) VALUES (0); -- ok now
+-- virtual generated columns are not physically stored, even when not null
+CREATE TABLE gtest21c (a int NOT NULL, b int GENERATED ALWAYS AS (a * 2) VIRTUAL NOT NULL, c int NOT NULL);
+INSERT INTO gtest21c (a, c) VALUES (10, 42);
+SELECT a, b, c FROM gtest21c;
+
-- not-null constraint with partitioned table
CREATE TABLE gtestnn_parent (
f1 int,
--
2.50.1 (Apple Git-155)
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]
Subject: Re: Fix tuple deformation with virtual generated NOT NULL columns
In-Reply-To: <[email protected]>
* 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