From 93765bd956bea26206043de8cbb0ae4b67e4df15 Mon Sep 17 00:00:00 2001 From: Andres Freund Date: Wed, 9 Oct 2019 22:00:50 -0700 Subject: [PATCH] Fix table rewrites that include a column without a default. In c2fe139c201c I made ATRewriteTable() use tuple slots. Unfortunately I did not notice that columns can be added in a rewrite that do not have a default, when another column is added/altered requiring one. Initialize columns to NULL again, and add tests. Bug: #16038 Reported-By: anonymous Author: Andres Freund Discussion: https://postgr.es/m/16038-5c974541f2bf6749@postgresql.org Backpatch: 12, where the bug was introduced in c2fe139c201c --- src/backend/commands/tablecmds.c | 10 +++++ src/test/regress/expected/alter_table.out | 47 ++++++++++++++++++++++ src/test/regress/sql/alter_table.sql | 48 +++++++++++++++++++++++ 3 files changed, 105 insertions(+) diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c index 05593f3316..ba8f4459f3 100644 --- a/src/backend/commands/tablecmds.c +++ b/src/backend/commands/tablecmds.c @@ -4890,6 +4890,16 @@ ATRewriteTable(AlteredTableInfo *tab, Oid OIDNewHeap, LOCKMODE lockmode) table_slot_callbacks(oldrel)); newslot = MakeSingleTupleTableSlot(newTupDesc, table_slot_callbacks(newrel)); + + /* + * Set all columns in the new slot to NULL initially, to ensure + * columns added as part of the rewrite are initialized to + * NULL. That is necessary as tab->newvals will not contain an + * expression for columns with a NULL default, e.g. when adding a + * column without a default together with a column with a default + * requiring an actual rewrite. + */ + ExecStoreAllNullTuple(newslot); } else { diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out index 23d4265555..41a7bcd6d0 100644 --- a/src/test/regress/expected/alter_table.out +++ b/src/test/regress/expected/alter_table.out @@ -2436,6 +2436,53 @@ select * from at_view_2; drop view at_view_2; drop view at_view_1; drop table at_base_table; +-- check adding a column not iself requiring a rewrite, together with +-- a column requiring a default (bug #16038) +-- ensure that rewrites aren't silently optimized away, removing the +-- value of the test +CREATE OR REPLACE FUNCTION evtrig_rewrite_log() RETURNS event_trigger +LANGUAGE plpgsql AS $$ +BEGIN + RAISE WARNING 'rewriting table %', + pg_event_trigger_table_rewrite_oid()::regclass; +END; +$$; +CREATE EVENT TRIGGER evtrig_rewrite_log ON table_rewrite + EXECUTE PROCEDURE evtrig_rewrite_log(); +CREATE TABLE rewrite_test(col text); +INSERT INTO rewrite_test VALUES ('something'); +INSERT INTO rewrite_test VALUES (NULL); +-- empty[12] doesn't need rewrite, but notempty[12]_rewrite will force one +ALTER TABLE rewrite_test + ADD COLUMN empty1 text, + ADD COLUMN notempty1_rewrite serial; +WARNING: rewriting table rewrite_test +ALTER TABLE rewrite_test + ADD COLUMN notempty2_rewrite serial, + ADD COLUMN empty2 text; +WARNING: rewriting table rewrite_test +-- also check that fast defaults cause no problem, first without rewrite +ALTER TABLE rewrite_test + ADD COLUMN empty3 text, + ADD COLUMN notempty3_norewrite int default 42; +ALTER TABLE rewrite_test + ADD COLUMN notempty4_norewrite int default 42, + ADD COLUMN empty4 text; +-- then with rewrite +ALTER TABLE rewrite_test + ADD COLUMN empty5 text, + ADD COLUMN notempty5_norewrite int default 42, + ADD COLUMN notempty5_rewrite serial; +WARNING: rewriting table rewrite_test +ALTER TABLE rewrite_test + ADD COLUMN notempty6_rewrite serial, + ADD COLUMN empty6 text, + ADD COLUMN notempty6_norewrite int default 42; +WARNING: rewriting table rewrite_test +-- cleanup +drop event trigger evtrig_rewrite_log; +drop function evtrig_rewrite_log(); +DROP TABLE rewrite_test; -- -- lock levels -- diff --git a/src/test/regress/sql/alter_table.sql b/src/test/regress/sql/alter_table.sql index 99af0b851b..3fde96b2b9 100644 --- a/src/test/regress/sql/alter_table.sql +++ b/src/test/regress/sql/alter_table.sql @@ -1550,6 +1550,54 @@ drop view at_view_2; drop view at_view_1; drop table at_base_table; +-- check adding a column not iself requiring a rewrite, together with +-- a column requiring a default (bug #16038) + +-- ensure that rewrites aren't silently optimized away, removing the +-- value of the test +CREATE OR REPLACE FUNCTION evtrig_rewrite_log() RETURNS event_trigger +LANGUAGE plpgsql AS $$ +BEGIN + RAISE WARNING 'rewriting table %', + pg_event_trigger_table_rewrite_oid()::regclass; +END; +$$; +CREATE EVENT TRIGGER evtrig_rewrite_log ON table_rewrite + EXECUTE PROCEDURE evtrig_rewrite_log(); + +CREATE TABLE rewrite_test(col text); +INSERT INTO rewrite_test VALUES ('something'); +INSERT INTO rewrite_test VALUES (NULL); + +-- empty[12] doesn't need rewrite, but notempty[12]_rewrite will force one +ALTER TABLE rewrite_test + ADD COLUMN empty1 text, + ADD COLUMN notempty1_rewrite serial; +ALTER TABLE rewrite_test + ADD COLUMN notempty2_rewrite serial, + ADD COLUMN empty2 text; +-- also check that fast defaults cause no problem, first without rewrite +ALTER TABLE rewrite_test + ADD COLUMN empty3 text, + ADD COLUMN notempty3_norewrite int default 42; +ALTER TABLE rewrite_test + ADD COLUMN notempty4_norewrite int default 42, + ADD COLUMN empty4 text; +-- then with rewrite +ALTER TABLE rewrite_test + ADD COLUMN empty5 text, + ADD COLUMN notempty5_norewrite int default 42, + ADD COLUMN notempty5_rewrite serial; +ALTER TABLE rewrite_test + ADD COLUMN notempty6_rewrite serial, + ADD COLUMN empty6 text, + ADD COLUMN notempty6_norewrite int default 42; + +-- cleanup +drop event trigger evtrig_rewrite_log; +drop function evtrig_rewrite_log(); +DROP TABLE rewrite_test; + -- -- lock levels --