fast default vs triggers

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.

cheers

andrew

--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services

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

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