From: Andres Freund <andres@anarazel.de>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: pgsql-committers@lists.postgresql.org
Subject: Re: pgsql: Fix table rewrites that include a column without a default.
Date: Thu, 10 Oct 2019 13:59:03 -0700
Message-ID: <20191010205903.tj73q3qcyfft3izz@alap3.anarazel.de> (raw)
In-Reply-To: <3328.1570740683@sss.pgh.pa.us>
References: <E1iIQrV-0005gk-1D@gemulon.postgresql.org>
<3328.1570740683@sss.pgh.pa.us>
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
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Reply to all the recipients using the --to and --cc options:
reply via email
To: pgsql-committers@postgresql.org
Cc: andres@anarazel.de, tgl@sss.pgh.pa.us, pgsql-committers@lists.postgresql.org
Subject: Re: pgsql: Fix table rewrites that include a column without a default.
In-Reply-To: <20191010205903.tj73q3qcyfft3izz@alap3.anarazel.de>
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
This inbox is served by DDX for PostgreSQL; see mirroring instructions
for how to clone and mirror all data and code used for this inbox