From 7636e5c60fea83a9f3cd2ad278c0819b98941c74 Mon Sep 17 00:00:00 2001 From: Andrew Dunstan Date: Mon, 24 Sep 2018 16:11:24 -0400 Subject: [PATCH] Fast default trigger and expand_tuple fixes Ensure that triggers get properly filled in tuples for the OLD value. Also fix the logic of detecting missing null values. The previous logic failed to detect a missing null column before the first missing column with a default. Fixing this has simplified the logic a bit. Regression tests are added to test changes. This should ensure better coverage of expand_tuple(). Original bug reports, and some code and test scripts from Tomas Vondra Backpatch to release 11. --- src/backend/access/common/heaptuple.c | 51 +++--- src/backend/commands/trigger.c | 5 +- src/test/regress/expected/fast_default.out | 191 ++++++++++++++++++++- src/test/regress/sql/fast_default.sql | 114 +++++++++++- 4 files changed, 328 insertions(+), 33 deletions(-) diff --git a/src/backend/access/common/heaptuple.c b/src/backend/access/common/heaptuple.c index d8b06bca7e..7fc2fee74a 100644 --- a/src/backend/access/common/heaptuple.c +++ b/src/backend/access/common/heaptuple.c @@ -823,44 +823,35 @@ expand_tuple(HeapTuple *targetHeapTuple, { if (attrmiss[firstmissingnum].am_present) break; + else + hasNulls = true; } /* - * If there are no more missing values everything else must be NULL + * Now walk the missing attributes. If there is a missing value + * make space for it. Otherwise, it's going to be NULL. */ - if (firstmissingnum >= natts) + for (attnum = firstmissingnum; + attnum < natts; + attnum++) { - hasNulls = true; - } - else - { - - /* - * Now walk the missing attributes. If there is a missing value - * make space for it. Otherwise, it's going to be NULL. - */ - for (attnum = firstmissingnum; - attnum < natts; - attnum++) + if (attrmiss[attnum].am_present) { - if (attrmiss[attnum].am_present) - { - Form_pg_attribute att = TupleDescAttr(tupleDesc, attnum); + Form_pg_attribute att = TupleDescAttr(tupleDesc, attnum); - targetDataLen = att_align_datum(targetDataLen, - att->attalign, - att->attlen, - attrmiss[attnum].am_value); + targetDataLen = att_align_datum(targetDataLen, + att->attalign, + att->attlen, + attrmiss[attnum].am_value); - targetDataLen = att_addlength_pointer(targetDataLen, - att->attlen, - attrmiss[attnum].am_value); - } - else - { - /* no missing value, so it must be null */ - hasNulls = true; - } + targetDataLen = att_addlength_pointer(targetDataLen, + att->attlen, + attrmiss[attnum].am_value); + } + else + { + /* no missing value, so it must be null */ + hasNulls = true; } } } /* end if have missing values */ diff --git a/src/backend/commands/trigger.c b/src/backend/commands/trigger.c index 2436692eb8..0665f110ba 100644 --- a/src/backend/commands/trigger.c +++ b/src/backend/commands/trigger.c @@ -3396,7 +3396,10 @@ 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; diff --git a/src/test/regress/expected/fast_default.out b/src/test/regress/expected/fast_default.out index f3d783c281..48bd360a79 100644 --- a/src/test/regress/expected/fast_default.out +++ b/src/test/regress/expected/fast_default.out @@ -539,8 +539,197 @@ FROM t1; 1 | 0 (20 rows) -DROP TABLE t1; DROP TABLE T; +-- test that we account for missing columns without defaults correctly +-- in expand_tuple, and that rows are correctly expanded for triggers +CREATE FUNCTION test_trigger() +RETURNS trigger +LANGUAGE plpgsql +AS $$ + +begin + raise notice 'old tuple: %', to_json(OLD)::text; + if TG_OP = 'DELETE' + then + return OLD; + else + return NEW; + end if; +end; + +$$; +-- 2 new columns, both have defaults +CREATE TABLE t (id serial PRIMARY KEY, a int, b int, c int); +INSERT INTO t (a,b,c) VALUES (1,2,3); +ALTER TABLE t ADD COLUMN x int NOT NULL DEFAULT 4; +ALTER TABLE t ADD COLUMN y int NOT NULL DEFAULT 5; +CREATE TRIGGER a BEFORE UPDATE ON t FOR EACH ROW EXECUTE PROCEDURE test_trigger(); +SELECT * FROM t; + id | a | b | c | x | y +----+---+---+---+---+--- + 1 | 1 | 2 | 3 | 4 | 5 +(1 row) + +UPDATE t SET y = 2; +NOTICE: old tuple: {"id":1,"a":1,"b":2,"c":3,"x":4,"y":5} +SELECT * FROM t; + id | a | b | c | x | y +----+---+---+---+---+--- + 1 | 1 | 2 | 3 | 4 | 2 +(1 row) + +DROP TABLE t; +-- 2 new columns, first has default +CREATE TABLE t (id serial PRIMARY KEY, a int, b int, c int); +INSERT INTO t (a,b,c) VALUES (1,2,3); +ALTER TABLE t ADD COLUMN x int NOT NULL DEFAULT 4; +ALTER TABLE t ADD COLUMN y int; +CREATE TRIGGER a BEFORE UPDATE ON t FOR EACH ROW EXECUTE PROCEDURE test_trigger(); +SELECT * FROM t; + id | a | b | c | x | y +----+---+---+---+---+--- + 1 | 1 | 2 | 3 | 4 | +(1 row) + +UPDATE t SET y = 2; +NOTICE: old tuple: {"id":1,"a":1,"b":2,"c":3,"x":4,"y":null} +SELECT * FROM t; + id | a | b | c | x | y +----+---+---+---+---+--- + 1 | 1 | 2 | 3 | 4 | 2 +(1 row) + +DROP TABLE t; +-- 2 new columns, second has default +CREATE TABLE t (id serial PRIMARY KEY, a int, b int, c int); +INSERT INTO t (a,b,c) VALUES (1,2,3); +ALTER TABLE t ADD COLUMN x int; +ALTER TABLE t ADD COLUMN y int NOT NULL DEFAULT 5; +CREATE TRIGGER a BEFORE UPDATE ON t FOR EACH ROW EXECUTE PROCEDURE test_trigger(); +SELECT * FROM t; + id | a | b | c | x | y +----+---+---+---+---+--- + 1 | 1 | 2 | 3 | | 5 +(1 row) + +UPDATE t SET y = 2; +NOTICE: old tuple: {"id":1,"a":1,"b":2,"c":3,"x":null,"y":5} +SELECT * FROM t; + id | a | b | c | x | y +----+---+---+---+---+--- + 1 | 1 | 2 | 3 | | 2 +(1 row) + +DROP TABLE t; +-- 2 new columns, neither has default +CREATE TABLE t (id serial PRIMARY KEY, a int, b int, c int); +INSERT INTO t (a,b,c) VALUES (1,2,3); +ALTER TABLE t ADD COLUMN x int; +ALTER TABLE t ADD COLUMN y int; +CREATE TRIGGER a BEFORE UPDATE ON t FOR EACH ROW EXECUTE PROCEDURE test_trigger(); +SELECT * FROM t; + id | a | b | c | x | y +----+---+---+---+---+--- + 1 | 1 | 2 | 3 | | +(1 row) + +UPDATE t SET y = 2; +NOTICE: old tuple: {"id":1,"a":1,"b":2,"c":3,"x":null,"y":null} +SELECT * FROM t; + id | a | b | c | x | y +----+---+---+---+---+--- + 1 | 1 | 2 | 3 | | 2 +(1 row) + +DROP TABLE t; +-- same as last 4 tests but here the last original column has a NULL value +-- 2 new columns, both have defaults +CREATE TABLE t (id serial PRIMARY KEY, a int, b int, c int); +INSERT INTO t (a,b,c) VALUES (1,2,NULL); +ALTER TABLE t ADD COLUMN x int NOT NULL DEFAULT 4; +ALTER TABLE t ADD COLUMN y int NOT NULL DEFAULT 5; +CREATE TRIGGER a BEFORE UPDATE ON t FOR EACH ROW EXECUTE PROCEDURE test_trigger(); +SELECT * FROM t; + id | a | b | c | x | y +----+---+---+---+---+--- + 1 | 1 | 2 | | 4 | 5 +(1 row) + +UPDATE t SET y = 2; +NOTICE: old tuple: {"id":1,"a":1,"b":2,"c":null,"x":4,"y":5} +SELECT * FROM t; + id | a | b | c | x | y +----+---+---+---+---+--- + 1 | 1 | 2 | | 4 | 2 +(1 row) + +DROP TABLE t; +-- 2 new columns, first has default +CREATE TABLE t (id serial PRIMARY KEY, a int, b int, c int); +INSERT INTO t (a,b,c) VALUES (1,2,NULL); +ALTER TABLE t ADD COLUMN x int NOT NULL DEFAULT 4; +ALTER TABLE t ADD COLUMN y int; +CREATE TRIGGER a BEFORE UPDATE ON t FOR EACH ROW EXECUTE PROCEDURE test_trigger(); +SELECT * FROM t; + id | a | b | c | x | y +----+---+---+---+---+--- + 1 | 1 | 2 | | 4 | +(1 row) + +UPDATE t SET y = 2; +NOTICE: old tuple: {"id":1,"a":1,"b":2,"c":null,"x":4,"y":null} +SELECT * FROM t; + id | a | b | c | x | y +----+---+---+---+---+--- + 1 | 1 | 2 | | 4 | 2 +(1 row) + +DROP TABLE t; +-- 2 new columns, second has default +CREATE TABLE t (id serial PRIMARY KEY, a int, b int, c int); +INSERT INTO t (a,b,c) VALUES (1,2,NULL); +ALTER TABLE t ADD COLUMN x int; +ALTER TABLE t ADD COLUMN y int NOT NULL DEFAULT 5; +CREATE TRIGGER a BEFORE UPDATE ON t FOR EACH ROW EXECUTE PROCEDURE test_trigger(); +SELECT * FROM t; + id | a | b | c | x | y +----+---+---+---+---+--- + 1 | 1 | 2 | | | 5 +(1 row) + +UPDATE t SET y = 2; +NOTICE: old tuple: {"id":1,"a":1,"b":2,"c":null,"x":null,"y":5} +SELECT * FROM t; + id | a | b | c | x | y +----+---+---+---+---+--- + 1 | 1 | 2 | | | 2 +(1 row) + +DROP TABLE t; +-- 2 new columns, neither has default +CREATE TABLE t (id serial PRIMARY KEY, a int, b int, c int); +INSERT INTO t (a,b,c) VALUES (1,2,NULL); +ALTER TABLE t ADD COLUMN x int; +ALTER TABLE t ADD COLUMN y int; +CREATE TRIGGER a BEFORE UPDATE ON t FOR EACH ROW EXECUTE PROCEDURE test_trigger(); +SELECT * FROM t; + id | a | b | c | x | y +----+---+---+---+---+--- + 1 | 1 | 2 | | | +(1 row) + +UPDATE t SET y = 2; +NOTICE: old tuple: {"id":1,"a":1,"b":2,"c":null,"x":null,"y":null} +SELECT * FROM t; + id | a | b | c | x | y +----+---+---+---+---+--- + 1 | 1 | 2 | | | 2 +(1 row) + +DROP TABLE t; +-- cleanup +DROP FUNCTION test_trigger(); +DROP TABLE t1; DROP FUNCTION set(name); DROP FUNCTION comp(); DROP TABLE m; diff --git a/src/test/regress/sql/fast_default.sql b/src/test/regress/sql/fast_default.sql index 7b9cc47cef..06205cb39f 100644 --- a/src/test/regress/sql/fast_default.sql +++ b/src/test/regress/sql/fast_default.sql @@ -360,8 +360,120 @@ SELECT a, AS z FROM t1; -DROP TABLE t1; DROP TABLE T; + +-- test that we account for missing columns without defaults correctly +-- in expand_tuple, and that rows are correctly expanded for triggers + +CREATE FUNCTION test_trigger() +RETURNS trigger +LANGUAGE plpgsql +AS $$ + +begin + raise notice 'old tuple: %', to_json(OLD)::text; + if TG_OP = 'DELETE' + then + return OLD; + else + return NEW; + end if; +end; + +$$; + +-- 2 new columns, both have defaults +CREATE TABLE t (id serial PRIMARY KEY, a int, b int, c int); +INSERT INTO t (a,b,c) VALUES (1,2,3); +ALTER TABLE t ADD COLUMN x int NOT NULL DEFAULT 4; +ALTER TABLE t ADD COLUMN y int NOT NULL DEFAULT 5; +CREATE TRIGGER a BEFORE UPDATE ON t FOR EACH ROW EXECUTE PROCEDURE test_trigger(); +SELECT * FROM t; +UPDATE t SET y = 2; +SELECT * FROM t; +DROP TABLE t; + +-- 2 new columns, first has default +CREATE TABLE t (id serial PRIMARY KEY, a int, b int, c int); +INSERT INTO t (a,b,c) VALUES (1,2,3); +ALTER TABLE t ADD COLUMN x int NOT NULL DEFAULT 4; +ALTER TABLE t ADD COLUMN y int; +CREATE TRIGGER a BEFORE UPDATE ON t FOR EACH ROW EXECUTE PROCEDURE test_trigger(); +SELECT * FROM t; +UPDATE t SET y = 2; +SELECT * FROM t; +DROP TABLE t; + +-- 2 new columns, second has default +CREATE TABLE t (id serial PRIMARY KEY, a int, b int, c int); +INSERT INTO t (a,b,c) VALUES (1,2,3); +ALTER TABLE t ADD COLUMN x int; +ALTER TABLE t ADD COLUMN y int NOT NULL DEFAULT 5; +CREATE TRIGGER a BEFORE UPDATE ON t FOR EACH ROW EXECUTE PROCEDURE test_trigger(); +SELECT * FROM t; +UPDATE t SET y = 2; +SELECT * FROM t; +DROP TABLE t; + +-- 2 new columns, neither has default +CREATE TABLE t (id serial PRIMARY KEY, a int, b int, c int); +INSERT INTO t (a,b,c) VALUES (1,2,3); +ALTER TABLE t ADD COLUMN x int; +ALTER TABLE t ADD COLUMN y int; +CREATE TRIGGER a BEFORE UPDATE ON t FOR EACH ROW EXECUTE PROCEDURE test_trigger(); +SELECT * FROM t; +UPDATE t SET y = 2; +SELECT * FROM t; +DROP TABLE t; + +-- same as last 4 tests but here the last original column has a NULL value +-- 2 new columns, both have defaults +CREATE TABLE t (id serial PRIMARY KEY, a int, b int, c int); +INSERT INTO t (a,b,c) VALUES (1,2,NULL); +ALTER TABLE t ADD COLUMN x int NOT NULL DEFAULT 4; +ALTER TABLE t ADD COLUMN y int NOT NULL DEFAULT 5; +CREATE TRIGGER a BEFORE UPDATE ON t FOR EACH ROW EXECUTE PROCEDURE test_trigger(); +SELECT * FROM t; +UPDATE t SET y = 2; +SELECT * FROM t; +DROP TABLE t; + +-- 2 new columns, first has default +CREATE TABLE t (id serial PRIMARY KEY, a int, b int, c int); +INSERT INTO t (a,b,c) VALUES (1,2,NULL); +ALTER TABLE t ADD COLUMN x int NOT NULL DEFAULT 4; +ALTER TABLE t ADD COLUMN y int; +CREATE TRIGGER a BEFORE UPDATE ON t FOR EACH ROW EXECUTE PROCEDURE test_trigger(); +SELECT * FROM t; +UPDATE t SET y = 2; +SELECT * FROM t; +DROP TABLE t; + +-- 2 new columns, second has default +CREATE TABLE t (id serial PRIMARY KEY, a int, b int, c int); +INSERT INTO t (a,b,c) VALUES (1,2,NULL); +ALTER TABLE t ADD COLUMN x int; +ALTER TABLE t ADD COLUMN y int NOT NULL DEFAULT 5; +CREATE TRIGGER a BEFORE UPDATE ON t FOR EACH ROW EXECUTE PROCEDURE test_trigger(); +SELECT * FROM t; +UPDATE t SET y = 2; +SELECT * FROM t; +DROP TABLE t; + +-- 2 new columns, neither has default +CREATE TABLE t (id serial PRIMARY KEY, a int, b int, c int); +INSERT INTO t (a,b,c) VALUES (1,2,NULL); +ALTER TABLE t ADD COLUMN x int; +ALTER TABLE t ADD COLUMN y int; +CREATE TRIGGER a BEFORE UPDATE ON t FOR EACH ROW EXECUTE PROCEDURE test_trigger(); +SELECT * FROM t; +UPDATE t SET y = 2; +SELECT * FROM t; +DROP TABLE t; + +-- cleanup +DROP FUNCTION test_trigger(); +DROP TABLE t1; DROP FUNCTION set(name); DROP FUNCTION comp(); DROP TABLE m;