More fixes for abbreviated keys infrastructure.
First, when LC_COLLATE = C, bttext_abbrev_convert should use memcpy() rather than strxfrm() to construct the abbreviated key, because the authoritative comparator uses memcpy(). If we do anything else here, we might get inconsistent answers, and the buildfarm says this risk is not theoretical. It should be faster this way, too. Second, while I'm looking at bttext_abbrev_convert, convert a needless use of goto into the loop it's trying to implement into an actual loop. Both of the above problems date to the original commit of abbreviated keys, commit4ea51cdfe8
. Third, fix a bogus assignment to tss->locale before tss is set up. That's a new goof in commitb529b65d1b
.
This commit is contained in:
parent
b529b65d1b
commit
b181a91981
|
@ -63,6 +63,7 @@ typedef struct
|
||||||
char *buf2; /* 2nd string, or abbreviation strxfrm() buf */
|
char *buf2; /* 2nd string, or abbreviation strxfrm() buf */
|
||||||
int buflen1;
|
int buflen1;
|
||||||
int buflen2;
|
int buflen2;
|
||||||
|
bool collate_c;
|
||||||
hyperLogLogState abbr_card; /* Abbreviated key cardinality state */
|
hyperLogLogState abbr_card; /* Abbreviated key cardinality state */
|
||||||
hyperLogLogState full_card; /* Full key cardinality state */
|
hyperLogLogState full_card; /* Full key cardinality state */
|
||||||
#ifdef HAVE_LOCALE_T
|
#ifdef HAVE_LOCALE_T
|
||||||
|
@ -1744,7 +1745,7 @@ static void
|
||||||
btsortsupport_worker(SortSupport ssup, Oid collid)
|
btsortsupport_worker(SortSupport ssup, Oid collid)
|
||||||
{
|
{
|
||||||
bool abbreviate = ssup->abbreviate;
|
bool abbreviate = ssup->abbreviate;
|
||||||
bool locale_aware = false;
|
bool collate_c = false;
|
||||||
TextSortSupport *tss;
|
TextSortSupport *tss;
|
||||||
|
|
||||||
#ifdef HAVE_LOCALE_T
|
#ifdef HAVE_LOCALE_T
|
||||||
|
@ -1769,7 +1770,10 @@ btsortsupport_worker(SortSupport ssup, Oid collid)
|
||||||
* bttextcmp() via the fmgr trampoline.
|
* bttextcmp() via the fmgr trampoline.
|
||||||
*/
|
*/
|
||||||
if (lc_collate_is_c(collid))
|
if (lc_collate_is_c(collid))
|
||||||
|
{
|
||||||
ssup->comparator = bttextfastcmp_c;
|
ssup->comparator = bttextfastcmp_c;
|
||||||
|
collate_c = true;
|
||||||
|
}
|
||||||
#ifdef WIN32
|
#ifdef WIN32
|
||||||
else if (GetDatabaseEncoding() == PG_UTF8)
|
else if (GetDatabaseEncoding() == PG_UTF8)
|
||||||
return;
|
return;
|
||||||
|
@ -1777,7 +1781,6 @@ btsortsupport_worker(SortSupport ssup, Oid collid)
|
||||||
else
|
else
|
||||||
{
|
{
|
||||||
ssup->comparator = bttextfastcmp_locale;
|
ssup->comparator = bttextfastcmp_locale;
|
||||||
locale_aware = true;
|
|
||||||
|
|
||||||
/*
|
/*
|
||||||
* We need a collation-sensitive comparison. To make things faster,
|
* We need a collation-sensitive comparison. To make things faster,
|
||||||
|
@ -1798,7 +1801,7 @@ btsortsupport_worker(SortSupport ssup, Oid collid)
|
||||||
errhint("Use the COLLATE clause to set the collation explicitly.")));
|
errhint("Use the COLLATE clause to set the collation explicitly.")));
|
||||||
}
|
}
|
||||||
#ifdef HAVE_LOCALE_T
|
#ifdef HAVE_LOCALE_T
|
||||||
tss->locale = pg_newlocale_from_collation(collid);
|
locale = pg_newlocale_from_collation(collid);
|
||||||
#endif
|
#endif
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
@ -1828,7 +1831,7 @@ btsortsupport_worker(SortSupport ssup, Oid collid)
|
||||||
* will make use of the temporary buffers we initialize here for scratch
|
* will make use of the temporary buffers we initialize here for scratch
|
||||||
* space, and the abbreviation case requires additional state.
|
* space, and the abbreviation case requires additional state.
|
||||||
*/
|
*/
|
||||||
if (abbreviate || locale_aware)
|
if (abbreviate || !collate_c)
|
||||||
{
|
{
|
||||||
tss = palloc(sizeof(TextSortSupport));
|
tss = palloc(sizeof(TextSortSupport));
|
||||||
tss->buf1 = palloc(TEXTBUFLEN);
|
tss->buf1 = palloc(TEXTBUFLEN);
|
||||||
|
@ -1838,6 +1841,7 @@ btsortsupport_worker(SortSupport ssup, Oid collid)
|
||||||
#ifdef HAVE_LOCALE_T
|
#ifdef HAVE_LOCALE_T
|
||||||
tss->locale = locale;
|
tss->locale = locale;
|
||||||
#endif
|
#endif
|
||||||
|
tss->collate_c = collate_c;
|
||||||
ssup->ssup_extra = tss;
|
ssup->ssup_extra = tss;
|
||||||
|
|
||||||
/*
|
/*
|
||||||
|
@ -2011,45 +2015,58 @@ bttext_abbrev_convert(Datum original, SortSupport ssup)
|
||||||
memset(pres, 0, sizeof(Datum));
|
memset(pres, 0, sizeof(Datum));
|
||||||
len = VARSIZE_ANY_EXHDR(authoritative);
|
len = VARSIZE_ANY_EXHDR(authoritative);
|
||||||
|
|
||||||
/* By convention, we use buffer 1 to store and NUL-terminate text */
|
|
||||||
if (len >= tss->buflen1)
|
|
||||||
{
|
|
||||||
pfree(tss->buf1);
|
|
||||||
tss->buflen1 = Max(len + 1, Min(tss->buflen1 * 2, MaxAllocSize));
|
|
||||||
tss->buf1 = palloc(tss->buflen1);
|
|
||||||
}
|
|
||||||
|
|
||||||
/* Just like strcoll(), strxfrm() expects a NUL-terminated string */
|
|
||||||
memcpy(tss->buf1, VARDATA_ANY(authoritative), len);
|
|
||||||
tss->buf1[len] = '\0';
|
|
||||||
|
|
||||||
/* Don't leak memory here */
|
|
||||||
if (PointerGetDatum(authoritative) != original)
|
|
||||||
pfree(authoritative);
|
|
||||||
|
|
||||||
retry:
|
|
||||||
|
|
||||||
/*
|
/*
|
||||||
* There is no special handling of the C locale here, unlike with
|
* If we're using the C collation, use memcmp(), rather than strxfrm(),
|
||||||
* varstr_cmp(). strxfrm() is used indifferently.
|
* to abbreviated keys. The full comparator for the C locale is always
|
||||||
|
* memcmp(), and we can't risk having this give a different answer.
|
||||||
|
* Besides, this should be faster, too.
|
||||||
*/
|
*/
|
||||||
#ifdef HAVE_LOCALE_T
|
if (tss->collate_c)
|
||||||
if (tss->locale)
|
memcpy(pres, VARDATA_ANY(authoritative), Min(len, sizeof(Datum)));
|
||||||
bsize = strxfrm_l(tss->buf2, tss->buf1, tss->buflen2, tss->locale);
|
|
||||||
else
|
else
|
||||||
#endif
|
|
||||||
bsize = strxfrm(tss->buf2, tss->buf1, tss->buflen2);
|
|
||||||
|
|
||||||
if (bsize >= tss->buflen2)
|
|
||||||
{
|
{
|
||||||
/*
|
/*
|
||||||
* The C standard states that the contents of the buffer is now
|
* We're not using the C collation, so fall back on strxfrm.
|
||||||
* unspecified. Grow buffer, and retry.
|
|
||||||
*/
|
*/
|
||||||
pfree(tss->buf2);
|
|
||||||
tss->buflen2 = Max(bsize + 1, Min(tss->buflen2 * 2, MaxAllocSize));
|
/* By convention, we use buffer 1 to store and NUL-terminate text */
|
||||||
tss->buf2 = palloc(tss->buflen2);
|
if (len >= tss->buflen1)
|
||||||
goto retry;
|
{
|
||||||
|
pfree(tss->buf1);
|
||||||
|
tss->buflen1 = Max(len + 1, Min(tss->buflen1 * 2, MaxAllocSize));
|
||||||
|
tss->buf1 = palloc(tss->buflen1);
|
||||||
|
}
|
||||||
|
|
||||||
|
/* Just like strcoll(), strxfrm() expects a NUL-terminated string */
|
||||||
|
memcpy(tss->buf1, VARDATA_ANY(authoritative), len);
|
||||||
|
tss->buf1[len] = '\0';
|
||||||
|
|
||||||
|
/* Don't leak memory here */
|
||||||
|
if (PointerGetDatum(authoritative) != original)
|
||||||
|
pfree(authoritative);
|
||||||
|
|
||||||
|
for (;;)
|
||||||
|
{
|
||||||
|
#ifdef HAVE_LOCALE_T
|
||||||
|
if (tss->locale)
|
||||||
|
bsize = strxfrm_l(tss->buf2, tss->buf1,
|
||||||
|
tss->buflen2, tss->locale);
|
||||||
|
else
|
||||||
|
#endif
|
||||||
|
bsize = strxfrm(tss->buf2, tss->buf1, tss->buflen2);
|
||||||
|
|
||||||
|
if (bsize < tss->buflen2)
|
||||||
|
break;
|
||||||
|
|
||||||
|
/*
|
||||||
|
* The C standard states that the contents of the buffer is now
|
||||||
|
* unspecified. Grow buffer, and retry.
|
||||||
|
*/
|
||||||
|
pfree(tss->buf2);
|
||||||
|
tss->buflen2 = Max(bsize + 1,
|
||||||
|
Min(tss->buflen2 * 2, MaxAllocSize));
|
||||||
|
tss->buf2 = palloc(tss->buflen2);
|
||||||
|
}
|
||||||
}
|
}
|
||||||
|
|
||||||
/*
|
/*
|
||||||
|
|
Loading…
Reference in New Issue