Re: Windows buildfarm animals are still not happy with abbreviated keys patch
От | Robert Haas |
---|---|
Тема | Re: Windows buildfarm animals are still not happy with abbreviated keys patch |
Дата | |
Msg-id | CA+Tgmoa96UDGYwHg=npXZtL_Q=EJR0S6Wn5OWzUbBMoshzybCw@mail.gmail.com обсуждение исходный текст |
Ответ на | Re: Windows buildfarm animals are still not happy with abbreviated keys patch (Robert Haas <robertmhaas@gmail.com>) |
Ответы |
Re: Windows buildfarm animals are still not happy with
abbreviated keys patch
(Robert Haas <robertmhaas@gmail.com>)
|
Список | pgsql-hackers |
On Thu, Jan 22, 2015 at 11:28 AM, Robert Haas <robertmhaas@gmail.com> wrote: > 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 used indifferently. > */ > > 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(). As noted also on the thread Kevin started, I've now pushed a fix for this and am eagerly awaiting new buildfarm returns. I wonder if this might account for the ordering failures we were seeing on Windows before. We thought it was a Windows-specific issue, but I bet the real problem was here: if (collid != DEFAULT_COLLATION_OID) { if (!OidIsValid(collid)) { /* * This typically means that the parser could not resolve a * conflict of implicit collations, so report it that way. */ ereport(ERROR, (errcode(ERRCODE_INDETERMINATE_COLLATION), errmsg("could not determine which collation to use for string comparison"), errhint("Use the COLLATE clause to set the collation explicitly."))); } #ifdef HAVE_LOCALE_T tss->locale = pg_newlocale_from_collation(collid); #endif } Before the abbreviated keys commit, this code only ran if we'd already determined that lc_collate_is_c(collid) was false. But that commit made this also run when that value was true. So if HAVE_LOCALE_T and collid != DEFAULT_COLLATION_ID, then tss->locale was getting set. If it got set to something that made strxfrm() behave like memcmp(), then all was well. If not, then life was bad. Now you'd hope that would work out, but maybe not. Also, suppose HAVE_LOCALE_T was defined but collid == DEFAULT_COLLATION_ID. Then tss->locale didn't get initialized at all, and bttext_abbrev_convert() just passed whatever stupid value it found there to strxfrm_l(). Finally, suppose we *didn't* HAVE_LOCALE_T. Then, the non-abbreviated comparator knew it should use memcmp(), but the abbreviated comparator was happy to use strxfrm() even though that would use the current locale, but the C locale that the user requested. Long story short: this was broken. It may be that when the dust settles we can try re-enabling this on Windows. It might work now that this issue is (hopefully) fixed. -- Robert Haas EnterpriseDB: http://www.enterprisedb.com The Enterprise PostgreSQL Company
В списке pgsql-hackers по дате отправления:
Предыдущее
От: Sawada MasahikoДата:
Сообщение: Re: Merging postgresql.conf and postgresql.auto.conf