Re: pgsql: ICU support

On 03/24/2017 09:26 AM, Tom Lane wrote:
> Andrew Dunstan <andrew(dot)dunstan(at)2ndquadrant(dot)com> writes:
>> On 03/23/2017 11:53 PM, Tom Lane wrote:
>>> + ERROR: collation "en-x-icu" for encoding "SQL_ASCII" does not exist
>>>
>>> I can reproduce this with vanilla configure options if I'm running
>>> with --with-icu and LANG=C. In short, this regression test does not
>>> work in C locale, and you need to find a way to disable it in that
>>> environment.
>> Possibly something like this:
>> REGRESS_OPTS = --dlpath=. $(EXTRA_REGRESS_OPTS)
>> ifeq ($(with_icu),yes)
>> +ifndef NO_LOCALE
>> +ifneq ($(LANG),C)
>> override EXTRA_TESTS := collate.icu $(EXTRA_TESTS)
>> endif
>> +endif
>> +endif
> That's better than nothing, certainly, but it doesn't catch all the ways
> that C locale could be selected (LC_ALL, no valid setting for LANG, etc).
>
> I suppose that what we really want to know is what encoding will be used
> in the regression database. I wonder whether the Makefile could find
> that out more directly. Although maybe it's time to bite the bullet
> and teach pg_regress how to select whether or not to run the test ---
> that would have the advantage of (probably) working on Windows, which
> no amount of Makefile-hacking will do.

All true, although Peter hasn't actually committed a change that would
enable the test in MSVC builds, so that's not a danger right now.

While the change could be made more robust, along with 3 lines of change
in the buildfarm code to make sure LANG is set in appropriate places it
is enough to turn prion green, and actually running the tests when
testing en_US.UTF8.

I agree we need a general mechanism for running tests conditionally.
Let's not wait for that, though :-)

>
> BTW, is there a reason that override isn't spelled more like
>
> override EXTRA_TESTS += collate.icu
>
> ? That seems more readable and idiomatic to me.
>
>

+1.

cheers

andrew

--
Andrew Dunstan https://www.2ndQuadrant.com
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services