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 <pe...@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.
