| 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 02:35:37 |
| Message-ID: | 2A622929-A45C-4370-943A-BC76FF4CC433@gmail.com |
| Views: | Whole Thread | Raw Message | Download mbox | Resend email |
| Thread: | |
| Lists: | pgsql-hackers |
> On Jun 18, 2026, at 07:37, David Rowley <dgrowleyml(at)gmail(dot)com> wrote:
>
> On Thu, 18 Jun 2026 at 11:06, David Rowley <dgrowleyml(at)gmail(dot)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/
| From | Date | Subject | |
|---|---|---|---|
| Next Message | Tatsuo Ishii | 2026-06-18 02:44:40 | Re: IGNORE/RESPECT NULLS can be specified for (prokind == 'f'). |
| Previous Message | Amit Kapila | 2026-06-18 02:34:58 | Re: DOCS - Clarify behaviour when EXCEPT tables are moved/renamed |