Re: B-Tree support function number 3 (strxfrm() optimization)

Поиск
Список
Период
Сортировка
От Peter Geoghegan
Тема Re: B-Tree support function number 3 (strxfrm() optimization)
Дата
Msg-id CAM3SWZSm-Xu5+Akz-Q7Wzb_v8pUOZfcX0L3K-eL_2v9Bdo0mtw@mail.gmail.com
обсуждение исходный текст
Ответ на Re: B-Tree support function number 3 (strxfrm() optimization)  (Michael Paquier <michael.paquier@gmail.com>)
Ответы Re: B-Tree support function number 3 (strxfrm() optimization)  (David Rowley <dgrowleyml@gmail.com>)
Список pgsql-hackers
On Mon, Jan 19, 2015 at 7:47 PM, Michael Paquier
<michael.paquier@gmail.com> wrote:
> On MinGW-32, not that I know of:
> $ find . -name *.h | xgrep strxfrm_l
> ./lib/gcc/mingw32/4.8.1/include/c++/mingw32/bits/c++config.h:/* Define if strxfr
> m_l is available in <string.h>. */
> ./mingw32/lib/gcc/mingw32/4.8.1/include/c++/mingw32/bits/c++config.h:/* Define i
> f strxfrm_l is available in <string.h>. */
> strxfrm is defined in string.h though.

I'm not quite following. Doesn't that imply that strxfrm_l() at least
*could* be available? I guess it doesn't matter, though, because the
animal with the successful build that fails the locale regression test
(brolga) does not have locale_t support. Therefore, there is no new
strxfrm_l() caller.

My next guess is that the real problem is an assumption I've made.
That is, my assumption that strxfrm() always behaves equivalently to
strcpy() when the C locale happens to be in use may not be portable
(due to external factors). I guess we're inconsistent about making
sure that LC_COLLATE is set correctly in WIN32 and/or EXEC_BACKEND
builds, or something along those lines. The implementation in the past
got away with strcoll()/strxfrm() not having the C locale set, since
strcoll() was never called when the C locale was in use -- we just
called strcmp() instead.

Assuming that's correct, it might be easier just to entirely disable
the optimization on Windows, even with the C locale. It may not be
worth it to even bother just for C locale support of abbreviated keys.
I'm curious about what will happen there when the "_strxfrm_l()" fix
patch is applied.

> With your patch applied, the failure with MSVC disappeared, but there
> is still a warning showing up:
> (ClCompile target) ->
>   src\backend\lib\hyperloglog.c(73): warning C4334: '<<' : result of
> 32-bit shift implicitly converted to 64 bits (was 64-bit shift
> intended?

That seems harmless. I suggest an explicit cast to "Size" here.

-- 
Peter Geoghegan



В списке pgsql-hackers по дате отправления:

Предыдущее
От: Michael Paquier
Дата:
Сообщение: Re: Patch: add recovery_timeout option to control timeout of restore_command nonzero status code
Следующее
От: Matt Kelly
Дата:
Сообщение: Re: Async execution of postgres_fdw.