You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@couchdb.apache.org by GitBox <gi...@apache.org> on 2021/04/02 21:02:25 UTC

[GitHub] [couchdb] nickva opened a new pull request #3490: Fix collation issue for older versions of libicu library

nickva opened a new pull request #3490:
URL: https://github.com/apache/couchdb/pull/3490


   Previously, mango tests with objects as keys were failing on CentOS 6 and CentOS 7. The reason for the failures was that old libicu collation algorithms didn't consider the `<<255,255,255,255>>` as the highest sortable string as CouchDB intends it to be. Later versions of libicu, at least as old as 59, [started to do that]( https://www.unicode.org/reports/tr35/tr35-collation.html#tailored_noncharacter_weights). However, as long as we support CentOS 7 we can fix the issue by explicitly checking for the highest marker.
   
   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [couchdb] bessbd edited a comment on pull request #3490: Fix collation issue for older versions of libicu library

Posted by GitBox <gi...@apache.org>.
bessbd edited a comment on pull request #3490:
URL: https://github.com/apache/couchdb/pull/3490#issuecomment-813563345


   > I left a more complete response in [#3488 (comment)](https://github.com/apache/couchdb/pull/3488#issuecomment-812723151) but the idea is it's a bit more general and works along the lines of how current libicu version does (treating `FFFF `as special), so when we stop supporting CentOS 7 we can remove just that one max handling section and let libicu do the right thing.
   
   I still don't get why (lower level) C code should be aware of a specific higher level (marker) constant (in erlang - caller code). - https://github.com/apache/couchdb/pull/3490/files#diff-e07aa2cf62bca24d714bb7b5d125f77a906db4145a60672e532e27a324b7dcf7R69-R73 . Sounds like a Separation of concerns violation.
   
   Nevertheless, I'm confident that we're better off with this PR merged than without. +1


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [couchdb] nickva merged pull request #3490: Fix collation issue for older versions of libicu library

Posted by GitBox <gi...@apache.org>.
nickva merged pull request #3490:
URL: https://github.com/apache/couchdb/pull/3490


   


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [couchdb] bessbd edited a comment on pull request #3490: Fix collation issue for older versions of libicu library

Posted by GitBox <gi...@apache.org>.
bessbd edited a comment on pull request #3490:
URL: https://github.com/apache/couchdb/pull/3490#issuecomment-812721292


   Thank you for this PR, @nickva !
   When I opened https://github.com/apache/couchdb/pull/3488/commits/4bccaa576a3d56912286e80dafb45e9bf43a137e , my thought was that the sentinel value(s) should never actually be passed to "low level" code. Ie. it's a nice thing that those sentinels could actually be items that "order well", but they aren't items that are to be compared with others.
   Having said that, I'd be +1 on this change, too.
   
   What do you think?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [couchdb] bessbd commented on pull request #3490: Fix collation issue for older versions of libicu library

Posted by GitBox <gi...@apache.org>.
bessbd commented on pull request #3490:
URL: https://github.com/apache/couchdb/pull/3490#issuecomment-812721292


   Thank you for this PR, @nickva !
   When I opened https://github.com/apache/couchdb/pull/3488/commits/4bccaa576a3d56912286e80dafb45e9bf43a137e , my thought was that the sentinel value(s) should never actually be passed to low level code. Ie. it's a nice thing that those sentinels could actually be items that "order well", but they aren't items that are to be compared with others.
   Having said that, I'd be +1 on this change, too.
   
   What do you think?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [couchdb] nickva edited a comment on pull request #3490: Fix collation issue for older versions of libicu library

Posted by GitBox <gi...@apache.org>.
nickva edited a comment on pull request #3490:
URL: https://github.com/apache/couchdb/pull/3490#issuecomment-812891475


   I left a more complete response in https://github.com/apache/couchdb/pull/3488#issuecomment-812723151 but the idea is it's a bit more general and works along the lines of how current libicu version does (treating `FFFF `as special), so when we stop supporting CentOS 7 we can remove just that one max handling section and let libicu do the right thing.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [couchdb] bessbd commented on pull request #3490: Fix collation issue for older versions of libicu library

Posted by GitBox <gi...@apache.org>.
bessbd commented on pull request #3490:
URL: https://github.com/apache/couchdb/pull/3490#issuecomment-813563345


   > I left a more complete response in [#3488 (comment)](https://github.com/apache/couchdb/pull/3488#issuecomment-812723151) but the idea is it's a bit more general and works along the lines of how current libicu version does (treating `FFFF `as special), so when we stop supporting CentOS 7 we can remove just that one max handling section and let libicu do the right thing.
   
   I still don't get why (lower level) C code should be aware of a specific higher level (marker) constant (in erlang - caller code). - https://github.com/apache/couchdb/pull/3490/files#diff-e07aa2cf62bca24d714bb7b5d125f77a906db4145a60672e532e27a324b7dcf7R69-R73 . Sounds like a Separation of concerns violation.
   
   Nevertheless, I'm confident that we're better of this PR merged than without. +1


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org



[GitHub] [couchdb] nickva commented on pull request #3490: Fix collation issue for older versions of libicu library

Posted by GitBox <gi...@apache.org>.
nickva commented on pull request #3490:
URL: https://github.com/apache/couchdb/pull/3490#issuecomment-812891475


   I left a more complete response in https://github.com/apache/couchdb/pull/3488#issuecomment-812723151 but the idea is it's a bit more general and works along the lines of how current libicu version do (treating FFFF as special), so when we stop supporting CentOS 7 we can remove just that one max handling section and let libicu do the right thing.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

For queries about this service, please contact Infrastructure at:
users@infra.apache.org