On Thu, Jan 22, 2015 at 10:57 AM, Robert Haas <robertmhaas@gmail.com> wrote:
> On Wed, Jan 21, 2015 at 2:30 PM, Peter Geoghegan <pg@heroku.com> wrote:
>> Even following Robert's disabling of abbreviated keys on Windows,
>> buildfarm animals hamerkop, brolga, currawong and bowerbird remain
>> unhappy, with failing regression tests for "collate" and sometimes
>> (but not always) "aggregates". Some of these only use the C locale.
>>
>> I think that "aggregates" does not fail for brolga because it's built
>> without "locale_t" support (not sure how to interpret MSVC "configure"
>> output, but I think that the other animals do have such support). So
>> maybe this code being executed within btsortsupport_worker(), for the
>> C locale on Windows is at issue (note that abbreviation is still
>> disabled):
>>
>> tss->locale = pg_newlocale_from_collation(collid);
>
> Yes, you seem to have rather unwisely changed around the order of the
> checks in btsortsupport_worker(). I've just rewritten it heavily to
> try to more clearly separate the decision about whether to do
> fmgr-elision, and if so which comparator to use, from the decision
> about whether to use abbreviation. Naturally I can't promise that
> this is completely right, but I hope that, if it's not, it will at
> least be easier to fix. There's no sanity in removing the
> lc_collate_is_c() check from the top of the function and then trying
> to remember to exclude that case individually from the multiple places
> further down that count on the fact that they'll never be reached in
> that case. This function may even have a third decision to make
> someday, and they can't all be intertwined.
This seems to have broken more stuff. My working hypothesis is that
the culprit is here:
/* * There is no special handling of the C locale here, unlike with * varstr_cmp(). strxfrm() is
usedindifferently. */
As far as I can see, that's just hoping that when the locale is C,
strxfrm() is memcpy(). If it isn't, then you will potentially get
different results when the abbreviated keys stuff is used vs. when it
isn't, because when it isn't, we're going to memcmp(), and when it is,
we're going to memcmp() the results of strxfrm().
--
Robert Haas
EnterpriseDB: http://www.enterprisedb.com
The Enterprise PostgreSQL Company