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

Hi,

On 2019-10-10 16:51:23 -0400, Tom Lane wrote:
> Andres Freund <andres(at)anarazel(dot)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