From b15e8f71dbf00eb18d9076302f39aa981355eb07 Mon Sep 17 00:00:00 2001 From: Tom Lane Date: Mon, 16 Apr 2018 16:06:47 -0400 Subject: [PATCH] Fix broken collation-aware searches in SP-GiST text opclass. spg_text_leaf_consistent() supposed that it should compare only Min(querylen, entrylen) bytes of the two strings, and then deal with any excess bytes in one string or the other by assuming the longer string is greater if the prefixes are equal. Quite aside from the fact that that's just wrong in some locales (e.g., 'ch' is not less than 'd' in cs_CZ), it also risked passing incomplete multibyte characters to strcoll(), with ensuing bad results. Instead, just pass the full strings to varstr_cmp, and let it decide what to do about unequal-length strings. Fortunately, this error doesn't imply any index corruption, it's just that searches might return the wrong set of entries. Per report from Emre Hasegeli, though this is not his patch. Thanks to Peter Geoghegan for review and discussion. This code was born broken, so back-patch to all supported branches. In HEAD, I failed to resist the temptation to do a bit of cosmetic cleanup/pgindent'ing on 710d90da1, too. Discussion: https://postgr.es/m/CAE2gYzzb6K51VnTq5i5p52z+j9p2duEa-K1T3RrC_GQEynAKEg@mail.gmail.com --- src/backend/access/spgist/spgtextproc.c | 30 ++++++++++++------------- 1 file changed, 15 insertions(+), 15 deletions(-) diff --git a/src/backend/access/spgist/spgtextproc.c b/src/backend/access/spgist/spgtextproc.c index 76c0305695..153c57b540 100644 --- a/src/backend/access/spgist/spgtextproc.c +++ b/src/backend/access/spgist/spgtextproc.c @@ -626,15 +626,15 @@ spg_text_leaf_consistent(PG_FUNCTION_ARGS) if (strategy == RTPrefixStrategyNumber) { /* - * if level >= length of query then reconstrValue is began with - * query (prefix) string and we don't need to check it again. + * if level >= length of query then reconstrValue must begin with + * query (prefix) string, so we don't need to check it again. */ - res = (level >= queryLen) || - DatumGetBool(DirectFunctionCall2(text_starts_with, - out->leafValue, PointerGetDatum(query))); + DatumGetBool(DirectFunctionCall2(text_starts_with, + out->leafValue, + PointerGetDatum(query))); - if (!res) /* no need to consider remaining conditions */ + if (!res) /* no need to consider remaining conditions */ break; continue; @@ -648,22 +648,22 @@ spg_text_leaf_consistent(PG_FUNCTION_ARGS) /* If asserts enabled, verify encoding of reconstructed string */ Assert(pg_verifymbstr(fullValue, fullLen, false)); - r = varstr_cmp(fullValue, Min(queryLen, fullLen), - VARDATA_ANY(query), Min(queryLen, fullLen), + r = varstr_cmp(fullValue, fullLen, + VARDATA_ANY(query), queryLen, PG_GET_COLLATION()); } else { /* Non-collation-aware comparison */ r = memcmp(fullValue, VARDATA_ANY(query), Min(queryLen, fullLen)); - } - if (r == 0) - { - if (queryLen > fullLen) - r = -1; - else if (queryLen < fullLen) - r = 1; + if (r == 0) + { + if (queryLen > fullLen) + r = -1; + else if (queryLen < fullLen) + r = 1; + } } switch (strategy)