[Review] Tests citext casts by David Wheeler.

Started by Ryan Bradetichover 17 years ago17 messages
#1Ryan Bradetich
rbradetich@gmail.com

Hello all,

Here is my review of the Test citext casts written by David Wheeler:
http://archives.postgresql.org/message-id/F721EFF1-553C-4E25-A293-7BD08D6957F4@kineticode.com

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

#2David E. Wheeler
david@kineticode.com
In reply to: Ryan Bradetich (#1)
Re: [Review] Tests citext casts by David Wheeler.

On Sep 4, 2008, at 21:40, Ryan Bradetich wrote:

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 for the review, Ryan!

Best,

David

#3Tom Lane
tgl@sss.pgh.pa.us
In reply to: Ryan Bradetich (#1)
Re: [Review] Tests citext casts by David Wheeler.

"Ryan Bradetich" <rbradetich@gmail.com> writes:

Here is my review of the Test citext casts written by David Wheeler:

Thanks for reviewing. I've committed this with your suggestions and
one additional non-cosmetic change: schema-qualify names in the
bodies of the SQL functions so that they are not search_path dependent.

One thing that didn't make a lot of sense to me was the last new
function:

CREATE OR REPLACE FUNCTION translate( citext, citext, text ) RETURNS TEXT AS $$
SELECT pg_catalog.translate( pg_catalog.translate( $1::pg_catalog.text, pg_catalog.lower($2::pg_catalog.text), $3), pg_catalog.upper($2::pg_catalog.text), $3);
$$ LANGUAGE SQL IMMUTABLE STRICT;

Why is it using upper()?

regards, tom lane

#4David E. Wheeler
david@kineticode.com
In reply to: Tom Lane (#3)
Re: [Review] Tests citext casts by David Wheeler.

On Sep 5, 2008, at 11:30, Tom Lane wrote:

Thanks for reviewing. I've committed this with your suggestions and
one additional non-cosmetic change: schema-qualify names in the
bodies of the SQL functions so that they are not search_path
dependent.

Thanks, I'll check that out.

One thing that didn't make a lot of sense to me was the last new
function:

CREATE OR REPLACE FUNCTION translate( citext, citext, text ) RETURNS
TEXT AS $$
SELECT
pg_catalog.translate( pg_catalog.translate( $1::pg_catalog.text,
pg_catalog.lower($2::pg_catalog.text), $3),
pg_catalog.upper($2::pg_catalog.text), $3);
$$ LANGUAGE SQL IMMUTABLE STRICT;

Why is it using upper()?

To make translate() work case-insensitively, it does two translates:
One lowercase and one uppercase. This allows the translated value to
be returned with its original casing in tact. No, this isn't ideal,
but it was simple to do.

Best,

David

#5David E. Wheeler
david@kineticode.com
In reply to: David E. Wheeler (#4)
1 attachment(s)
Re: [Review] Tests citext casts by David Wheeler.

On Sep 5, 2008, at 11:33, David E. Wheeler wrote:

On Sep 5, 2008, at 11:30, Tom Lane wrote:

Thanks for reviewing. I've committed this with your suggestions and
one additional non-cosmetic change: schema-qualify names in the
bodies of the SQL functions so that they are not search_path
dependent.

Thanks, I'll check that out.

Finally got to this; sorry for the delay.

Two things I noticed:

1. Did I neglect to include the documentation patch? I've attached it
here. It's necessary because of the addition of the new functions.

2. Many thanks for switching to using the network_show function
instead of the SQL-based casting I had. Can you tell me how to go
about finding such functions? Because for my 8.3 version of citext, I
have a whole bunch of functions that do casting like this:

CREATE OR REPLACE FUNCTION int8(citext)
RETURNS int8
AS 'SELECT int8( $1::text )'
LANGUAGE SQL IMMUTABLE STRICT;

CREATE OR REPLACE FUNCTION citext(int8)
RETURNS citext
AS 'SELECT text( $1 )::citext'
LANGUAGE SQL IMMUTABLE STRICT;

CREATE OR REPLACE FUNCTION int4(citext)
RETURNS int4
AS 'SELECT int4( $1::text )'
LANGUAGE SQL IMMUTABLE STRICT;

CREATE OR REPLACE FUNCTION citext(int4)
RETURNS citext
AS 'SELECT text( $1 )::citext'
LANGUAGE SQL IMMUTABLE STRICT;

...and so on. I'd love to be able to replace these (and many others)
with internal C functions, if only I could figure out what those
functions were. A pointer to making that determination (if they even
exist in 8.3) would be greatly appreciated.

Thanks,

David

Attachments:

citext_doc.patchapplication/octet-stream; name=citext_doc.patch; x-unix-mode=0644Download
Index: doc/src/sgml/citext.sgml
===================================================================
RCS file: /projects/cvsroot/pgsql/doc/src/sgml/citext.sgml,v
retrieving revision 1.1
diff -u -r1.1 citext.sgml
--- doc/src/sgml/citext.sgml	29 Jul 2008 18:31:20 -0000	1.1
+++ doc/src/sgml/citext.sgml	12 Sep 2008 16:48:34 -0000
@@ -96,6 +96,72 @@
  </sect2>
 
  <sect2>
+  <title>String Comparison Notes</title>
+  <para>
+   In order to emulate a case-insentive collation as closely as possible, a
+   number all of the comparision operators and functions have been overridden
+   to support <type>citext</>. So, for example, the regular expression
+   operators <literal>~</> and <literal>~*</> exhibit the same behavior, which
+   is to say that they both compare case-insensitively. The same is true
+   for <literal>!~</> and <literal>!~*</>, as well as for the
+   <literal>LIKE</> operators <literal>~~</> and <literal>~~*</>, and
+   <literal>!~~</> and <literal>!~~*</>. If you'd like to match
+   case-sensitively, you can always cast to <type>text</> before comparing.
+  </para>
+
+  <para>
+   The same holds true for comparision functions. All of the following
+   functions behave case-insensitively if either of their comparators
+   is <type>citext</>:
+  </para>
+
+  <itemizedlist>
+   <listitem>
+    <para>
+      <function>regexp_replace()</>
+    </para>
+   </listitem>
+   <listitem>
+    <para>
+      <function>regexp_split_to_array()</>
+    </para>
+   </listitem>
+   <listitem>
+    <para>
+      <function>regexp_split_to_table()</>
+    </para>
+   </listitem>
+   <listitem>
+    <para>
+      <function>replace()</>
+    </para>
+   </listitem>
+   <listitem>
+    <para>
+      <function>split_part()</>
+    </para>
+   </listitem>
+   <listitem>
+    <para>
+      <function>strpos()</>
+    </para>
+   </listitem>
+   <listitem>
+    <para>
+      <function>translate()</>
+    </para>
+   </listitem>
+  </itemizedlist>
+
+  <para>
+   For the regexp functions, if you want to match case-sensitively, you can
+   always use the <quote>c</> flag to force a case-senstive match. Otherwise,
+   you must cast to <type>text</> before calling a function.
+  </para>
+
+ </sect2>
+
+ <sect2>
   <title>Limitations</title>
 
    <itemizedlist>
@@ -116,60 +182,6 @@
 
     <listitem>
      <para>
-      The pattern-matching comparison operators, such as <literal>LIKE</>,
-      <literal>~</>, <literal>~~</>, <literal>!~</>, <literal>!~~</>, etc.,
-      have been overloaded to make case-insensitive comparisons when their
-      left-hand argument is of type <type>citext</>.  However, related
-      functions have not been changed, including:
-     </para>
-
-     <itemizedlist>
-      <listitem>
-       <para>
-         <function>regexp_replace()</>
-       </para>
-      </listitem>
-      <listitem>
-       <para>
-         <function>regexp_split_to_array()</>
-       </para>
-      </listitem>
-      <listitem>
-       <para>
-         <function>regexp_split_to_table()</>
-       </para>
-      </listitem>
-      <listitem>
-       <para>
-         <function>replace()</>
-       </para>
-      </listitem>
-      <listitem>
-       <para>
-         <function>split_part()</>
-       </para>
-      </listitem>
-      <listitem>
-       <para>
-         <function>strpos()</>
-       </para>
-      </listitem>
-      <listitem>
-       <para>
-         <function>translate()</>
-       </para>
-      </listitem>
-     </itemizedlist>
-
-     <para>
-      Of course, for the regular expression functions, you can specify
-      case-insensitive comparisons in their <parameter>flags</> arguments, but
-      the same cannot be done for the the non-regexp functions.
-     </para>
-    </listitem>
-
-    <listitem>
-     <para>
        <type>citext</> is not as efficient as <type>text</> because the
        operator functions and the btree comparison functions must make copies
        of the data and convert it to lower case for comparisons. It is,
#6Tom Lane
tgl@sss.pgh.pa.us
In reply to: David E. Wheeler (#5)
Re: [Review] Tests citext casts by David Wheeler.

"David E. Wheeler" <david@kineticode.com> writes:

1. Did I neglect to include the documentation patch? I've attached it
here. It's necessary because of the addition of the new functions.

Maybe it got left out of the later patch iterations? Anyway,
will take care of it.

2. Many thanks for switching to using the network_show function
instead of the SQL-based casting I had. Can you tell me how to go
about finding such functions?

Er, look into pg_cast and then pg_proc? For instance

select oid::regprocedure, prosrc from pg_proc
where oid in (select castfunc from pg_cast);

regards, tom lane

#7David E. Wheeler
david@kineticode.com
In reply to: Tom Lane (#6)
Re: [Review] Tests citext casts by David Wheeler.

On Sep 12, 2008, at 10:58, Tom Lane wrote:

1. Did I neglect to include the documentation patch? I've attached it
here. It's necessary because of the addition of the new functions.

Maybe it got left out of the later patch iterations? Anyway,
will take care of it.

Great, thank you.

2. Many thanks for switching to using the network_show function
instead of the SQL-based casting I had. Can you tell me how to go
about finding such functions?

Er, look into pg_cast and then pg_proc? For instance

select oid::regprocedure, prosrc from pg_proc
where oid in (select castfunc from pg_cast);

That looks like *exactly* what I need. Thanks!

Best,

David

#8David E. Wheeler
david@kineticode.com
In reply to: David E. Wheeler (#7)
Re: [Review] Tests citext casts by David Wheeler.

On Sep 12, 2008, at 11:06, David E. Wheeler wrote:

Er, look into pg_cast and then pg_proc? For instance

select oid::regprocedure, prosrc from pg_proc
where oid in (select castfunc from pg_cast);

That looks like *exactly* what I need. Thanks!

Pity. Looks like there were only a few I wasn't using, text_char,
char_text, text_name, and texttoxml. Do I really need to keep all my
other casts like these in 8.3?

CREATE OR REPLACE FUNCTION int8(citext)
RETURNS int8
AS 'SELECT int8( $1::text )'
LANGUAGE SQL IMMUTABLE STRICT;

CREATE OR REPLACE FUNCTION citext(int8)
RETURNS citext
AS 'SELECT text( $1 )::citext'
LANGUAGE SQL IMMUTABLE STRICT;

Thanks,

David

#9David E. Wheeler
david@kineticode.com
In reply to: David E. Wheeler (#8)
Re: [Review] Tests citext casts by David Wheeler.

On Sep 12, 2008, at 11:14, David E. Wheeler wrote:

Pity. Looks like there were only a few I wasn't using, text_char,
char_text, text_name, and texttoxml.

Oh, and text_name seems to give me this error:

ERROR: compressed data is corrupt

That's when I have this cast:

CREATE OR REPLACE FUNCTION citext(name)
RETURNS citext
AS 'text_name'
LANGUAGE internal IMMUTABLE STRICT;

This version does not give me an error:

CREATE OR REPLACE FUNCTION citext(name)
RETURNS citext
AS 'SELECT text( $1 )::citext'
LANGUAGE SQL IMMUTABLE STRICT;

Maybe I did something wrong?

Thanks,

David

#10Tom Lane
tgl@sss.pgh.pa.us
In reply to: David E. Wheeler (#9)
Re: [Review] Tests citext casts by David Wheeler.

"David E. Wheeler" <david@kineticode.com> writes:

Oh, and text_name seems to give me this error:

ERROR: compressed data is corrupt

That's when I have this cast:

CREATE OR REPLACE FUNCTION citext(name)
RETURNS citext
AS 'text_name'
LANGUAGE internal IMMUTABLE STRICT;

I think you've got the direction backwards.

BTW, I removed the "Limitations" entry about I/O casting not working
with citext; we fixed that, no?

regards, tom lane

#11Tom Lane
tgl@sss.pgh.pa.us
In reply to: David E. Wheeler (#8)
Re: [Review] Tests citext casts by David Wheeler.

"David E. Wheeler" <david@kineticode.com> writes:

Pity. Looks like there were only a few I wasn't using, text_char,
char_text, text_name, and texttoxml. Do I really need to keep all my
other casts like these in 8.3?

CREATE OR REPLACE FUNCTION int8(citext)
RETURNS int8
AS 'SELECT int8( $1::text )'
LANGUAGE SQL IMMUTABLE STRICT;

Yeah, those are all replaced by the CoerceViaIO mechanism.

regards, tom lane

#12David E. Wheeler
david@kineticode.com
In reply to: Tom Lane (#10)
Re: [Review] Tests citext casts by David Wheeler.

On Sep 12, 2008, at 11:31, Tom Lane wrote:

"David E. Wheeler" <david@kineticode.com> writes:

Oh, and text_name seems to give me this error:

ERROR: compressed data is corrupt

That's when I have this cast:

CREATE OR REPLACE FUNCTION citext(name)
RETURNS citext
AS 'text_name'
LANGUAGE internal IMMUTABLE STRICT;

I think you've got the direction backwards.

Oh. Duh.

BTW, I removed the "Limitations" entry about I/O casting not working
with citext; we fixed that, no?

Yes, we did. Thanks for the catch.

I've got another patch I'm working on adding support for "char" (and
tests for char). Just to fill out a gap I saw in the casting coverage.
I'm trying to get it done now. With that, AFAIK, citext will work just
like text.

Best,

David

#13David E. Wheeler
david@kineticode.com
In reply to: Tom Lane (#11)
Re: [Review] Tests citext casts by David Wheeler.

On Sep 12, 2008, at 11:34, Tom Lane wrote:

CREATE OR REPLACE FUNCTION int8(citext)
RETURNS int8
AS 'SELECT int8( $1::text )'
LANGUAGE SQL IMMUTABLE STRICT;

Yeah, those are all replaced by the CoerceViaIO mechanism

Okay, thanks for the sanity check. The SQL versions are fine for me in
8.3.

Best,

David

#14David E. Wheeler
david@kineticode.com
In reply to: David E. Wheeler (#12)
1 attachment(s)
Re: [Review] Tests citext casts by David Wheeler.

On Sep 12, 2008, at 11:35, David E. Wheeler wrote:

I've got another patch I'm working on adding support for "char" (and
tests for char). Just to fill out a gap I saw in the casting
coverage. I'm trying to get it done now. With that, AFAIK, citext
will work just like text.

Looks like the IO conversions handle char and "char", so the attached
patch just updates the regression test.

Best,

David

Attachments:

char_casts.patchapplication/octet-stream; name=char_casts.patch; x-unix-mode=0644Download
Index: expected/citext.out
===================================================================
RCS file: /projects/cvsroot/pgsql/contrib/citext/expected/citext.out,v
retrieving revision 1.3
diff -u -r1.3 citext.out
--- expected/citext.out	5 Sep 2008 18:25:17 -0000	1.3
+++ expected/citext.out	12 Sep 2008 18:47:32 -0000
@@ -1041,6 +1041,8 @@
     text        text,
     varchar     varchar,
     bpchar      bpchar,
+    char        char,
+    chr         "char",
     name        name,    
     bytea       bytea,
     boolean     boolean,
@@ -1083,6 +1085,14 @@
 INSERT INTO caster (text)          VALUES ('foo'::bpchar);
 INSERT INTO caster (bpchar)        VALUES ('foo'::citext);
 INSERT INTO caster (citext)        VALUES ('foo'::bpchar);
+INSERT INTO caster (char)          VALUES ('f'::text);
+INSERT INTO caster (text)          VALUES ('f'::char);
+INSERT INTO caster (char)          VALUES ('f'::citext);
+INSERT INTO caster (citext)        VALUES ('f'::char);
+INSERT INTO caster (chr)           VALUES ('f'::text);
+INSERT INTO caster (text)          VALUES ('f'::"char");
+INSERT INTO caster (chr)           VALUES ('f'::citext);
+INSERT INTO caster (citext)        VALUES ('f'::"char");
 INSERT INTO caster (name)          VALUES ('foo'::text);
 INSERT INTO caster (text)          VALUES ('foo'::name);
 INSERT INTO caster (name)          VALUES ('foo'::citext);
@@ -1815,6 +1825,52 @@
  t
 (1 row)
 
+<<<<<<< citext.out
+SELECT regexp_matches('foobarbequebaz'::citext, '(BAR)(BEQUE)') = ARRAY[ 'bar', 'beque' ] AS t;
+ t 
+---
+ t
+(1 row)
+
+SELECT regexp_matches('foobarbequebaz'::citext, '(BAR)(BEQUE)'::citext) = ARRAY[ 'bar', 'beque' ] AS t;
+ t 
+---
+ t
+(1 row)
+
+SELECT regexp_matches('foobarbequebaz'::citext, '(BAR)(BEQUE)'::citext, '') = ARRAY[ 'bar', 'beque' ] AS t;
+ t 
+---
+ t
+(1 row)
+
+SELECT regexp_matches('foobarbequebaz'::citext, '(BAR)(BEQUE)', '') = ARRAY[ 'bar', 'beque' ] AS t;
+ t 
+---
+ t
+(1 row)
+
+SELECT regexp_matches('foobarbequebaz', '(BAR)(BEQUE)'::citext, '') = ARRAY[ 'bar', 'beque' ] AS t;
+ t 
+---
+ t
+(1 row)
+
+SELECT regexp_matches('foobarbequebaz'::citext, '(BAR)(BEQUE)'::citext, ''::citext) = ARRAY[ 'bar', 'beque' ] AS t;
+ t 
+---
+ t
+(1 row)
+
+-- c forces case-sensitive
+SELECT regexp_matches('foobarbequebaz'::citext, '(BAR)(BEQUE)'::citext, 'c'::citext) = ARRAY[ 'bar', 'beque' ] AS "null";
+ null 
+------
+ 
+(1 row)
+
+SELECT regexp_replace('Thomas'::citext, '.[mN]a.', 'M') = 'ThM' AS t;
+=======
 SELECT regexp_matches('foobarbequebaz'::citext, '(BAR)(BEQUE)') = ARRAY[ 'bar', 'beque' ] AS t;
  t 
 ---
@@ -1884,6 +1940,32 @@
 
 -- c forces case-sensitive
 SELECT regexp_replace('Thomas'::citext, '.[MN]A.'::citext, 'M', 'c') = 'Thomas' AS t;
+>>>>>>> 1.3
+ t 
+---
+ t
+(1 row)
+
+SELECT regexp_replace('Thomas'::citext, '.[MN]A.',         'M') = 'ThM' AS t;
+ t 
+---
+ t
+(1 row)
+
+SELECT regexp_replace('Thomas',         '.[MN]A.'::citext, 'M') = 'ThM' AS t;
+ t 
+---
+ t
+(1 row)
+
+SELECT regexp_replace('Thomas'::citext, '.[MN]A.'::citext, 'M') = 'ThM' AS t;
+ t 
+---
+ t
+(1 row)
+
+-- c forces case-sensitive
+SELECT regexp_replace('Thomas'::citext, '.[MN]A.'::citext, 'M', 'c') = 'Thomas' AS t;
  t 
 ---
  t
Index: expected/citext_1.out
===================================================================
RCS file: /projects/cvsroot/pgsql/contrib/citext/expected/citext_1.out,v
retrieving revision 1.1
diff -u -r1.1 citext_1.out
--- expected/citext_1.out	5 Sep 2008 18:25:17 -0000	1.1
+++ expected/citext_1.out	12 Sep 2008 18:47:32 -0000
@@ -1041,6 +1041,8 @@
     text        text,
     varchar     varchar,
     bpchar      bpchar,
+    char        char,
+    chr         "char",
     name        name,    
     bytea       bytea,
     boolean     boolean,
@@ -1083,6 +1085,14 @@
 INSERT INTO caster (text)          VALUES ('foo'::bpchar);
 INSERT INTO caster (bpchar)        VALUES ('foo'::citext);
 INSERT INTO caster (citext)        VALUES ('foo'::bpchar);
+INSERT INTO caster (char)          VALUES ('f'::text);
+INSERT INTO caster (text)          VALUES ('f'::char);
+INSERT INTO caster (char)          VALUES ('f'::citext);
+INSERT INTO caster (citext)        VALUES ('f'::char);
+INSERT INTO caster (chr)           VALUES ('f'::text);
+INSERT INTO caster (text)          VALUES ('f'::"char");
+INSERT INTO caster (chr)           VALUES ('f'::citext);
+INSERT INTO caster (citext)        VALUES ('f'::"char");
 INSERT INTO caster (name)          VALUES ('foo'::text);
 INSERT INTO caster (text)          VALUES ('foo'::name);
 INSERT INTO caster (name)          VALUES ('foo'::citext);
Index: sql/citext.sql
===================================================================
RCS file: /projects/cvsroot/pgsql/contrib/citext/sql/citext.sql,v
retrieving revision 1.3
diff -u -r1.3 citext.sql
--- sql/citext.sql	5 Sep 2008 18:25:17 -0000	1.3
+++ sql/citext.sql	12 Sep 2008 18:47:32 -0000
@@ -300,6 +300,8 @@
     text        text,
     varchar     varchar,
     bpchar      bpchar,
+    char        char,
+    chr         "char",
     name        name,    
     bytea       bytea,
     boolean     boolean,
@@ -346,6 +348,16 @@
 INSERT INTO caster (bpchar)        VALUES ('foo'::citext);
 INSERT INTO caster (citext)        VALUES ('foo'::bpchar);
 
+INSERT INTO caster (char)          VALUES ('f'::text);
+INSERT INTO caster (text)          VALUES ('f'::char);
+INSERT INTO caster (char)          VALUES ('f'::citext);
+INSERT INTO caster (citext)        VALUES ('f'::char);
+
+INSERT INTO caster (chr)           VALUES ('f'::text);
+INSERT INTO caster (text)          VALUES ('f'::"char");
+INSERT INTO caster (chr)           VALUES ('f'::citext);
+INSERT INTO caster (citext)        VALUES ('f'::"char");
+
 INSERT INTO caster (name)          VALUES ('foo'::text);
 INSERT INTO caster (text)          VALUES ('foo'::name);
 INSERT INTO caster (name)          VALUES ('foo'::citext);
#15Alvaro Herrera
alvherre@commandprompt.com
In reply to: David E. Wheeler (#14)
Re: [Review] Tests citext casts by David Wheeler.

David E. Wheeler escribi�:

On Sep 12, 2008, at 11:35, David E. Wheeler wrote:

I've got another patch I'm working on adding support for "char" (and
tests for char). Just to fill out a gap I saw in the casting coverage.
I'm trying to get it done now. With that, AFAIK, citext will work just
like text.

Looks like the IO conversions handle char and "char", so the attached
patch just updates the regression test.

There are unresolved conflicts in the patch ...

--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

#16David E. Wheeler
david@kineticode.com
In reply to: Alvaro Herrera (#15)
1 attachment(s)
Re: [Review] Tests citext casts by David Wheeler.

On Sep 12, 2008, at 12:49, Alvaro Herrera wrote:

Looks like the IO conversions handle char and "char", so the attached
patch just updates the regression test.

There are unresolved conflicts in the patch ...

Bah! Sorry. Let me try that again.

Best,

David

Attachments:

char_tests.patchapplication/octet-stream; name=char_tests.patch; x-unix-mode=0644Download
? contrib/citext/citext.sql
? contrib/citext/results
Index: contrib/citext/expected/citext.out
===================================================================
RCS file: /projects/cvsroot/pgsql/contrib/citext/expected/citext.out,v
retrieving revision 1.3
diff -u -r1.3 citext.out
--- contrib/citext/expected/citext.out	5 Sep 2008 18:25:17 -0000	1.3
+++ contrib/citext/expected/citext.out	14 Sep 2008 22:42:07 -0000
@@ -710,6 +710,30 @@
  t
 (1 row)
 
+SELECT 'f'::char::citext = 'f' AS t;
+ t 
+---
+ t
+(1 row)
+
+SELECT 'f'::citext::char = 'f'::char AS t;
+ t 
+---
+ t
+(1 row)
+
+SELECT 'f'::"char"::citext = 'f' AS t;
+ t 
+---
+ t
+(1 row)
+
+SELECT 'f'::citext::"char" = 'f'::"char" AS t;
+ t 
+---
+ t
+(1 row)
+
 SELECT 'foo'::bytea::citext = 'foo' AS t;
  t 
 ---
@@ -1041,6 +1065,8 @@
     text        text,
     varchar     varchar,
     bpchar      bpchar,
+    char        char,
+    chr         "char",
     name        name,    
     bytea       bytea,
     boolean     boolean,
@@ -1083,6 +1109,14 @@
 INSERT INTO caster (text)          VALUES ('foo'::bpchar);
 INSERT INTO caster (bpchar)        VALUES ('foo'::citext);
 INSERT INTO caster (citext)        VALUES ('foo'::bpchar);
+INSERT INTO caster (char)          VALUES ('f'::text);
+INSERT INTO caster (text)          VALUES ('f'::char);
+INSERT INTO caster (char)          VALUES ('f'::citext);
+INSERT INTO caster (citext)        VALUES ('f'::char);
+INSERT INTO caster (chr)           VALUES ('f'::text);
+INSERT INTO caster (text)          VALUES ('f'::"char");
+INSERT INTO caster (chr)           VALUES ('f'::citext);
+INSERT INTO caster (citext)        VALUES ('f'::"char");
 INSERT INTO caster (name)          VALUES ('foo'::text);
 INSERT INTO caster (text)          VALUES ('foo'::name);
 INSERT INTO caster (name)          VALUES ('foo'::citext);
Index: contrib/citext/expected/citext_1.out
===================================================================
RCS file: /projects/cvsroot/pgsql/contrib/citext/expected/citext_1.out,v
retrieving revision 1.1
diff -u -r1.1 citext_1.out
--- contrib/citext/expected/citext_1.out	5 Sep 2008 18:25:17 -0000	1.1
+++ contrib/citext/expected/citext_1.out	14 Sep 2008 22:42:08 -0000
@@ -710,12 +710,60 @@
  t
 (1 row)
 
+SELECT 'f'::char::citext = 'f' AS t;
+ t 
+---
+ t
+(1 row)
+
+SELECT 'f'::citext::char = 'f'::char AS t;
+ t 
+---
+ t
+(1 row)
+
+SELECT 'f'::"char"::citext = 'f' AS t;
+ t 
+---
+ t
+(1 row)
+
+SELECT 'f'::citext::"char" = 'f'::"char" AS t;
+ t 
+---
+ t
+(1 row)
+
 SELECT 'foo'::bytea::citext = 'foo' AS t;
  t 
 ---
  t
 (1 row)
 
+SELECT 'f'::char::citext = 'f' AS t;
+ t 
+---
+ t
+(1 row)
+
+SELECT 'f'::citext::char = 'f'::char AS t;
+ t 
+---
+ t
+(1 row)
+
+SELECT 'f'::"char"::citext = 'f' AS t;
+ t 
+---
+ t
+(1 row)
+
+SELECT 'f'::citext::"char" = 'f'::"char" AS t;
+ t 
+---
+ t
+(1 row)
+
 SELECT 'foo'::citext::bytea = 'foo'::bytea AS t;
  t 
 ---
@@ -1041,6 +1089,8 @@
     text        text,
     varchar     varchar,
     bpchar      bpchar,
+    char        char,
+    chr         "char",
     name        name,    
     bytea       bytea,
     boolean     boolean,
@@ -1083,6 +1133,14 @@
 INSERT INTO caster (text)          VALUES ('foo'::bpchar);
 INSERT INTO caster (bpchar)        VALUES ('foo'::citext);
 INSERT INTO caster (citext)        VALUES ('foo'::bpchar);
+INSERT INTO caster (char)          VALUES ('f'::text);
+INSERT INTO caster (text)          VALUES ('f'::char);
+INSERT INTO caster (char)          VALUES ('f'::citext);
+INSERT INTO caster (citext)        VALUES ('f'::char);
+INSERT INTO caster (chr)           VALUES ('f'::text);
+INSERT INTO caster (text)          VALUES ('f'::"char");
+INSERT INTO caster (chr)           VALUES ('f'::citext);
+INSERT INTO caster (citext)        VALUES ('f'::"char");
 INSERT INTO caster (name)          VALUES ('foo'::text);
 INSERT INTO caster (text)          VALUES ('foo'::name);
 INSERT INTO caster (name)          VALUES ('foo'::citext);
Index: contrib/citext/sql/citext.sql
===================================================================
RCS file: /projects/cvsroot/pgsql/contrib/citext/sql/citext.sql,v
retrieving revision 1.3
diff -u -r1.3 citext.sql
--- contrib/citext/sql/citext.sql	5 Sep 2008 18:25:17 -0000	1.3
+++ contrib/citext/sql/citext.sql	14 Sep 2008 22:42:08 -0000
@@ -220,6 +220,12 @@
 SELECT 'foo'::name::citext = 'foo' AS t;
 SELECT 'foo'::citext::name = 'foo'::name AS t;
 
+SELECT 'f'::char::citext = 'f' AS t;
+SELECT 'f'::citext::char = 'f'::char AS t;
+
+SELECT 'f'::"char"::citext = 'f' AS t;
+SELECT 'f'::citext::"char" = 'f'::"char" AS t;
+
 SELECT 'foo'::bytea::citext = 'foo' AS t;
 SELECT 'foo'::citext::bytea = 'foo'::bytea AS t;
 
@@ -300,6 +306,8 @@
     text        text,
     varchar     varchar,
     bpchar      bpchar,
+    char        char,
+    chr         "char",
     name        name,    
     bytea       bytea,
     boolean     boolean,
@@ -346,6 +354,16 @@
 INSERT INTO caster (bpchar)        VALUES ('foo'::citext);
 INSERT INTO caster (citext)        VALUES ('foo'::bpchar);
 
+INSERT INTO caster (char)          VALUES ('f'::text);
+INSERT INTO caster (text)          VALUES ('f'::char);
+INSERT INTO caster (char)          VALUES ('f'::citext);
+INSERT INTO caster (citext)        VALUES ('f'::char);
+
+INSERT INTO caster (chr)           VALUES ('f'::text);
+INSERT INTO caster (text)          VALUES ('f'::"char");
+INSERT INTO caster (chr)           VALUES ('f'::citext);
+INSERT INTO caster (citext)        VALUES ('f'::"char");
+
 INSERT INTO caster (name)          VALUES ('foo'::text);
 INSERT INTO caster (text)          VALUES ('foo'::name);
 INSERT INTO caster (name)          VALUES ('foo'::citext);
#17David E. Wheeler
david@kineticode.com
In reply to: David E. Wheeler (#16)
1 attachment(s)
Re: [Review] Tests citext casts by David Wheeler.

Just want to make sure that this wasn't lost in the shuffle somewhere…

Best,

David
On Sep 14, 2008, at 15:42, David E. Wheeler wrote:

Show quoted text

On Sep 12, 2008, at 12:49, Alvaro Herrera wrote:

Looks like the IO conversions handle char and "char", so the
attached
patch just updates the regression test.

There are unresolved conflicts in the patch ...

Bah! Sorry. Let me try that again.

Best,

David

Attachments:

char_tests.patchapplication/octet-stream; name=char_tests.patch; x-unix-mode=0644Download
? contrib/citext/citext.sql
? contrib/citext/results
Index: contrib/citext/expected/citext.out
===================================================================
RCS file: /projects/cvsroot/pgsql/contrib/citext/expected/citext.out,v
retrieving revision 1.3
diff -u -r1.3 citext.out
--- contrib/citext/expected/citext.out	5 Sep 2008 18:25:17 -0000	1.3
+++ contrib/citext/expected/citext.out	14 Sep 2008 22:42:07 -0000
@@ -710,6 +710,30 @@
  t
 (1 row)
 
+SELECT 'f'::char::citext = 'f' AS t;
+ t 
+---
+ t
+(1 row)
+
+SELECT 'f'::citext::char = 'f'::char AS t;
+ t 
+---
+ t
+(1 row)
+
+SELECT 'f'::"char"::citext = 'f' AS t;
+ t 
+---
+ t
+(1 row)
+
+SELECT 'f'::citext::"char" = 'f'::"char" AS t;
+ t 
+---
+ t
+(1 row)
+
 SELECT 'foo'::bytea::citext = 'foo' AS t;
  t 
 ---
@@ -1041,6 +1065,8 @@
     text        text,
     varchar     varchar,
     bpchar      bpchar,
+    char        char,
+    chr         "char",
     name        name,    
     bytea       bytea,
     boolean     boolean,
@@ -1083,6 +1109,14 @@
 INSERT INTO caster (text)          VALUES ('foo'::bpchar);
 INSERT INTO caster (bpchar)        VALUES ('foo'::citext);
 INSERT INTO caster (citext)        VALUES ('foo'::bpchar);
+INSERT INTO caster (char)          VALUES ('f'::text);
+INSERT INTO caster (text)          VALUES ('f'::char);
+INSERT INTO caster (char)          VALUES ('f'::citext);
+INSERT INTO caster (citext)        VALUES ('f'::char);
+INSERT INTO caster (chr)           VALUES ('f'::text);
+INSERT INTO caster (text)          VALUES ('f'::"char");
+INSERT INTO caster (chr)           VALUES ('f'::citext);
+INSERT INTO caster (citext)        VALUES ('f'::"char");
 INSERT INTO caster (name)          VALUES ('foo'::text);
 INSERT INTO caster (text)          VALUES ('foo'::name);
 INSERT INTO caster (name)          VALUES ('foo'::citext);
Index: contrib/citext/expected/citext_1.out
===================================================================
RCS file: /projects/cvsroot/pgsql/contrib/citext/expected/citext_1.out,v
retrieving revision 1.1
diff -u -r1.1 citext_1.out
--- contrib/citext/expected/citext_1.out	5 Sep 2008 18:25:17 -0000	1.1
+++ contrib/citext/expected/citext_1.out	14 Sep 2008 22:42:08 -0000
@@ -710,12 +710,60 @@
  t
 (1 row)
 
+SELECT 'f'::char::citext = 'f' AS t;
+ t 
+---
+ t
+(1 row)
+
+SELECT 'f'::citext::char = 'f'::char AS t;
+ t 
+---
+ t
+(1 row)
+
+SELECT 'f'::"char"::citext = 'f' AS t;
+ t 
+---
+ t
+(1 row)
+
+SELECT 'f'::citext::"char" = 'f'::"char" AS t;
+ t 
+---
+ t
+(1 row)
+
 SELECT 'foo'::bytea::citext = 'foo' AS t;
  t 
 ---
  t
 (1 row)
 
+SELECT 'f'::char::citext = 'f' AS t;
+ t 
+---
+ t
+(1 row)
+
+SELECT 'f'::citext::char = 'f'::char AS t;
+ t 
+---
+ t
+(1 row)
+
+SELECT 'f'::"char"::citext = 'f' AS t;
+ t 
+---
+ t
+(1 row)
+
+SELECT 'f'::citext::"char" = 'f'::"char" AS t;
+ t 
+---
+ t
+(1 row)
+
 SELECT 'foo'::citext::bytea = 'foo'::bytea AS t;
  t 
 ---
@@ -1041,6 +1089,8 @@
     text        text,
     varchar     varchar,
     bpchar      bpchar,
+    char        char,
+    chr         "char",
     name        name,    
     bytea       bytea,
     boolean     boolean,
@@ -1083,6 +1133,14 @@
 INSERT INTO caster (text)          VALUES ('foo'::bpchar);
 INSERT INTO caster (bpchar)        VALUES ('foo'::citext);
 INSERT INTO caster (citext)        VALUES ('foo'::bpchar);
+INSERT INTO caster (char)          VALUES ('f'::text);
+INSERT INTO caster (text)          VALUES ('f'::char);
+INSERT INTO caster (char)          VALUES ('f'::citext);
+INSERT INTO caster (citext)        VALUES ('f'::char);
+INSERT INTO caster (chr)           VALUES ('f'::text);
+INSERT INTO caster (text)          VALUES ('f'::"char");
+INSERT INTO caster (chr)           VALUES ('f'::citext);
+INSERT INTO caster (citext)        VALUES ('f'::"char");
 INSERT INTO caster (name)          VALUES ('foo'::text);
 INSERT INTO caster (text)          VALUES ('foo'::name);
 INSERT INTO caster (name)          VALUES ('foo'::citext);
Index: contrib/citext/sql/citext.sql
===================================================================
RCS file: /projects/cvsroot/pgsql/contrib/citext/sql/citext.sql,v
retrieving revision 1.3
diff -u -r1.3 citext.sql
--- contrib/citext/sql/citext.sql	5 Sep 2008 18:25:17 -0000	1.3
+++ contrib/citext/sql/citext.sql	14 Sep 2008 22:42:08 -0000
@@ -220,6 +220,12 @@
 SELECT 'foo'::name::citext = 'foo' AS t;
 SELECT 'foo'::citext::name = 'foo'::name AS t;
 
+SELECT 'f'::char::citext = 'f' AS t;
+SELECT 'f'::citext::char = 'f'::char AS t;
+
+SELECT 'f'::"char"::citext = 'f' AS t;
+SELECT 'f'::citext::"char" = 'f'::"char" AS t;
+
 SELECT 'foo'::bytea::citext = 'foo' AS t;
 SELECT 'foo'::citext::bytea = 'foo'::bytea AS t;
 
@@ -300,6 +306,8 @@
     text        text,
     varchar     varchar,
     bpchar      bpchar,
+    char        char,
+    chr         "char",
     name        name,    
     bytea       bytea,
     boolean     boolean,
@@ -346,6 +354,16 @@
 INSERT INTO caster (bpchar)        VALUES ('foo'::citext);
 INSERT INTO caster (citext)        VALUES ('foo'::bpchar);
 
+INSERT INTO caster (char)          VALUES ('f'::text);
+INSERT INTO caster (text)          VALUES ('f'::char);
+INSERT INTO caster (char)          VALUES ('f'::citext);
+INSERT INTO caster (citext)        VALUES ('f'::char);
+
+INSERT INTO caster (chr)           VALUES ('f'::text);
+INSERT INTO caster (text)          VALUES ('f'::"char");
+INSERT INTO caster (chr)           VALUES ('f'::citext);
+INSERT INTO caster (citext)        VALUES ('f'::"char");
+
 INSERT INTO caster (name)          VALUES ('foo'::text);
 INSERT INTO caster (text)          VALUES ('foo'::name);
 INSERT INTO caster (name)          VALUES ('foo'::citext);