pg_upgrade tests vs alter generic changes

The recent alter generic tests have broken pg_upgrade testing on Windows
and probably other non-collation-supporting platforms. This can be cured
by making the pg_upgrade test set up its test installs with "initdb
--no-locale." as shown below (with similar changes for the MSVC build
system also being needed)

Is there any reason we don't want to do that, or to restrict it to just
those platforms?

cheers

andrew

diff --git a/contrib/pg_upgrade/test.sh b/contrib/pg_upgrade/test.sh
index 32fb6bf..96da6f1 100644
--- a/contrib/pg_upgrade/test.sh
+++ b/contrib/pg_upgrade/test.sh
@@ -64,7 +64,7 @@ mkdir "$logdir"

set -x

-$oldbindir/initdb -N
+$oldbindir/initdb -N --no-locale
$oldbindir/pg_ctl start -l "$logdir/postmaster1.log" -o '-F' -w
if "$MAKE" -C "$oldsrc" installcheck; then
pg_dumpall -f "$temp_root"/dump1.sql || pg_dumpall1_status=$?
@@ -104,7 +104,7 @@ fi

mv "${PGDATA}" "${PGDATA}.old"

-initdb -N
+initdb -N --no-locale

pg_upgrade -d "${PGDATA}.old" -D "${PGDATA}" -b "$oldbindir" -B "$bindir"

Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
> The recent alter generic tests have broken pg_upgrade testing on Windows
> and probably other non-collation-supporting platforms.

Is it still broken after Alvaro added the alternate expected file, and
if so why? I don't see a reason that this should be failing only there.
I also note that it seems to be passing fine on buildfarm members other
than pitta.

> This can be cured
> by making the pg_upgrade test set up its test installs with "initdb
> --no-locale." as shown below (with similar changes for the MSVC build
> system also being needed)

That seems like a kluge, not a solution. There's no reason I can see
why pg_upgrade's repeat of the regression tests should be more sensitive
to locale environment than the main run is.

regards, tom lane

On 09/29/2012 11:49 AM, Tom Lane wrote:
> Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
>> The recent alter generic tests have broken pg_upgrade testing on Windows
>> and probably other non-collation-supporting platforms.
> Is it still broken after Alvaro added the alternate expected file, and
> if so why? I don't see a reason that this should be failing only there.
> I also note that it seems to be passing fine on buildfarm members other
> than pitta.

Well, that's a very good point. chough is actually the same machine,
doing an MSVC build. So why would this test pass there? I'll investigate
a bit more. Here's what the regression diffs look like when run from
pg_upgrade on pitta:

cheers

andrew

***
c:/MinGW/msys/1.0/home/pgrunner/bf/root/HEAD/pgsql.7132/../pgsql/src/test/regress/expected/alter_generic_1.out
Sat Sep 29 02:01:44 2012
---
c:/MinGW/msys/1.0/home/pgrunner/bf/root/HEAD/pgsql.7132/src/test/regress/results/alter_generic.out
Sat Sep 29 13:41:39 2012
***************
*** 110,137 ****
-- can't test this: the error message includes the encoding name
-- ALTER COLLATION alt_coll1 RENAME TO alt_coll2; -- failed (name
conflict)
ALTER COLLATION alt_coll1 RENAME TO alt_coll3; -- OK
! ERROR: collation "alt_coll1" for encoding "SQL_ASCII" does not exist
ALTER COLLATION alt_coll2 OWNER TO regtest_alter_user2; -- failed
(no role membership)
! ERROR: collation "alt_coll2" for encoding "SQL_ASCII" does not exist
ALTER COLLATION alt_coll2 OWNER TO regtest_alter_user3; -- OK
! ERROR: collation "alt_coll2" for encoding "SQL_ASCII" does not exist
ALTER COLLATION alt_coll2 SET SCHEMA alt_nsp2; -- OK
! ERROR: collation "alt_coll2" for encoding "SQL_ASCII" does not exist
SET SESSION AUTHORIZATION regtest_alter_user2;
CREATE COLLATION alt_coll1 (locale = 'C');
ERROR: nondefault collations are not supported on this platform
CREATE COLLATION alt_coll2 (locale = 'C');
ERROR: nondefault collations are not supported on this platform
ALTER COLLATION alt_coll3 RENAME TO alt_coll4; -- failed (not owner)
! ERROR: collation "alt_coll3" for encoding "SQL_ASCII" does not exist
ALTER COLLATION alt_coll1 RENAME TO alt_coll4; -- OK
! ERROR: collation "alt_coll1" for encoding "SQL_ASCII" does not exist
ALTER COLLATION alt_coll3 OWNER TO regtest_alter_user2; -- failed
(not owner)
! ERROR: collation "alt_coll3" for encoding "SQL_ASCII" does not exist
ALTER COLLATION alt_coll2 OWNER TO regtest_alter_user3; -- failed
(no role membership)
! ERROR: collation "alt_coll2" for encoding "SQL_ASCII" does not exist
ALTER COLLATION alt_coll3 SET SCHEMA alt_nsp2; -- failed (not owner)
! ERROR: collation "alt_coll3" for encoding "SQL_ASCII" does not exist
-- can't test this: the error message includes the encoding name
-- ALTER COLLATION alt_coll2 SET SCHEMA alt_nsp2; -- failed (name
conflict)
RESET SESSION AUTHORIZATION;
--- 110,137 ----
-- can't test this: the error message includes the encoding name
-- ALTER COLLATION alt_coll1 RENAME TO alt_coll2; -- failed (name
conflict)
ALTER COLLATION alt_coll1 RENAME TO alt_coll3; -- OK
! ERROR: collation "alt_coll1" for encoding "WIN1252" does not exist
ALTER COLLATION alt_coll2 OWNER TO regtest_alter_user2; -- failed
(no role membership)
! ERROR: collation "alt_coll2" for encoding "WIN1252" does not exist
ALTER COLLATION alt_coll2 OWNER TO regtest_alter_user3; -- OK
! ERROR: collation "alt_coll2" for encoding "WIN1252" does not exist
ALTER COLLATION alt_coll2 SET SCHEMA alt_nsp2; -- OK
! ERROR: collation "alt_coll2" for encoding "WIN1252" does not exist
SET SESSION AUTHORIZATION regtest_alter_user2;
CREATE COLLATION alt_coll1 (locale = 'C');
ERROR: nondefault collations are not supported on this platform
CREATE COLLATION alt_coll2 (locale = 'C');
ERROR: nondefault collations are not supported on this platform
ALTER COLLATION alt_coll3 RENAME TO alt_coll4; -- failed (not owner)
! ERROR: collation "alt_coll3" for encoding "WIN1252" does not exist
ALTER COLLATION alt_coll1 RENAME TO alt_coll4; -- OK
! ERROR: collation "alt_coll1" for encoding "WIN1252" does not exist
ALTER COLLATION alt_coll3 OWNER TO regtest_alter_user2; -- failed
(not owner)
! ERROR: collation "alt_coll3" for encoding "WIN1252" does not exist
ALTER COLLATION alt_coll2 OWNER TO regtest_alter_user3; -- failed
(no role membership)
! ERROR: collation "alt_coll2" for encoding "WIN1252" does not exist
ALTER COLLATION alt_coll3 SET SCHEMA alt_nsp2; -- failed (not owner)
! ERROR: collation "alt_coll3" for encoding "WIN1252" does not exist
-- can't test this: the error message includes the encoding name
-- ALTER COLLATION alt_coll2 SET SCHEMA alt_nsp2; -- failed (name
conflict)
RESET SESSION AUTHORIZATION;

======================================================================

Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
> On 09/29/2012 11:49 AM, Tom Lane wrote:
>> Is it still broken after Alvaro added the alternate expected file, and
>> if so why? I don't see a reason that this should be failing only there.
>> I also note that it seems to be passing fine on buildfarm members other
>> than pitta.

> Well, that's a very good point. chough is actually the same machine,
> doing an MSVC build. So why would this test pass there? I'll investigate
> a bit more. Here's what the regression diffs look like when run from
> pg_upgrade on pitta:

> ALTER COLLATION alt_coll1 RENAME TO alt_coll3; -- OK
> ! ERROR: collation "alt_coll1" for encoding "SQL_ASCII" does not exist

vs

> ALTER COLLATION alt_coll1 RENAME TO alt_coll3; -- OK
> ! ERROR: collation "alt_coll1" for encoding "WIN1252" does not exist

Oh! So Alvaro's second expected file is assuming that machines without
custom-locale support will only ever be testing with SQL_ASCII encoding.
Wrong.

At this point I'm inclined to think that we should just drop the
collation-specific portions of the alter_generic test. It looks to me
like making that adequately portable is going to be far more trouble
than it's worth.

regards, tom lane

Excerpts from Tom Lane's message of sáb sep 29 14:57:11 -0300 2012:
>
> Andrew Dunstan <andrew(at)dunslane(dot)net> writes:

> > Well, that's a very good point. chough is actually the same machine,
> > doing an MSVC build. So why would this test pass there? I'll investigate
> > a bit more. Here's what the regression diffs look like when run from
> > pg_upgrade on pitta:
>
> > ALTER COLLATION alt_coll1 RENAME TO alt_coll3; -- OK
> > ! ERROR: collation "alt_coll1" for encoding "SQL_ASCII" does not exist
>
> vs
>
> > ALTER COLLATION alt_coll1 RENAME TO alt_coll3; -- OK
> > ! ERROR: collation "alt_coll1" for encoding "WIN1252" does not exist
>
> Oh! So Alvaro's second expected file is assuming that machines without
> custom-locale support will only ever be testing with SQL_ASCII encoding.
> Wrong.
>
> At this point I'm inclined to think that we should just drop the
> collation-specific portions of the alter_generic test. It looks to me
> like making that adequately portable is going to be far more trouble
> than it's worth.

Ah, yes. We already dropped some plperl tests because of a similar
problem. I will remove that part of the test.

--
Álvaro Herrera http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Training & Services

On 09/29/2012 01:49 PM, Andrew Dunstan wrote:
>
> On 09/29/2012 11:49 AM, Tom Lane wrote:
>> Andrew Dunstan <andrew(at)dunslane(dot)net> writes:
>>> The recent alter generic tests have broken pg_upgrade testing on
>>> Windows
>>> and probably other non-collation-supporting platforms.
>> Is it still broken after Alvaro added the alternate expected file, and
>> if so why? I don't see a reason that this should be failing only there.
>> I also note that it seems to be passing fine on buildfarm members other
>> than pitta.
>
> Well, that's a very good point. chough is actually the same machine,
> doing an MSVC build. So why would this test pass there? I'll
> investigate a bit more. Here's what the regression diffs look like
> when run from pg_upgrade on pitta:

[hours of digging later]

It turns out that the reason is that we support collations on MSVC but
not on Mingw.

*sigh*

I'll have to put it on my ever lengthening TODO list.

cheers

andrew

On Sat, 2012-09-29 at 17:52 -0400, Andrew Dunstan wrote:
> It turns out that the reason is that we support collations on MSVC but
> not on Mingw.

The cause for that is that on Windows locale_t is called _locale_t, and
there is a workaround for that in src/include/port/win32.h for the MSVC
build, but with MinGW, the configure test result still says that
locale_t does not exist, so the code is omitted.

This could be fixed either by making the configure test a bit smarter,
or just by sticking a

#define HAVE_LOCALE_T 1

into the aforementioned win32.h.