Thread: citex regression fails with de.UTF8 locale

I setup more locale testing on gothic moth and citext regression test
fails.

See
http://www.pgbuildfarm.org/cgi-bin/show_log.pl?nm=gothic_moth&dt=2009-04-22%2020:06:01


Problem is here:

---------------------------------------------------
SELECT citext_cmp('B'::citext, 'a'::citext) AS one;  one  -----
!    1 (1 row)  -- Do some tests using a table and index.
--- 216,222 ---- SELECT citext_cmp('B'::citext, 'a'::citext) AS one;  one  -----
!   28 (1 row)  -- Do some tests using a table and index.
---------------------------------------------------

It seems to me that citex_cmp can return any integer value. It depends
what wcscoll() returns. I think it should be changed to:

SELECT citext_cmp('B'::citext, 'a'::citext) > 0 AS one;
Zdenek

Zdenek Kotala wrote:
> It seems to me that citex_cmp can return any integer value. It depends
> what wcscoll() returns. I think it should be changed to:
> 
> SELECT citext_cmp('B'::citext, 'a'::citext) > 0 AS one;

The comment in varstr_cmp() claims that it returns -1, 0 or 1. That's 
not accurate then.

Comment and test case fixed. I considered changing varstr_cmp to really 
return -1, 0 or 1, but I didn't because the behavior has been unchanged 
for ages and all the callers are happy with it. That's a heavily called 
function so any extra cycles there could make a difference.

--   Heikki Linnakangas  EnterpriseDB   http://www.enterprisedb.com

Heikki Linnakangas píše v čt 23. 04. 2009 v 10:22 +0300:
> Zdenek Kotala wrote:

> Comment and test case fixed. 

Thanks

> I considered changing varstr_cmp to really 
> return -1, 0 or 1, but I didn't because the behavior has been unchanged 
> for ages and all the callers are happy with it. That's a heavily called 
> function so any extra cycles there could make a difference.

I think keep it as it is good decision. strcmp() also does not guarantee
-1, 0, 1.
Zdenek

On Apr 23, 2009, at 12:22 AM, Heikki Linnakangas wrote:

> Zdenek Kotala wrote:
>> It seems to me that citex_cmp can return any integer value. It  
>> depends
>> what wcscoll() returns. I think it should be changed to:
>> SELECT citext_cmp('B'::citext, 'a'::citext) > 0 AS one;
>
> The comment in varstr_cmp() claims that it returns -1, 0 or 1.  
> That's not accurate then.
>
> Comment and test case fixed. I considered changing varstr_cmp to  
> really return -1, 0 or 1, but I didn't because the behavior has been  
> unchanged for ages and all the callers are happy with it. That's a  
> heavily called function so any extra cycles there could make a  
> difference.

Thanks, I also changed it in my version for 8.3.
  https://svn.kineticode.com/citext/trunk/

Best,

David