Re: insensitive collations

On 2019-02-21 03:17, Peter Geoghegan wrote:
> I wonder if it would be better to break this into distinct commits?

I thought about that. Especially the planner/executor changes could be
done separately, sort of as a way to address the thread
"ExecBuildGroupingEqual versus collations". But I'm not sure if they
would have good test coverage on their own. I can work on this if
people think this would be useful.

> * Why is support for non-deterministic comparisons limited to the ICU
> provider? If that's the only case that could possibly be affected,
> then why did we ever add the varstrcmp() tie-breaker call to strcmp()
> in the first place? The behavior described in the commit message of
> bugfix commit 656beff5 describes a case where Hungarian text caused
> index corruption by being strcoll()-wise equal but not bitwise equal.
> Besides, I think that you can vendor your own case insensitive
> collation with glibc, since it's based on UCA.

The original test case (described here:
https://www.postgresql.org/message-id/27064.1134753128%40sss.pgh.pa.us)
no longer works, so it was probably fixed on the glibc side. The git
log of the hu_HU locale definition shows that it has been "fixed" in
major ways several times over the years, so that's plausible.

I tried reproducing some more practical scenarios involving combining
diacritics, but glibc apparently doesn't believe strings in different
normal forms are equal. At that point I gave up because this doesn't
seem worthwhile to support.

Moreover, I think allowing this would require a "trusted" strxfrm(),
which is currently disabled.

>> + return DatumGetBool(FunctionCall2Coll(&entry->eq_opr_finfo,
>> + DEFAULT_COLLATION_OID,
>> + oldvalue, newvalue));
>> }
>
> The existing restriction on ICU collations (that they cannot be
> database-wide) side-steps the issue.
>
> * Can said restriction somehow be lifted? That seems like it would be
> a lot cleaner.

Lift what restriction? That ICU collations cannot be database-wide?
Sure that would be nice, but it's a separate project. Even then, I'm
not sure that we would allow a database-wide collation to be
nondeterministic. That would for example disallow the use of LIKE,
which would be weird. In any case, the above issue can be addressed
then. I think it's not worth complicating this right now.

> * Why have you disable this optimization?:
>
>> /* Fast pre-check for equality, as discussed in varstr_cmp() */
>> - if (len1 == len2 && memcmp(a1p, a2p, len1) == 0)
>> + if ((!sss->locale || sss->locale->deterministic) &&
>> + len1 == len2 && memcmp(a1p, a2p, len1) == 0)
>
> I don't see why this is necessary. A non-deterministic collation
> cannot indicate that bitwise identical strings are non-equal.

Right, I went too far there.

> * Perhaps you should add a "Tip" referencing the feature to the
> contrib/citext documentation.

Good idea.

--
Peter Eisentraut http://www.2ndQuadrant.com/
PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services