Re: pgsql: Fix table rewrites that include a column without adefault.

Hi,

On 2019-10-10 16:51:23 -0400, Tom Lane wrote:
> Andres Freund <andres@anarazel.de> writes:
> > Fix table rewrites that include a column without a default.
> 
> This patch added use of an event trigger in alter_table.sql.
> As we have learned the hard way, it's not acceptable to create event
> triggers in test scripts that run in parallel with anything else,
> because they will intermittently screw up DDL happening in the
> parallel scripts.  As seen just now on clam:

Dang :(


> https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=clam&dt=2019-10-10%2020%3A16%3A19
> 
> diff -U3 /home/pgbfarm/buildroot/HEAD/pgsql.build/src/test/regress/expected/plpgsql.out
/home/pgbfarm/buildroot/HEAD/pgsql.build/src/test/regress/results/plpgsql.out
> --- /home/pgbfarm/buildroot/HEAD/pgsql.build/src/test/regress/expected/plpgsql.out    2019-05-09 09:00:10.653795677
+0100
> +++ /home/pgbfarm/buildroot/HEAD/pgsql.build/src/test/regress/results/plpgsql.out    2019-10-10 21:21:46.406007734
+0100
> @@ -5419,6 +5419,7 @@
>  -- 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
> 
> 
> Do you *really* need an event trigger to test this?

No. I added it to avoid the tests getting "accidentally" optimized away,
the next time somebody adds some ALTER TABLE optimizations. The fast
default stuff had removed coverage from a test that'd have uncovered
this bug. And an event trigger seemed to be the easiest way to achieve
that.

The only real alternative I can see to just removing the trigger is
embedding a check in the trigger that will cause it to only work in the
current session.  Something like SET regress.log_table_rewrites, and a
IF current_setting('regress.log_table_rewrites')::bool in the function.


Greetings,

Andres Freund