force C locale for temp regression installations

Started by Andrew Dunstanabout 21 years ago12 messagespatches
Jump to latest
#1Andrew Dunstan
andrew@dunslane.net

The attached tiny patch forces C locale on the temp location built by
pg_regress - it fixes recent regression problems observed on Windows.

cheers

andrew

Attachments:

regresslocale.patchtext/x-patch; name=regresslocale.patchDownload+2-2
#2Tom Lane
tgl@sss.pgh.pa.us
In reply to: Andrew Dunstan (#1)
Re: force C locale for temp regression installations

Andrew Dunstan <andrew@dunslane.net> writes:

The attached tiny patch forces C locale on the temp location built by
pg_regress - it fixes recent regression problems observed on Windows.

I don't think this is really a good idea, because it makes it impossible
to run the tests any other way.

If you're speaking of the current buildfarm results from loris, I'm
unconvinced that that's a locale problem --- the opr_sanity results
in particular shouldn't be locale-sensitive.

regards, tom lane

#3Andrew Dunstan
andrew@dunslane.net
In reply to: Tom Lane (#2)
Re: force C locale for temp regression installations

Tom Lane wrote:

Andrew Dunstan <andrew@dunslane.net> writes:

The attached tiny patch forces C locale on the temp location built by
pg_regress - it fixes recent regression problems observed on Windows.

I don't think this is really a good idea, because it makes it impossible
to run the tests any other way.

If you're speaking of the current buildfarm results from loris, I'm
unconvinced that that's a locale problem --- the opr_sanity results
in particular shouldn't be locale-sensitive.

(through bleary dilated eyes)

But we aleady do it implicitly for everything but Windows now, with this:

unset LC_COLLATE LC_CTYPE LC_MONETARY LC_MESSAGES LC_NUMERIC LC_TIME
LC_ALL LANG LANGUAGE

Both Petr and I have seen that with --no-locale set the regression set
gets a clean run. If you want us to dig deeper into those 9 failures,
just tell us what to look for.

cheers

andrew

#4Petr Jelinek
petr@2ndquadrant.com
In reply to: Tom Lane (#2)
Re: force C locale for temp regression installations

Tom Lane weote:

If you're speaking of the current buildfarm results from loris, I'm
unconvinced that that's a locale problem --- the opr_sanity results
in particular shouldn't be locale-sensitive.

It is locale problem, I have 9 failures (like loris) with my locale and
0 failures with --no-locale

Output from make cheks are at http://pjmodos.parba.cz/pgsql/locale/ and
http://pjmodos.parba.cz/pgsql/no-locale/
(it's same as loris, I posted it just as evidence that --no-locale fixes
all failures)

--
Regards
Petr Jelinek (PJMODOS)

#5Tom Lane
tgl@sss.pgh.pa.us
In reply to: Andrew Dunstan (#3)
Re: force C locale for temp regression installations

Andrew Dunstan <andrew@dunslane.net> writes:

Both Petr and I have seen that with --no-locale set the regression set
gets a clean run. If you want us to dig deeper into those 9 failures,
just tell us what to look for.

Well, for example, the first diff in the opr_sanity test appears to
indicate that 'abstime_timestamptz'::text is equal to
'abstime_timestamp'::text (because it is claiming to have joined two
pg_proc rows with those prosrc values). I don't care *what* locale
you're using, that is a wrong answer.

There are some pretty bogus-looking results in the union test, as well,
implying that it thinks 'abcd' is not equal to 'abcd' for instance.

And why did it suddenly start failing today? My bet is those UTF8/UTF16
routines we put in are a few bricks shy of a load yet, and that
--no-locale "fixes" it by preventing those routines from being used.

regards, tom lane

#6Andrew Dunstan
andrew@dunslane.net
In reply to: Tom Lane (#5)
Re: force C locale for temp regression installations

Tom Lane said:

Andrew Dunstan <andrew@dunslane.net> writes:

You mean the changes in varlena.c? Should I try reverting it and see
what happens?

Sure. If we'd been seeing these failures all along, I might think that
--no-locale was a good solution, but since they just popped up I think
suspicion has to fall on the latest patches.

That's it alright. When I change the ifndef in varlena.c to #ifndef xxxWIN32
the regression tests sail through.

cheers

andrew

#7Tom Lane
tgl@sss.pgh.pa.us
In reply to: Petr Jelinek (#4)
Re: force C locale for temp regression installations

Petr Jelinek <pjmodos@seznam.cz> writes:

(it's same as loris, I posted it just as evidence that --no-locale fixes
all failures)

I think --no-locale is masking a problem with the recently-installed
UTF16-based comparison code. See my response to Andrew just now.

regards, tom lane

#8Andrew Dunstan
andrew@dunslane.net
In reply to: Tom Lane (#5)
Re: force C locale for temp regression installations

Tom Lane wrote:

And why did it suddenly start failing today? My bet is those UTF8/UTF16
routines we put in are a few bricks shy of a load yet, and that
--no-locale "fixes" it by preventing those routines from being used.

You mean the changes in varlena.c? Should I try reverting it and see
what happens?

cheers

andrew

#9Tom Lane
tgl@sss.pgh.pa.us
In reply to: Andrew Dunstan (#8)
Re: force C locale for temp regression installations

Andrew Dunstan <andrew@dunslane.net> writes:

You mean the changes in varlena.c? Should I try reverting it and see
what happens?

Sure. If we'd been seeing these failures all along, I might think that
--no-locale was a good solution, but since they just popped up I think
suspicion has to fall on the latest patches.

regards, tom lane

#10Tom Lane
tgl@sss.pgh.pa.us
In reply to: Andrew Dunstan (#6)
Re: force C locale for temp regression installations

"Andrew Dunstan" <andrew@dunslane.net> writes:

Tom Lane said:

Sure. If we'd been seeing these failures all along, I might think that
--no-locale was a good solution, but since they just popped up I think
suspicion has to fall on the latest patches.

That's it alright. When I change the ifndef in varlena.c to #ifndef xxxWIN32
the regression tests sail through.

OK, so what is your default locale on that machine? In particular, is
it UTF16-based?

Now that I look at it, the strncoll code is utterly obviously broken :-(

regards, tom lane

#11Andrew Dunstan
andrew@dunslane.net
In reply to: Tom Lane (#10)
Re: force C locale for temp regression installations

Tom Lane wrote:

That's it alright. When I change the ifndef in varlena.c to #ifndef xxxWIN32
the regression tests sail through.

OK, so what is your default locale on that machine? In particular, is
it UTF16-based?

initdb picks up: "English_United States.1252"

cheers

andrew

#12Tom Lane
tgl@sss.pgh.pa.us
In reply to: Andrew Dunstan (#11)
Re: force C locale for temp regression installations

Andrew Dunstan <andrew@dunslane.net> writes:

Tom Lane wrote:

OK, so what is your default locale on that machine? In particular, is
it UTF16-based?

initdb picks up: "English_United States.1252"

Yup. So it was trying to use this little hack:

/* Win32 has strncoll(), so use it to avoid copying */
return _strncoll(arg1, arg2, Min(len1, len2));

which does not actually do what's desired, if the strings are of
different length. That's what we get for focusing on the hard case
and not testing the easy case ;-)

I've committed a fix.

regards, tom lane