Prev: Hot Standby 0.2.1
Next: Rejecting weak passwords
From: Tom Lane on 29 Sep 2009 18:42 Alvaro Herrera <alvherre(a)commandprompt.com> writes: > Robert Haas escribi�: >> This seems to mean that we can't apply this patch, since failing the >> regression tests is not an acceptable behavior. > Does the patch pass regression tests in normal conditions? If you consider that "normal" means "LANG=C in environment", then yeah. Else not so much. In particular, every one of the buildfarm machines that tests a non-C locale environment can be expected to go red. I think that the required patch might not be a whole lot larger than the one Brad already submitted, so I'm willing to allow a couple more days for the author to investigate the issue. regards, tom lane -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Andrew Dunstan on 29 Sep 2009 18:45 Alvaro Herrera wrote: > Does the patch pass regression tests in normal conditions? If it does, > I see no reason to reject it. If it fails in --locale only, and even > then only when the given locale is UTF8, which IIRC it's a seldom-used > case, we can see about fixing that separately. > > Numerous buildfarm machines (including one of mine) run the regression suite using UTF8. So it's far from seldom-used. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Alvaro Herrera on 29 Sep 2009 18:50 Tom Lane escribió: > Alvaro Herrera <alvherre(a)commandprompt.com> writes: > > Robert Haas escribi�: > >> This seems to mean that we can't apply this patch, since failing the > >> regression tests is not an acceptable behavior. > > > Does the patch pass regression tests in normal conditions? > > If you consider that "normal" means "LANG=C in environment", then yeah. > Else not so much. In particular, every one of the buildfarm machines > that tests a non-C locale environment can be expected to go red. Huh, you're right, it doesn't work at all. -- Alvaro Herrera http://www.CommandPrompt.com/ The PostgreSQL Company - Command Prompt, Inc. -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Roger Leigh on 30 Sep 2009 10:41 On Tue, Sep 29, 2009 at 04:28:57PM -0400, Tom Lane wrote: > Peter Eisentraut <peter_e(a)gmx.net> writes: > > On Tue, 2009-09-29 at 12:01 -0400, Tom Lane wrote: > >> The bigger question is exactly how we expect this stuff to interact with > >> pg_regress' --no-locale switch. We already do clear all these variables > >> when --no-locale is specified. I am wondering just what --locale is > >> supposed to do, and whether selectively lobotomizing the LC stuff has > >> any real use at all. > > > We should do the LANG or LC_CTYPE thing only on the client, > > unconditionally. The --no-locale/--locale options should primarily > > determine what the temporary server uses. > > Well, that seems fairly reasonable, but it's going to require some > refactoring of pg_regress. The initialize_environment function > determines what happens in both the client and the temp server. Two possible approaches to fix the tests are as follows: diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c index f2f9603..74cdaa2 100644 --- a/src/test/regress/pg_regress.c +++ b/src/test/regress/pg_regress.c @@ -711,8 +711,7 @@ initialize_environment(void) * is actually called.) */ unsetenv("LANGUAGE"); - unsetenv("LC_ALL"); - putenv("LC_MESSAGES=C"); + putenv("LC_ALL=C"); /* * Set multibyte as requested Here we just force the locale to C. This does have the disadvantage that --no-locale is made redundant, and any tests which are dependent upon locale (if any?) will be run in the C locale. diff --git a/src/test/regress/pg_regress.c b/src/test/regress/pg_regress.c index f2f9603..65fb49a 100644 --- a/src/test/regress/pg_regress.c +++ b/src/test/regress/pg_regress.c @@ -712,6 +712,7 @@ initialize_environment(void) */ unsetenv("LANGUAGE"); unsetenv("LC_ALL"); + putenv("LC_CTYPE=C"); putenv("LC_MESSAGES=C"); /* Here we set LC_CTYPE to C in addition to LC_MESSAGES (and for much the same reasons). However, if you test on non-C locales to check for issues with other locale codesets, those tests are all going to be forced to use ASCII. Is this a problem in practice? From the POV of my patch, it's working as designed: if the locale codeset is UTF-8 it's outputting UTF-8. But, due to the way the test machinery is looking at the output, this is breaking the tests. I'm not sure what I can do with my patch to make it behave differently that is both compatible with its intended use and not break the tests-- this is really just breaking an assumption in the testing code that the test output will always be ASCII. Forcing the LC_CTYPE to C will force ASCII output and work around this problem with the tests. Another approach would be to let psql know it's being run in a test environment with a PG_TEST or some other environment variable which we can check for and use to turn off UTF-8 output if set. Would that be better? Regards, Roger -- .''`. Roger Leigh : :' : Debian GNU/Linux http://people.debian.org/~rleigh/ `. `' Printing on GNU/Linux? http://gutenprint.sourceforge.net/ `- GPG Public Key: 0x25BFB848 Please GPG sign your mail. -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers
From: Andrew Dunstan on 30 Sep 2009 10:47
Roger Leigh wrote: > Here we just force the locale to C. This does have the disadvantage > that --no-locale is made redundant, and any tests which are dependent > upon locale (if any?) will be run in the C locale. > > > That is not a solution. We have not that long ago gone to some lengths to provide for buildfarm testing in various locales. We're not going to undo that. cheers andrew -- Sent via pgsql-hackers mailing list (pgsql-hackers(a)postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers |