Horton

Re: Virtual generated columns

Peter Eisentraut <peter@eisentraut.org>

View thread
From: Peter Eisentraut <peter@eisentraut.org>
To: Alvaro Herrera <alvherre@alvh.no-ip.org>
Cc: jian he <jian.universality@gmail.com>, pgsql-hackers <pgsql-hackers@postgresql.org>, Dean Rasheed <dean.a.rasheed@gmail.com>
Date: 2024-11-29T09:47:46Z
Lists: pgsql-hackers

Commits

Same data as JSON: GET /api/v1/messages/:b64id/commits the thread's linked commits as JSON, with link sources. API reference →

  1. Expand virtual generated columns for ALTER COLUMN TYPE

  2. Eliminate code duplication in replace_rte_variables callbacks

  3. Expand virtual generated columns in the planner

  4. Virtual generated columns

  5. Additional tests for stored generated columns

  6. Improve generated_stored test

  7. Fix handling of CREATE DOMAIN with GENERATED constraint syntax

  8. Add pg_constraint rows for not-null constraints

  9. Put generated_stored test objects in a schema

  10. Rename regress test generated to generated_stored

  11. Small code simplification

  12. Remove useless code

  13. Remove useless initializations

  14. doc: Clarify that pg_attrdef also stores generation expressions

  15. Clean out column-level pg_init_privs entries when dropping tables.

  16. Re-implement the ereport() macro using __VA_ARGS__.

On 12.11.24 17:50, Alvaro Herrera wrote:
>> On 12.11.24 09:49, jian he wrote:
>>>> On Wed, Nov 6, 2024 at 12:17 AM Peter Eisentraut <peter@eisentraut.org> wrote:
> 
>>> check_modified_virtual_generated, we can replace fastgetattr to
>>> heap_attisnull? like:
>>>               // bool        isnull;
>>>               // fastgetattr(tuple, i + 1, tupdesc, &isnull);
>>>               // if (!isnull)
>>>               //     ereport(ERROR,
>>>               //             (errcode(ERRCODE_E_R_I_E_TRIGGER_PROTOCOL_VIOLATED),
>>>               //              errmsg("trigger modified virtual generated
>>> column value")));
>>>               if (!heap_attisnull(tuple, i+1, tupdesc))
>>>                   ereport(ERROR,
>>>                           (errcode(ERRCODE_E_R_I_E_TRIGGER_PROTOCOL_VIOLATED),
>>>                            errmsg("trigger modified virtual generated
>>> column value")));
>>
>> I don't know.  fastgetattr() is supposed to be "fast". ;-)  It's all inline
>> functions, so maybe that is actually correct.  I don't have a strong opinion
>> either way.
> 
> I think Jian is right: if you're only interested in the isnull bit, then
> heap_attisnull is more appropriate, because it doesn't have to decode
> ("deform") the tuple before giving you the answer; it knows the answer
> by checking just the nulls bitmap.  With fastgetattr you still fetch the
> value from the data bytes, even though your function doesn't care about
> it.  That's probably even measurable for wide tuples if the generated
> attrs are at the end, which sounds common.

Ok, I have fixed that in v10.

> Personally I dislike using 0-based loops for attribute numbers, which
> are 1-based.  For peace of mind, I'd write this as
> 
>     for (AttrNumber i = 1; i <= tupdesc->natts; i++)
>     {
>         if (TupleDescAttr(tupdesc, i - 1)->attgenerated == ATTRIBUTE_GENERATED_VIRTUAL)
>         {
>             bool        isnull;
> 
>             fastgetattr(tuple, i, tupdesc, &isnull); // heap_attisnull here actually
> 
> I'm kind of annoyed that TupleDescAttr() was made to refer to array
> indexes rather than attribute numbers, but by the time I realized it had
> happened, it was too late.

Yes, this is unfortunately a constant source of confusion.