Thread: contrib/citext versus collations
I've been looking into bug #6053, in which Regina Obe complains that hash-based DISTINCT queries fail for type "citext". The cause is not far to seek: the header comment for execGrouping.c states * Note: we currently assume that equality and hashing functions are not* collation-sensitive, so the code in this file hasno support for passing* collation settings through from callers. That may have to change someday. and indeed the failure comes directly from the fact that citext's hash function *does* expect a collation to be passed to it. I'm a bit embarrassed to not have noticed that citext was a counterexample for this assumption, especially since I already fixed one bug that should have clued me in (commit a0b75a41a907e1582acdb8aa6ebb9cacca39d7d8). Now, removing this assumption from execGrouping.c is already a pretty sizable task --- for starters, at least plan node types Agg, Group, SetOp, Unique, and WindowAgg would need collation attributes that they don't have today. But the assumption that equality operators are not collation-sensitive is baked into a number of other places too; for instancenodeAgg.c @ line 600indxpath.c @ line 2200prepunion.c @ line 640ri_triggers.c @ line 3000 and that's just places where there's a comment about it :-(. It's worth noting also that in many of these places, paying attention to collation is not merely going to need more coding; it will directly translate to a performance hit, one that is entirely unnecessary for the normal case where collation doesn't affect equality. So this leaves us between a rock and a hard place. I think there's just about no chance of fixing all these things without a serious fresh slip in the 9.1 schedule. Also, I'm *not* prepared to fix these things personally. I already regret the amount of time I put into collations this past winter/spring, and am not willing to drop another several weeks down that sinkhole right now. The most workable alternative that I can see is to lobotomize citext so that it always does lower-casing according to the database's "default" collation, which would allow us to pretend that its notion of equality is not collation-sensitive after all. We could hope to improve this in future release cycles, but not till we've done the infrastructure work outlined above. One bit of infrastructure that might be a good idea is a flag to indicate whether an equality operator's behavior is potentially collation-dependent, so that we could avoid taking performance hits in the normal case. Comments, other ideas? regards, tom lane
On Jun 6, 2011, at 1:14 PM, Tom Lane wrote: > The most workable alternative that I can see is to lobotomize citext so > that it always does lower-casing according to the database's "default" > collation, which would allow us to pretend that its notion of equality > is not collation-sensitive after all. +1 Seems like the right thing to do for now. > We could hope to improve this in > future release cycles, but not till we've done the infrastructure work > outlined above. One bit of infrastructure that might be a good idea is > a flag to indicate whether an equality operator's behavior is > potentially collation-dependent, so that we could avoid taking > performance hits in the normal case. That sounds like a good idea. Best, David
"David E. Wheeler" <david@kineticode.com> writes: > On Jun 6, 2011, at 1:14 PM, Tom Lane wrote: >> ... One bit of infrastructure that might be a good idea is >> a flag to indicate whether an equality operator's behavior is >> potentially collation-dependent, so that we could avoid taking >> performance hits in the normal case. > That sounds like a good idea. BTW, it struck me shortly after sending this that we'd already discussed the idea of a flag in pg_proc showing whether a function pays attention to collation. We could of course use that for this purpose. regards, tom lane
On Jun 6, 2011, at 4:35 PM, Tom Lane wrote: >> That sounds like a good idea. > > BTW, it struck me shortly after sending this that we'd already discussed > the idea of a flag in pg_proc showing whether a function pays attention > to collation. We could of course use that for this purpose. Seems like a no-brainer. Best, David
On Mon, Jun 6, 2011 at 9:14 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: > The most workable alternative that I can see is to lobotomize citext so > that it always does lower-casing according to the database's "default" > collation, which would allow us to pretend that its notion of equality > is not collation-sensitive after all. We could hope to improve this in > future release cycles, but not till we've done the infrastructure work > outlined above. One bit of infrastructure that might be a good idea is > a flag to indicate whether an equality operator's behavior is > potentially collation-dependent, so that we could avoid taking > performance hits in the normal case. > > Comments, other ideas? That would also mean that 9.1's citext will be no worse than 9.0, it just won't have the 9.1 collation goodness. Random thought -- the collation used for citext is not really the same as the default collation for ordering in sql. Perhaps it could be stored in the typmod? So you could declare different columns to be case insensitive according to specific collations. And it would be free to cast between them but would have to be explicit. I'm not sure that's actually a good idea, it was just a first thought, -- greg
Greg Stark <gsstark@mit.edu> writes: > On Mon, Jun 6, 2011 at 9:14 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote: >> The most workable alternative that I can see is to lobotomize citext so >> that it always does lower-casing according to the database's "default" >> collation, which would allow us to pretend that its notion of equality >> is not collation-sensitive after all. > That would also mean that 9.1's citext will be no worse than 9.0, it > just won't have the 9.1 collation goodness. On further reflection, I'm wondering exactly how much goodness to chop off there. What I'd originally been thinking was to just lobotomize the case-folding step, and allow citext's comparison operators to still respond to input collation when comparing the folded strings. However, I can imagine that some combinations of languages might produce pretty weird results if we do that. Should we lobotomize the comparisons too? Or is the ability to affect the sort order valuable enough to put up with whatever corner-case funnies there might be? > Random thought -- the collation used for citext is not really the same > as the default collation for ordering in sql. Perhaps it could be > stored in the typmod? Again, I'm wondering whether that's really a good idea. I think the currently implemented behavior of citext (fold and compare both act according to input collation) is really the right thing ... we just can't do it all yet. regards, tom lane
I wrote: > On further reflection, I'm wondering exactly how much goodness to chop > off there. What I'd originally been thinking was to just lobotomize the > case-folding step, and allow citext's comparison operators to still > respond to input collation when comparing the folded strings. However, > I can imagine that some combinations of languages might produce pretty > weird results if we do that. Should we lobotomize the comparisons too? > Or is the ability to affect the sort order valuable enough to put up > with whatever corner-case funnies there might be? For lack of any comment on this point, I went with the first approach. Patch is committed. regards, tom lane