Martijn van Oosterhout wrote:
> FWIW, I disagree with the stripping down and think we should aim for
> the whole patch as submitted. Historically, the hardest part of getting
> collation support into postgres has been the catalog changes and easier
> the support for something other than OS locales. I supported the patch as
> it was precisely because it finally did that, and the stripping down
> takes us back to square one.
The catalog changes are one issue, but hardly the biggest one.
Database-level collation works just as well without them, and for
finer-grain control, the proposed patch was nowhere near enough. You'll
need further catalog changes to track collation for each index column,
for example, probably something in the operator families or classes etc.
In addition to catalog changes, for finer grained collation you need
changes in the planner and executor as well. In planner, to provide
support for the concept of a sort order using a specific collation, and
track that whenever sort order is handled. In executor, the capability
to compare and sort using different locales at different times. Those
changes seem a lot more difficult to me than adding a couple of system
catalogs, which is pretty straightforward.
It's not like the patch is going to disappear from planet Earth if it
doesn't get committed for 8.4. It's still valuable and available when
the new catalogs are needed.
> Implementing the COLLATE keyword is much easier once you have the underlying
> support and woldn't involve any backward incompatabilities. The
> stripped down version gets us to a state which will only be supported
> (hopefully) for one release.
Yes, I hope so too. Implementation in phases is a good thing in my mind,
but adding a lot of complexity that isn't need until in a future release
is not. We will need to support the 8.4 CREATE DATABASE syntax for
backwards-compatibility, but I don't see a problem with that.
> For anyone counting, Firebird added support for ICU more than three
> years ago.
ICU is orthogonal to this patch. This patch didn't provide ICU support,
and we could start using ICU without the catalog changes.
-- Heikki Linnakangas EnterpriseDB http://www.enterprisedb.com
