Re: PATCH: CITEXT 2.0 v3

"David E. Wheeler" <david(at)kineticode(dot)com> writes:
> On Jul 12, 2008, at 12:17, Tom Lane wrote:
>> * You should provide binary I/O (send/receive) functions, if you want
>> this to be an industrial-strength module. It's easy since you can
>> piggyback on text's.

> I'm confused. Is that not what the citextin and citextout functions are?

No, those are text I/O. You need analogues of textsend and textrecv
too.

>> You might try running the
>> opr_sanity regression test on this module to see if it finds any
>> other silliness. (Procedure: insert the citext definition script
>> into the serial_schedule list just ahead of opr_sanity, run tests,
>> make sure you understand the reason for any diffs in the opr_sanity
>> result. There will be at least one from the uses of text-related
>> functions for citext.)

> Thanks. Added to my list.

BTW, actually a better idea would be to put citext.sql at the front of
the list and just run the whole main regression series with it present.
typ_sanity and oidjoins might possibly find issues too.

>> * Don't use the OPERATOR() notation when you don't need to.
>> It's just clutter.

> Sorry, don't know what you're referring to here.

Some (not all) of your CREATE OPERATOR commands have things like

NEGATOR = OPERATOR(!~),

which seems unnecessary, and is certainly inconsistent.

regards, tom lane