On Thu, Oct 10, 2019 at 05:16:08PM -0400, Tom Lane wrote: > I really really don't want an event trigger running in everything that > runs in parallel with alter_table. That's a debugging nightmare of > monster proportions.
+1, that's not stable. elver has just complained about a side effect of this commit: https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=elver&dt=2019-10-16%2005%3A04%3A24 -- now change 'name' to an integer to see what happens... ALTER TABLE alter_table_under_transition_tables ALTER COLUMN name TYPE int USING name::integer; +WARNING: rewriting table alter_table_under_transition_tables UPDATE alter_table_under_transition_tables SET name = (name::text || name::text)::integer; WARNING: old table = 1=11,2=22,3=33, new table = 1=1111,2=2222,3=3333 Another method would be to simply store the past relfilenodes into a temporary table with the relation OID, and do a comparison with what's in pg_class post-rewrite as some partition-related tests do in the same file. For one relation the \gset method is more readable anyway. If you really wish to keep an event trigger, you could also apply a filter within the function to only consider rewrite_test as something to report about, still I would recommend avoiding global effects if not necessary. So I would fix the issue as per the attached, as Tom has suggested. Thoughts? -- Michael
diff --git a/src/test/regress/expected/alter_table.out b/src/test/regress/expected/alter_table.out
index 41a7bcd6d0..b041247476 100644
--- a/src/test/regress/expected/alter_table.out
+++ b/src/test/regress/expected/alter_table.out
@@ -2437,51 +2437,96 @@ 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();
+-- a column requiring a default (bug #16038). This checks
+-- pg_class.relfilenode before and after the rewrite for differences
+-- to see if a rewrite has been done or not.
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
+SELECT relfilenode AS oldfilenode FROM pg_class
+ WHERE relname = 'rewrite_test'
+ \gset
ALTER TABLE rewrite_test
ADD COLUMN empty1 text,
ADD COLUMN notempty1_rewrite serial;
-WARNING: rewriting table rewrite_test
+SELECT relfilenode != :oldfilenode AS changed FROM pg_class
+ WHERE relname = 'rewrite_test';
+ changed
+---------
+ t
+(1 row)
+
+SELECT relfilenode AS oldfilenode FROM pg_class
+ WHERE relname = 'rewrite_test'
+ \gset
ALTER TABLE rewrite_test
ADD COLUMN notempty2_rewrite serial,
ADD COLUMN empty2 text;
-WARNING: rewriting table rewrite_test
+SELECT relfilenode != :oldfilenode AS changed FROM pg_class
+ WHERE relname = 'rewrite_test';
+ changed
+---------
+ t
+(1 row)
+
-- also check that fast defaults cause no problem, first without rewrite
+SELECT relfilenode AS oldfilenode FROM pg_class
+ WHERE relname = 'rewrite_test'
+ \gset
ALTER TABLE rewrite_test
ADD COLUMN empty3 text,
ADD COLUMN notempty3_norewrite int default 42;
+SELECT relfilenode != :oldfilenode AS changed FROM pg_class
+ WHERE relname = 'rewrite_test';
+ changed
+---------
+ f
+(1 row)
+
+SELECT relfilenode AS oldfilenode FROM pg_class
+ WHERE relname = 'rewrite_test'
+ \gset
ALTER TABLE rewrite_test
ADD COLUMN notempty4_norewrite int default 42,
ADD COLUMN empty4 text;
+SELECT relfilenode != :oldfilenode AS changed FROM pg_class
+ WHERE relname = 'rewrite_test';
+ changed
+---------
+ f
+(1 row)
+
-- then with rewrite
+SELECT relfilenode AS oldfilenode FROM pg_class
+ WHERE relname = 'rewrite_test'
+ \gset
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
+SELECT relfilenode != :oldfilenode AS changed FROM pg_class
+ WHERE relname = 'rewrite_test';
+ changed
+---------
+ t
+(1 row)
+
+SELECT relfilenode AS oldfilenode FROM pg_class
+ WHERE relname = 'rewrite_test'
+ \gset
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
+SELECT relfilenode != :oldfilenode AS changed FROM pg_class
+ WHERE relname = 'rewrite_test';
+ changed
+---------
+ t
+(1 row)
+
-- 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 3fde96b2b9..67672008ff 100644
--- a/src/test/regress/sql/alter_table.sql
+++ b/src/test/regress/sql/alter_table.sql
@@ -1551,51 +1551,71 @@ 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();
-
+-- a column requiring a default (bug #16038). This checks
+-- pg_class.relfilenode before and after the rewrite for differences
+-- to see if a rewrite has been done or not.
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
+SELECT relfilenode AS oldfilenode FROM pg_class
+ WHERE relname = 'rewrite_test'
+ \gset
ALTER TABLE rewrite_test
ADD COLUMN empty1 text,
ADD COLUMN notempty1_rewrite serial;
+SELECT relfilenode != :oldfilenode AS changed FROM pg_class
+ WHERE relname = 'rewrite_test';
+
+SELECT relfilenode AS oldfilenode FROM pg_class
+ WHERE relname = 'rewrite_test'
+ \gset
ALTER TABLE rewrite_test
ADD COLUMN notempty2_rewrite serial,
ADD COLUMN empty2 text;
+SELECT relfilenode != :oldfilenode AS changed FROM pg_class
+ WHERE relname = 'rewrite_test';
+
-- also check that fast defaults cause no problem, first without rewrite
+SELECT relfilenode AS oldfilenode FROM pg_class
+ WHERE relname = 'rewrite_test'
+ \gset
ALTER TABLE rewrite_test
ADD COLUMN empty3 text,
ADD COLUMN notempty3_norewrite int default 42;
+SELECT relfilenode != :oldfilenode AS changed FROM pg_class
+ WHERE relname = 'rewrite_test';
+SELECT relfilenode AS oldfilenode FROM pg_class
+ WHERE relname = 'rewrite_test'
+ \gset
ALTER TABLE rewrite_test
ADD COLUMN notempty4_norewrite int default 42,
ADD COLUMN empty4 text;
+SELECT relfilenode != :oldfilenode AS changed FROM pg_class
+ WHERE relname = 'rewrite_test';
+
-- then with rewrite
+SELECT relfilenode AS oldfilenode FROM pg_class
+ WHERE relname = 'rewrite_test'
+ \gset
ALTER TABLE rewrite_test
ADD COLUMN empty5 text,
ADD COLUMN notempty5_norewrite int default 42,
ADD COLUMN notempty5_rewrite serial;
+SELECT relfilenode != :oldfilenode AS changed FROM pg_class
+ WHERE relname = 'rewrite_test';
+SELECT relfilenode AS oldfilenode FROM pg_class
+ WHERE relname = 'rewrite_test'
+ \gset
ALTER TABLE rewrite_test
ADD COLUMN notempty6_rewrite serial,
ADD COLUMN empty6 text,
ADD COLUMN notempty6_norewrite int default 42;
+SELECT relfilenode != :oldfilenode AS changed FROM pg_class
+ WHERE relname = 'rewrite_test';
-- cleanup
-drop event trigger evtrig_rewrite_log;
-drop function evtrig_rewrite_log();
DROP TABLE rewrite_test;
--
signature.asc
Description: PGP signature
