Thread

Commits

  1. Update JIT tuple deforming code for virtual generated columns

  2. Add tuple deformation test for virtual generated columns

  3. Fix tuple deforming with virtual generated columns

  1. Fix tuple deformation with virtual generated NOT NULL columns

    Chao Li <li.evan.chao@gmail.com> — 2026-06-04T05:57:05Z

    Hi,
    
    While testing "Optimize tuple deformation”, I found a bug:
    ```
    evantest=# create table t (a int not null,
    evantest(# g int generated always as (a+1) virtual not null,
    evantest(# b int not null);
    CREATE TABLE
    evantest=# insert into t (a, b) values (10, 20);
    INSERT 0 1
    evantest=# select a, g, b from t;
     a  | g  | b
    ----+----+---
     10 | 11 | 0
    (1 row)
    ```
    
    Here, b was inserted as 20, but select only returned 0.
    
    I think the problem is in finding the first non-guaranteed attribute where virtual generated attributes are not considered:
    ```
    	for (int i = 0; i < tupdesc->natts; i++)
    	{
    		CompactAttribute *cattr = TupleDescCompactAttr(tupdesc, i);
    
    		/*
    		 * Find the highest attnum which is guaranteed to exist in all tuples
    		 * in the table.  We currently only pay attention to byval attributes
    		 * to allow additional optimizations during tuple deformation.
    		 */
    		if (firstNonGuaranteedAttr == tupdesc->natts &&
    			(cattr->attnullability != ATTNULLABLE_VALID || !cattr->attbyval ||
    			 cattr->atthasmissing || cattr->attisdropped || cattr->attlen <= 0))
    			firstNonGuaranteedAttr = i;
    ```
    
    To fix this, we should consider virtual generated attributes as non-guaranteed. The tricky part is that cattr->attgenerated is only a boolean and cannot distinguish virtual generated from stored. So we have to further check TupleDescAttr(tupdesc, i)->attgenerated. In the patch, I changed the check as follows:
    ```
    		if (firstNonGuaranteedAttr == tupdesc->natts &&
    			(cattr->attnullability != ATTNULLABLE_VALID || !cattr->attbyval ||
    			 cattr->atthasmissing || cattr->attisdropped || cattr->attlen <= 0 ||
    			 (cattr->attgenerated &&
    			  TupleDescAttr(tupdesc, i)->attgenerated == ATTRIBUTE_GENERATED_VIRTUAL)))
    			firstNonGuaranteedAttr = i;
    ```
    
    This way, we only check TupleDescAttr(tupdesc, i)->attgenerated when needed.
    
    See the attached patch for details. I also added a regression test case to cover this fix. With the fix, select now returns correct values:
    ```
    evantest=# select a, g, b from t;
     a  | g  | b
    ----+----+----
     10 | 11 | 20
    (1 row)
    ```
    
    Best regards,
    --
    Chao Li (Evan)
    HighGo Software Co., Ltd.
    https://www.highgo.com/
    
    
    
    
    
  2. Re: Fix tuple deformation with virtual generated NOT NULL columns

    cca5507 <cca5507@qq.com> — 2026-06-04T09:32:17Z

    > While testing "Optimize tuple deformation”, I found a bug:
    > ```
    > evantest=# create table t (a int not null,
    > evantest(# g int generated always as (a+1) virtual not null,
    > evantest(# b int not null);
    > CREATE TABLE
    > evantest=# insert into t (a, b) values (10, 20);
    > INSERT 0 1
    > evantest=# select a, g, b from t;
    >  a  | g  | b
    > ----+----+---
    >  10 | 11 | 0
    > (1 row)
    > ```
    
    Nice catch! I can reproduce this bug on master. Some comments about the fix:
    
    I find that a virtual generated column is stored as a null in heap tuple, so I think
    we should stop setting 'attcacheoff' when we see a virtual generated column in
    TupleDescFinalize(), or we will set wrong 'attcacheoff' value. But it seems that
    we don't use these wrong value because we can only use 'attcacheoff' up until
    the first NULL.
    
    --
    Regards,
    ChangAo Chen
    
  3. Re: Fix tuple deformation with virtual generated NOT NULL columns

    Chao Li <li.evan.chao@gmail.com> — 2026-06-04T10:30:09Z

    
    > On Jun 4, 2026, at 17:32, cca5507 <cca5507@qq.com> wrote:
    > 
    >> While testing "Optimize tuple deformation”, I found a bug:
    >> ```
    >> evantest=# create table t (a int not null,
    >> evantest(# g int generated always as (a+1) virtual not null,
    >> evantest(# b int not null);
    >> CREATE TABLE
    >> evantest=# insert into t (a, b) values (10, 20);
    >> INSERT 0 1
    >> evantest=# select a, g, b from t;
    >>  a  | g  | b
    >> ----+----+---
    >>  10 | 11 | 0
    >> (1 row)
    >> ```
    > 
    > Nice catch! I can reproduce this bug on master. Some comments about the fix:
    > 
    > I find that a virtual generated column is stored as a null in heap tuple, so I think
    > we should stop setting 'attcacheoff' when we see a virtual generated column in
    > TupleDescFinalize(), or we will set wrong 'attcacheoff' value. But it seems that
    > we don't use these wrong value because we can only use 'attcacheoff' up until
    > the first NULL.
    > 
    > --
    > Regards,
    > ChangAo Chen
    
    Hi ChangAo, 
    
    Thanks for your review. Please see v2 that addressed your comment.
    
    Best regards,
    --
    Chao Li (Evan)
    HighGo Software Co., Ltd.
    https://www.highgo.com/
    
    
    
    
    
  4. Re: Fix tuple deformation with virtual generated NOT NULL columns

    cca5507 <cca5507@qq.com> — 2026-06-05T08:52:46Z

    > Thanks for your review. Please see v2 that addressed your comment.
    
    The v2 patch LGTM.
    
    --
    Regards,
    ChangAo Chen
    
  5. Re: Fix tuple deformation with virtual generated NOT NULL columns

    David Rowley <dgrowleyml@gmail.com> — 2026-06-06T04:50:29Z

    On Thu, 4 Jun 2026 at 17:57, Chao Li <li.evan.chao@gmail.com> 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().
    
    David
    
    
    
    
  6. Re: Fix tuple deformation with virtual generated NOT NULL columns

    Chao Li <li.evan.chao@gmail.com> — 2026-06-07T01:43:34Z

    
    > On Jun 6, 2026, at 12:50, David Rowley <dgrowleyml@gmail.com> wrote:
    > 
    > On Thu, 4 Jun 2026 at 17:57, Chao Li <li.evan.chao@gmail.com> 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.
    
    Thanks for pushing.
    
    > Form_pg_attribute.attgenerated is '\0' for non-generated columns, so
    > there's no point in checking cattr->attgenerated as well as that.
    > 
    
    I was trying to avoid unconditionally fetching TupleDescAttr(tupdesc, i), since cattr->attgenerated is already available and most columns won't be virtual generated columns. But I was probably overthinking the performance angle here. TupleDescFinalize() runs only when finalizing a tuple descriptor, not during per-tuple deformation. I agree your adjusted version is cleaner.
    
    Best regards,
    --
    Chao Li (Evan)
    HighGo Software Co., Ltd.
    https://www.highgo.com/
    
    
    
    
    
    
    
    
  7. Re: Fix tuple deformation with virtual generated NOT NULL columns

    Andres Freund <andres@anarazel.de> — 2026-06-08T13:01:24Z

    Hi,
    
    On 2026-06-06 16:50:29 +1200, David Rowley wrote:
    > On Thu, 4 Jun 2026 at 17:57, Chao Li <li.evan.chao@gmail.com> 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
    
    
    
    
  8. Re: Fix tuple deformation with virtual generated NOT NULL columns

    Chao Li <li.evan.chao@gmail.com> — 2026-06-09T02:19:25Z

    
    > On Jun 8, 2026, at 21:01, Andres Freund <andres@anarazel.de> wrote:
    > 
    > Hi,
    > 
    > On 2026-06-06 16:50:29 +1200, David Rowley wrote:
    >> On Thu, 4 Jun 2026 at 17:57, Chao Li <li.evan.chao@gmail.com> 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/
    
    
    
    
    
  9. Re: Fix tuple deformation with virtual generated NOT NULL columns

    David Rowley <dgrowleyml@gmail.com> — 2026-06-12T04:26:41Z

    On Tue, 9 Jun 2026 at 14:20, Chao Li <li.evan.chao@gmail.com> wrote:
    > > On Jun 8, 2026, at 21:01, Andres Freund <andres@anarazel.de> wrote:
    > > Seems like a test for some of this would be good too?
    
    > 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
    
    I was on the fence about that test as I felt it was mostly
    highlighting that there was a bug once with that particular case, and
    didn't feel like it did much to prevent future omissions from
    TupleDescFinalize(). I felt the Assert would help highlight future
    stuff.
    
    However, I don't object to adding the test if others feel it's
    worthwhile, but on looking at the patch, there are a couple of things
    that stand out.
    
    1. Nothing has been done for the comment at the top of the file that
    says "-- keep these tests aligned with generated_stored.sql".  It
    looks like there have been quite a few commits already which have
    neglected this. Maybe that means we should do away with the comment
    rather than try to align the two.
    2. I can't quite figure out the pattern in these tests for dropping vs
    not dropping the tables at the end of the test. Many tests do DROP
    TABLE and a large number of others don't bother. What's meant to be
    happening here?
    
    I've added Peter as I think it was his intention to keep these tests
    aligned with generated_stored.sql.
    
    David
    
    
    
    
  10. Re: Fix tuple deformation with virtual generated NOT NULL columns

    David Rowley <dgrowleyml@gmail.com> — 2026-06-17T05:04:43Z

    On Fri, 12 Jun 2026 at 16:26, David Rowley <dgrowleyml@gmail.com> wrote:
    > However, I don't object to adding the test if others feel it's
    > worthwhile, but on looking at the patch, there are a couple of things
    > that stand out.
    >
    > 1. Nothing has been done for the comment at the top of the file that
    > says "-- keep these tests aligned with generated_stored.sql".  It
    > looks like there have been quite a few commits already which have
    > neglected this. Maybe that means we should do away with the comment
    > rather than try to align the two.
    
    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.
    
    Since a few commits have already put these files out of sync, maybe we
    should consider having a single test file that we include with \i
    after doing something like:
    
    \set generated_type STORED
    \set test_generated_stored true
    \set test_generated_virtual false
    \i generated_generic.sql
    
    and have generated_generic.sql use \if test_generated_(stored|virtual)
    as and when required.
    
    > 2. I can't quite figure out the pattern in these tests for dropping vs
    > not dropping the tables at the end of the test. Many tests do DROP
    > TABLE and a large number of others don't bother. What's meant to be
    > happening here?
    
    I added the DROP TABLE too, as I didn't see any reason mentioned
    anywhere that they should be kept.
    
    David
    
    
    
    
  11. Re: Fix tuple deformation with virtual generated NOT NULL columns

    Tom Lane <tgl@sss.pgh.pa.us> — 2026-06-17T06:11:01Z

    David Rowley <dgrowleyml@gmail.com> writes:
    >> 2. I can't quite figure out the pattern in these tests for dropping vs
    >> not dropping the tables at the end of the test. Many tests do DROP
    >> TABLE and a large number of others don't bother. What's meant to be
    >> happening here?
    
    > I added the DROP TABLE too, as I didn't see any reason mentioned
    > anywhere that they should be kept.
    
    A very rough rule of thumb is that we leave tables around if they
    might be interesting for the pg_upgrade tests (which try to upgrade
    the ending state of the core regression tests).
    
    			regards, tom lane
    
    
    
    
  12. Re: Fix tuple deformation with virtual generated NOT NULL columns

    Tom Lane <tgl@sss.pgh.pa.us> — 2026-06-17T22:23:00Z

    David Rowley <dgrowleyml@gmail.com> writes:
    > 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.
    
    The buildfarm is showing clear evidence of a bug here somewhere:
    
    https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=bushmaster&dt=2026-06-17%2021%3A53%3A38
    https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=taipan&dt=2026-06-17%2018%3A50%3A42
    https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=canebrake&dt=2026-06-17%2018%3A02%3A59
    
    All three failures look like
    
    diff -U3 /home/bf/bf-build/taipan/HEAD/pgsql/src/test/regress/expected/generated_virtual.out /home/bf/bf-build/taipan/HEAD/pgsql.build/src/test/regress/results/generated_virtual.out
    --- /home/bf/bf-build/taipan/HEAD/pgsql/src/test/regress/expected/generated_virtual.out	2026-06-17 07:00:34.298450176 +0200
    +++ /home/bf/bf-build/taipan/HEAD/pgsql.build/src/test/regress/results/generated_virtual.out	2026-06-17 20:51:26.084674716 +0200
    @@ -731,9 +731,9 @@
     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
    + a  | b  | c 
    +----+----+---
    + 10 | 20 | 0
     (1 row)
     
     DROP TABLE gtest21c;
    
    Speculating wildly, I'm wondering about an uninitialized variable that
    happens to usually have the right value.
    
    			regards, tom lane
    
    
    
    
  13. Re: Fix tuple deformation with virtual generated NOT NULL columns

    Tom Lane <tgl@sss.pgh.pa.us> — 2026-06-17T22:45:49Z

    I wrote:
    > Speculating wildly, I'm wondering about an uninitialized variable that
    > happens to usually have the right value.
    
    Nope: valgrind finds nothing, and neither does debug_discard_caches.
    What does reproduce it, with seeming 100% reliability, is
    
    set jit = 1;
    set jit_above_cost = 0;
    set jit_optimize_above_cost = 1000;
    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);
    table gtest21c;
    
    I conclude that something in the JIT code for tuple formation
    is unaware of virtual generated columns.
    
    			regards, tom lane
    
    
    
    
  14. Re: Fix tuple deformation with virtual generated NOT NULL columns

    David Rowley <dgrowleyml@gmail.com> — 2026-06-17T23:06:54Z

    On Thu, 18 Jun 2026 at 10:45, Tom Lane <tgl@sss.pgh.pa.us> wrote:
    > set jit = 1;
    > set jit_above_cost = 0;
    > set jit_optimize_above_cost = 1000;
    > 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);
    > table gtest21c;
    >
    > I conclude that something in the JIT code for tuple formation
    > is unaware of virtual generated columns.
    
    Just starting to look now, but I suspect that this code in
    llvmjit_deform.c needs to be updated now that we have virtual
    generated columns.
    
    /*
    * 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.
    *
    * 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)
    guaranteed_column_number = attnum;
    
    David
    
    
    
    
  15. Re: Fix tuple deformation with virtual generated NOT NULL columns

    David Rowley <dgrowleyml@gmail.com> — 2026-06-17T23:37:18Z

    On Thu, 18 Jun 2026 at 11:06, David Rowley <dgrowleyml@gmail.com> wrote:
    > Just starting to look now, but I suspect that this code in
    > llvmjit_deform.c needs to be updated now that we have virtual
    > generated columns.
    
    I've not fully processed all this code yet. I've still not worked out
    why the loop that sets guaranteed_column_number doesn't break when it
    finds something non-guaranteed.
    
    Here's a draft patch I was experimenting with. It seems to fix the
    issue. I need to spend more time to check it's correct.
    
    David
    
  16. Re: Fix tuple deformation with virtual generated NOT NULL columns

    Chao Li <li.evan.chao@gmail.com> — 2026-06-18T02:35:37Z

    
    > On Jun 18, 2026, at 07:37, David Rowley <dgrowleyml@gmail.com> wrote:
    > 
    > On Thu, 18 Jun 2026 at 11:06, David Rowley <dgrowleyml@gmail.com> wrote:
    >> Just starting to look now, but I suspect that this code in
    >> llvmjit_deform.c needs to be updated now that we have virtual
    >> generated columns.
    > 
    > I've not fully processed all this code yet. I've still not worked out
    > why the loop that sets guaranteed_column_number doesn't break when it
    > finds something non-guaranteed.
    > 
    > Here's a draft patch I was experimenting with. It seems to fix the
    > issue. I need to spend more time to check it's correct.
    > 
    > David
    > <fix_jit_deform_for_virtual_generated_cols.patch>
    
    I was not aware of the JIT code before. I think the good thing is that the new test uncovered this JIT bug.
    
    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:
    ```
    CREATE TABLE gtest21c (a int NOT NULL, b int GENERATED ALWAYS AS (a * 2) VIRTUAL NOT NULL, c int NOT NULL);
    ```
    
    Here, “c" is a real NOT NULL column, so we can skip “b" and let “c" set guaranteed_column_number. This matters for the later check:
    ```
    	/*
    	 * Check if it is guaranteed that all the desired attributes are available
    	 * in the tuple (but still possibly NULL), by dint of either the last
    	 * to-be-deformed column being NOT NULL, or subsequent ones not accessed
    	 * here being NOT NULL.  If that's not guaranteed the tuple headers natt's
    	 * has to be checked, and missing attributes potentially have to be
    	 * fetched (using slot_getmissingattrs().
    	 */
    	if ((natts - 1) <= guaranteed_column_number)
    	{
    ```
    
    If we break at “b", then this check goes to the else branch and invokes the maxatt check plus slot_getmissingattrs(), even though the later NOT NULL “c" proves that the tuple has enough attributes.
    
    Otherwise, the fix looks good to me.
    
    Best regards,
    --
    Chao Li (Evan)
    HighGo Software Co., Ltd.
    https://www.highgo.com/
    
    
    
    
    
    
    
    
  17. Re: Fix tuple deformation with virtual generated NOT NULL columns

    David Rowley <dgrowleyml@gmail.com> — 2026-06-18T05:18:47Z

    On Thu, 18 Jun 2026 at 14:36, Chao Li <li.evan.chao@gmail.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
    
  18. Re: Fix tuple deformation with virtual generated NOT NULL columns

    Chao Li <li.evan.chao@gmail.com> — 2026-06-18T05:55:58Z

    
    > On Jun 18, 2026, at 13:18, David Rowley <dgrowleyml@gmail.com> wrote:
    > 
    > On Thu, 18 Jun 2026 at 14:36, Chao Li <li.evan.chao@gmail.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/
    
    
    
    
    
    
    
    
  19. Re: Fix tuple deformation with virtual generated NOT NULL columns

    David Rowley <dgrowleyml@gmail.com> — 2026-06-18T06:27:42Z

    On Thu, 18 Jun 2026 at 17:56, Chao Li <li.evan.chao@gmail.com> 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_wHBKdv5PCV5OGCaJOSmJexeaFqfmUHA@mail.gmail.com
    
  20. Re: Fix tuple deformation with virtual generated NOT NULL columns

    Chao Li <li.evan.chao@gmail.com> — 2026-06-18T06:46:04Z

    
    > On Jun 18, 2026, at 14:27, David Rowley <dgrowleyml@gmail.com> wrote:
    > 
    > On Thu, 18 Jun 2026 at 17:56, Chao Li <li.evan.chao@gmail.com> 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.
    
    Thanks for the explanation.
    
    > 
    > I've attached a version with the commented table names fixed. Thanks
    > for looking.
    > 
    > David
    > 
    > [1] https://postgr.es/m/CAApHDvrJBXEhet4=Es_wHBKdv5PCV5OGCaJOSmJexeaFqfmUHA@mail.gmail.com
    > <fix_jit_deform_for_virtual_generated_cols_v3.patch>
    
    This version feels all set.
    
    I also tried to build and run tests, all passed.
    
    Best regards,
    --
    Chao Li (Evan)
    HighGo Software Co., Ltd.
    https://www.highgo.com/
    
    
    
    
    
    
    
    
  21. Re: Fix tuple deformation with virtual generated NOT NULL columns

    David Rowley <dgrowleyml@gmail.com> — 2026-06-19T03:30:11Z

    On Thu, 18 Jun 2026 at 18:46, Chao Li <li.evan.chao@gmail.com> wrote:
    > This version feels all set.
    
    Thanks for looking. I've pushed and backpatched this now.
    
    For the v18 version, I included the tests added by 1f7dfe8c8 in the
    same commit. I did it that way so as not to briefly introduce a test
    that would fail into REL_18_STABLE.
    
    David
    
    
    
    
  22. Re: Fix tuple deformation with virtual generated NOT NULL columns

    Tom Lane <tgl@sss.pgh.pa.us> — 2026-06-19T03:34:27Z

    David Rowley <dgrowleyml@gmail.com> writes:
    > For the v18 version, I included the tests added by 1f7dfe8c8 in the
    > same commit. I did it that way so as not to briefly introduce a test
    > that would fail into REL_18_STABLE.
    
    +1, doing it the other way would create a landmine for git-bisect
    testing.
    
    			regards, tom lane