On Tue, Nov 16, 2010 at 04:42, Peter Eisentraut <pete...@gmx.net> wrote: > On mån, 2010-11-15 at 11:34 +0100, Pavel Stehule wrote: >> I am checking a patch. I found a problem with initdb > Ah, late night brain farts, it appears. Here is a corrected version.
This version cannot be applied cleanly any more. Please update it. (I think you don't have to include changes for catversion.h) ./src/backend/optimizer/util/plancat.c.rej ./src/backend/optimizer/plan/createplan.c.rej ./src/backend/optimizer/path/indxpath.c.rej ./src/include/catalog/catversion.h.rej I didn't compile nor run the patched server, but I found a couple of issues in the design and source code: * COLLATE information must be explicitly passed by caller in the patch, but we might forgot the handover when we write new codes. Is it possible to pass it automatically, say using a global variable? If we could do so, existing extensions might work with collation without rewritten. * Did you check the regression test on Windows? We probably cannot use en_US.utf8 on Windows. Also, some output of the test includes non-ASCII characters. How will we test COLLATE feature on non-UTF8 databases? [src/test/regress/sql/collate.sql] +CREATE TABLE collate_test1 ( + a int, + b text COLLATE "en_US.utf8" not null +); * Did you see any performance regression by collation? I found a bug in lc_collate_is_c(); result >= 0 should be checked before any other checks. SearchSysCache1() here would be a performance regression. [src/backend/utils/adt/pg_locale.c] -lc_collate_is_c(void) +lc_collate_is_c(Oid collation) { ... + tp = SearchSysCache1(COLLOID, ObjectIdGetDatum(collation)); ... HERE => if (result >= 0) return (bool) result; -- Itagaki Takahiro -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers