Horton

Re: Virtual generated columns

Alvaro Herrera <alvherre@alvh.no-ip.org>

View thread
From: Alvaro Herrera <alvherre@alvh.no-ip.org>
To: Peter Eisentraut <peter@eisentraut.org>
Cc: jian he <jian.universality@gmail.com>, pgsql-hackers <pgsql-hackers@postgresql.org>, Dean Rasheed <dean.a.rasheed@gmail.com>
Date: 2024-11-12T16:50:41Z
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 2024-Nov-12, Peter Eisentraut 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.


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.

-- 
Álvaro Herrera         PostgreSQL Developer  —  https://www.EnterpriseDB.com/
"El Maquinismo fue proscrito so pena de cosquilleo hasta la muerte"
(Ijon Tichy en Viajes, Stanislaw Lem)