Обсуждение: fast default vs triggers
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.
cheers
andrew
--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
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