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

diff --git a/src/backend/commands/tablecmds.c b/src/backend/commands/tablecmds.c
index 05593f331624650a1886187a08157bb81bf0aa20..ba8f4459f3d5ca1a9606ddd1309db84b0d1f1e4b 100644 (file)
--- 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 23d4265555cda7b2a9a28e2821ae3ce9bc7a2052..41a7bcd6d0d2defa01276ee2bde9b667a0f66b6b 100644 (file)
--- 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 99af0b851b3f985cb27278e53f49151dcf2d6a04..3fde96b2b9039ce101ae9c48a5753b17e28ca2c1 100644 (file)
--- 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
 --