Re: Fix tuple deformation with virtual generated NOT NULL columns

From: Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com>
To: David Rowley <dgrowleyml(at)gmail(dot)com>
Cc: Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Peter Eisentraut <peter(at)eisentraut(dot)org>, Andres Freund <andres(at)anarazel(dot)de>, Postgres hackers <pgsql-hackers(at)lists(dot)postgresql(dot)org>
Subject: Re: Fix tuple deformation with virtual generated NOT NULL columns
Date: 2026-06-18 05:55:58
Message-ID: 03C47613-5C89-4E69-BB72-3B2912BDDDA8@gmail.com
Views: Whole Thread | Raw Message | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

> On Jun 18, 2026, at 13:18, David Rowley <dgrowleyml(at)gmail(dot)com> wrote:
>
> On Thu, 18 Jun 2026 at 14:36, Chao Li <li(dot)evan(dot)chao(at)gmail(dot)com> 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
> <fix_jit_deform_for_virtual_generated_cols_v2.patch>

This version looks good to me. Only a small comment:

```
+-- 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;
```

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.

Best regards,
--
Chao Li (Evan)
HighGo Software Co., Ltd.
https://www.highgo.com/

In response to

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Shlok Kyal 2026-06-18 05:59:40 Re: Proposal: Conflict log history table for Logical Replication
Previous Message Ashutosh Bapat 2026-06-18 05:48:16 Re: (SQL/PGQ) cache lookup failed for label