Hello all, Here is my review of the Test citext casts written by David Wheeler: http://archives.postgresql.org/message-id/[EMAIL PROTECTED]
1. The patch applies cleanly to the latest GIT repository. 2. The citext type installs, uninstalls, and re-installs cleanly. 3. The coding style is mostly consistent with the existing code. The only coding style difference I noticed was introduced in this patch: In the citext.sql.in file the following functions are created using the non-dollar quoting syntax: * regex_matches * regexp_replace * regexp_split_to_array * regexp_split_to table * strpos In the citext.sql.in file the following functions are created using the dollar quoting syntax: * replay * split_part * translate I do not have a strong preference either way and I do not even care if they are consistent. I am interested to see if there was a reason for using both syntaxes for these functions. 4. The regression tests successfully pass when PostgreSQL is built with --with-libxml. They fail when the PostgreSQL is built without --with-libxml. Since the default PostgreSQL configuration does not include --with-libxml and is not run-time detected when the libxml2 libraries are present on the system I would recommend adding an additional expected output file (citext_1.out) that covers the conditions when PostgreSQL is compiled without --with-libxml. As an experiment, I was able to add the citext_1.out and the regression tests passed with and without the --with-libxml option. Review of the additional regression tests show they provide coverage of the new casts and functions added with this patch. Overall I think the patch looks good. After reviewing the patch, I played with citext for an hour or so and I did not encounter any bugs or other surprises. Thanks, - Ryan -- Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org) To make changes to your subscription: http://www.postgresql.org/mailpref/pgsql-hackers