Re: fast default vs triggers

On 09/19/2018 10:35 PM, Andrew Dunstan wrote:
> 
> 
> On 09/18/2018 03:36 PM, Andrew Dunstan wrote:
>>
>>
>> Tomas Vondra has pointed out to me that there's an issue with triggers 
>> not getting expanded tuples for columns with fast defaults. Here is an 
>> example that shows the issue:
>>
>>
>>    andrew=# create table blurfl (id int);
>>    CREATE TABLE
>>    andrew=# insert into blurfl select x from generate_series(1,5) x;
>>    INSERT 0 5
>>    andrew=# alter table blurfl add column x int default 100;
>>    ALTER TABLE
>>    andrew=# create or replace function showmej() returns trigger
>>    language plpgsql as $$ declare j json; begin j := to_json(old);
>>    raise notice 'old x: %', j->>'x'; return new; end; $$;
>>    CREATE FUNCTION
>>    andrew=# create trigger show_x before update on blurfl for each row
>>    execute procedure showmej();
>>    CREATE TRIGGER
>>    andrew=# update blurfl set id = id where id = 1;
>>    NOTICE:  old x: <NULL>
>>    UPDATE 1
>>    andrew=# update blurfl set id = id where id = 1;
>>    NOTICE:  old x: 100
>>    UPDATE 1
>>    andrew=#
>>
>>
>> The error is fixed with this patch:
>>
>>
>>    diff --git a/src/backend/commands/trigger.c 
>> b/src/backend/commands/trigger.c
>>    index 2436692..f34a72a 100644
>>    --- a/src/backend/commands/trigger.c
>>    +++ b/src/backend/commands/trigger.c
>>    @@ -3396,7 +3396,11 @@ ltrmark:;
>>             LockBuffer(buffer, BUFFER_LOCK_UNLOCK);
>>         }
>>         -   result = heap_copytuple(&tuple);
>>    +   if (HeapTupleHeaderGetNatts(tuple.t_data) < 
>> relation->rd_att->natts)
>>    +       result = heap_expand_tuple(&tuple, relation->rd_att);
>>    +   else
>>    +       result = heap_copytuple(&tuple);
>>    +
>>         ReleaseBuffer(buffer);
>>              return result;
>>
>> I'm going to re-check the various places that this might have been 
>> missed. I guess it belongs on the open items list.
>>
>>
>>
> 
> 
> 
> I haven't found anything further that is misbehaving. I paid particular 
> attention to indexes and index-only scans.
> 
> I propose to commit this along with an appropriate regression test.
> 

Seems reasonable to me.

regards

-- 
Tomas Vondra                  http://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services