On Wed, Oct 16, 2013 at 02:17:11PM -0400, Bruce Momjian wrote:
> > > You can see the UTF8 case is fine because \n is considered greater
> > > than space, but in the C locale, where \n is less than space, the
> > > false return value shows the problem with
> > > internal_bpchar_pattern_compare() trimming the string and first
> > > comparing on lengths.  This is exactly the problem you outline, where
> > > space trimming assumes everything is less than a space.
> > 
> > For collations other than C some of those issues that have to do with
> > string comparisons might simply be hidden, depending on how strcoll()
> > handles inputs off different lengths: If strcoll() applies implicit
> > space padding to the shorter value, there won't be any visible
> > difference in ordering between bpchar and varchar values.  If strcoll()
> > does not apply such space padding, the right-trimming of bpchar values
> > causes very similar issues even in a en_US collation.

I have added the attached C comment to explain the problem, and added a
TODO item to fix it if we ever break binary upgrading.

Does anyone think this warrants a doc mention?

-- 
  Bruce Momjian  <br...@momjian.us>        http://momjian.us
  EnterpriseDB                             http://enterprisedb.com

  + Everyone has their own god. +
diff --git a/src/backend/utils/adt/varchar.c b/src/backend/utils/adt/varchar.c
new file mode 100644
index 502ca44..284b5d1
*** a/src/backend/utils/adt/varchar.c
--- b/src/backend/utils/adt/varchar.c
*************** bpcharcmp(PG_FUNCTION_ARGS)
*** 846,851 ****
--- 846,863 ----
  				len2;
  	int			cmp;
  
+ 	/*
+ 	 * Trimming trailing spaces off of both strings can cause a string
+ 	 * with a character less than a space to compare greater than a
+ 	 * space-extended string, e.g. this returns false:
+ 	 *		SELECT E'ab\n'::CHAR(10) < E'ab '::CHAR(10);
+ 	 * even though '\n' is less than the space if CHAR(10) was
+ 	 * space-extended.  The correct solution would be to trim only
+ 	 * the longer string to be the same length of the shorter, if
+ 	 * possible, then do the comparison.  However, changing this
+ 	 * might break existing indexes, breaking binary upgrades.
+ 	 * For details, see http://www.postgresql.org/message-id/CAK+WP1xdmyswEehMuetNztM4H199Z1w9KWRHVMKzyyFM+hV=z...@mail.gmail.com
+ 	 */
  	len1 = bcTruelen(arg1);
  	len2 = bcTruelen(arg2);
  
-- 
Sent via pgsql-hackers mailing list (pgsql-hackers@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

Reply via email to