Re: Getting server crash on Windows when using ICU collation

Hi,

On Sun, Jun 18, 2017 at 6:39 PM, Amit Kapila <amit(dot)kapila16(at)gmail(dot)com> wrote:
> On Sat, Jun 17, 2017 at 8:27 PM, Ashutosh Sharma <ashu(dot)coek88(at)gmail(dot)com> wrote:
>> Hi,
>>
>> On Sat, Jun 17, 2017 at 6:38 PM, Peter Eisentraut
>> <peter(dot)eisentraut(at)2ndquadrant(dot)com> wrote:
>>> On 6/16/17 23:46, Amit Kapila wrote:
>>>> I have just posted one way
>>>> to determine if icu library has support for ucol_strcollUTF8, see if
>>>> that sounds like a way forward to you.
>>>
>>> I'm not in a position to test such patches, so someone else will have to
>>> take that on.
>>
>
> I can verify the patch on Win7 setup to which I have access if that helps.
>
>> Well, I have tested it from my end. I've basically tried to test it by
>> running multi-byte tests as Amit suggested and by verifing the things
>> manually through debugger . And, i had mistakenly attached wrong patch
>> in my earlier email. Here, i attach the correct patch.
>>
>
> Is it possible for you to once verify this patch with icu library
> version where ucol_strcollUTF8 is not defined?

Okay, I have verified the attached patch with following ICU versions
and didn't find any problem

1) ICU 4.8.1.1
2) ICU 49.1.2
3) ICU 50.1.2
4) ICU 53.1

The first two ICU versions i.e. 'ICU 4.8.1.1' and 'ICU 49.1.2' doesn't
have 'ucol_strcollUTF8' defined in it whereas the last two versions
i.e. 'ICU 50.1.2' and 'ICU 53.1', includes 'ucol_strcollUTF8'.

>
> + # get the icu version.
> + my $output = `uconv -V /? 2>&1`;
> + $? >> 8 == 0 or die "uconv command not found";
>
> If we don't find unconv, isn't it better to fall back to non-UTF8
> version rather than saying command not found?

Well, if any of the ICU package is installed on our system then we
will certainly find uconv command. The only case where we can see such
error is when user doesn't have any of the ICU packages installed on
their system and are somehow trying to perform icu enabled build and
in such case the build configuration has to fail which i think is the
expected behaviour. Anyways, it is not uconv that decides whether we
should fallback to non-UTF8 or not. It's the ICU version that decides
whether to stick to UTF8 or fallback to nonUTF8 version. Thanks.

>
> + print $o "#define HAVE_UCOL_STRCOLLUTF8 1\n"
> + if ($self->{ICUVersion} >= 50);
>
> Indentation looks slightly odd, see the other usage in the same file.

I have corrected it. Please have a look into the attached patch. Thanks.

--
With Regards,
Ashutosh Sharma
EnterpriseDB:http://www.enterprisedb.com