On 09/20/2018 09:04 AM, Tomas Vondra wrote:
>
>
> 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.
>
This exposed a further issue with nulls in certain positions. A fix for
both issues, and some regression tests, has been committed.
Thanks to Tomas for his help.
cheers
andrew
--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services