> 11 aug. 2016 kl. 03:05 skrev Peter Geoghegan <p...@heroku.com>:
> On Wed, Aug 10, 2016 at 1:42 PM, Palle Girgensohn <gir...@pingpong.net> wrote:
>> They've been used for the FreeBSD ports since 2005, and have served us well. 
>> I have of course updated them regularly. In this latest version, I've 
>> removed support for other encodings beside UTF-8, mostly since I don't know 
>> how to test them, but also, I see little point in supporting anything else 
>> using ICU.
> Looks like you're not using the ICU equivalent of strxfrm(). While 9.5
> is not the release that introduced its use, it did expand it
> significantly. I think you need to fix this, even though it isn't
> actually used to sort text at present, since presumably FreeBSD builds
> of 9.5 don't TRUST_STRXFRM. Since you're using ICU, though, you could
> reasonably trust the ICU equivalent of strxfrm(), so that's a missed
> opportunity. (See the wiki page on the abbreviated keys issue [1] if
> you don't know what I'm talking about.)

My plan was to get it working without TRUST_STRXFRM first, and then add that 
functinality. I've made some preliminary tests using ICU:s ucol_getSortKey but 
I will have to test it a bit more. For now, I just expect not to trust strxfrm. 
It is the first iteration wrt strxfrm, the plan is to use that code base.

> Shouldn't you really have a strxfrm() wrapper, used across the board,
> including for callers outside of varlena.c? convert_string_datum() has
> been calling strxfrm() for many releases now. These calls are still
> used in FreeBSD builds, I would think, which seems like a bug that is
> not dodged by simply not defining TRUST_STRXFRM. Isn't its assumption
> that that matching the ordering used elsewhere not really hold on
> FreeBSD builds?

I was not aware of convert_string_datum, I will check that, thanks! Using a 
wrapper across the board seems like a good idea for refactoring.

> [1] https://wiki.postgresql.org/wiki/Abbreviated_keys_glibc_issue
> --
> Peter Geoghegan

Attachment: signature.asc
Description: Message signed with OpenPGP using GPGMail

Reply via email to