some issue processing \uFFFF in query string in our binary packages [was: garbage chars in query string when Looking up doc via UI ]

Aleksey Kondratenko added a comment - 10/Sep/12 4:17 PM This endkey suffix is there (incorrectly but still) in order to find _all_ docs with given prefix. AFAIK it actually works. I think in your case it probably didn't spot document because it wasn't in .couch file yet.

Tommie McAfee added a comment - 10/Sep/12 4:23 PM nope :) # UI DOES THIS # curl " http://10.3.3.62:8091/couchBase/default/_all_docs?startkey=%22simple%22&endkey=%22simple%EF%BF%BF%22&skip=0&include_docs=true&limit=6 " {"error":"query_parse_error","reason":"No rows can match your key range, reverse your start_key and end_key or set descending=true"} # REMOVE EXTRA CHARS FROM ENDKEY # curl " http://10.3.3.62:8091/couchBase/default/_all_docs?startkey=%22simple%22&endkey=%22simple%22&skip=0&include_docs=true&limit=6 " {"total_rows":16602312,"rows":[ {"id":"simple","key":"simple","value":{"rev":"14-0003547b259ab3170000000000000000"},"doc":{"meta":{"id":"simple","rev":"14-0003547b259ab3170000000000000000","expiration":0,"flags":0},"json":{"click":"to edit","new in 2.0":"there are no reserved field names"}}} ] } this looks like same issue as MB-6442

Aleksey Kondratenko added a comment - 10/Sep/12 4:51 PM Are you sure you have everything latest? It works here: http://i.imgur.com/imaYv.png

Aleksey Kondratenko added a comment - 10/Sep/12 4:58 PM Hm. Looks like some issue somewhere. Works on my cluster_run (v8 and icu is from distro) but does _not_ when I install our package

Aleksey Kondratenko added a comment - 10/Sep/12 5:01 PM Hm. Same stuff but with \uFFFD which _is_ assigned unicode char works

Farshid Ghods added a comment - 10/Sep/12 5:04 PM manifest <?xml version="1.0" encoding="UTF-8"?> <manifest>   <remote name="apache" fetch=" git://github.com/apache/ "/>   <remote name="couchbase" fetch=" git://github.com/couchbase/ "/>   <remote name="erlang" fetch=" git://github.com/erlang/ "/>   <remote name="membase" fetch=" git://github.com/membase/ "/>   <project path="otp" name="otp" remote="erlang"            revision="refs/tags/OTP_R14B04" />   <project path="icu4c" name="icu4c" remote="couchbase"            revision="26359393672c378f41f2103a8699c4357c894be7" />   <project path="snappy" name="snappy" remote="couchbase"            revision="5681dde156e9d07adbeeab79666c9a9d7a10ec95" />   <project name="v8" path="v8" remote="couchbase"            revision="refs/tags/3.9.7" />   <project name="gperftools" path="gperftools" remote="couchbase"            revision="master" />   <project name="pysqlite" path="pysqlite" remote="couchbase"            revision="master" /> </manifest> is this what you are also building from ?

Aleksey Kondratenko added a comment - 10/Sep/12 5:05 PM Looks like some libicu difference. Not sure it's bug however. We're trying to compare string with with char \uFFFF. And that char is not officially registered in unicode. Still worth a quick look. On UI side using this char to match 'everything with given prefix' is clearly broken anyway (ignores unicode chars with code greater than FFFF) and I'll fix it. Feel free to close or assign back to me, Filipe.

Aleksey Kondratenko added a comment - 10/Sep/12 5:07 PM Farshid, I'm not building icu, snappy or v8. My understanding is nobody does that on dev boxes

Aleksey Kondratenko added a comment - 10/Sep/12 5:13 PM BTW, what's right way to query all keys with given prefix? I was thinking about instead of appending this junk to just increment last char, but apparently this may disappoint libicu as well. Unless we upgrade to a version that we know can handle any unicode code point, even unregistered.

Farshid Ghods added a comment - 10/Sep/12 5:15 PM Aliaksey, yes. i am asssuming all developers build v8 and icu based on what is mentioned in the extenral-override dependency. if that is not the case I need to send an email to db_dev and mgmt_dev to confirm that

Aleksey Kondratenko added a comment - 10/Sep/12 5:24 PM I can bet quite a lot that nobody is doing that (well except and _maybe_ osx folks)

Filipe Manana added a comment - 11/Sep/12 3:40 AM _all_docs uses raw collation, there's no ICU or V8 involved when querying _all_docs (unlike regular views). And \uFFFF is not rejected because the ejson decoder accepts it as valid, and so does Python's json decoder and node js, example: Python 2.7.1 (r271:86832, Jun 16 2011, 16:59:05) [GCC 4.2.1 (Based on Apple Inc. build 5658) (LLVM build 2335.15.00)] on darwin Type "help", "copyright", "credits" or "license" for more information. >>> import json >>> json.loads('"\uFFFF"') u'\uffff' I don't see a problem here in the view engine.

Aleksey Kondratenko added a comment - 11/Sep/12 10:39 AM Well from message above I see that view engine thinks end key is less then start key, which is clearly wrong. Moreover this works when I run locally built version (icu-config --version reports 4.8.1.1) and it clearly does not work if I run our binary package.

Farshid Ghods added a comment - 12/Sep/12 3:36 PM not sure why this ticket is assigned to me ? per Filipe comments this is not using v8 or icu code and yet we see this issue. moving this to 2.0 and keeping this as critical for now

Aleksey Kondratenko added a comment - 24/Sep/12 8:59 AM Filipe, you need to take a look. we're passing: start_key = "simple" and end_key = "simple\uffff" and _your_ code is incorrectly returning view_parse_error telling us end_key is _smaller_ then start_key. Which is clearly wrong. I'm sure it's something related to ICU. Particularly it _works_ on my box if I build everything myself, but it _does not work_ if I install our binary package. Particular question is: * should we upgrade libicu? You reply (I think premature) means it's not icu related, if that's correct, then true reason for this failure is elsewhere and we need to understand that. * alternatively how do I request all keys with given prefix (appending 0xffff is clearly wrong w.r.t. HJK chars with codes greater than that)

Volker Mische added a comment - 04/Oct/12 9:28 AM The problem is ICU, the builds are with 4.4 and in 4.6 things changes. From 4.6 on you can use \uFFFFF as the highest weighted character. See (2.) in section "Changes in ICU code" of [1]. I suggest to upgrade our builds to the latest stable version of ICU. Another things is that when querying with the raw collation (as it is the case for _all_docs) the query parameters should also use raw collation and not the Unicode one. I'll have a look and see how hard it will be to fix it. Nonetheless I'd still upgrade ICU. [1] http://site.icu-project.org/design/collation/icu-4-6-changes

Tommie McAfee added a comment - 04/Oct/12 12:15 PM Interesting if you put \uFFFF on startkey and set descending=true, it seems to work: curl " http://10.3.3.59:8091/couchBase/default/_all_docs?startkey=%22sim%EF%BF%BF%22&endkey=%22sim%22&skip=0&include_docs=true&descending=true&limit=6&_=1349370462492 " {"total_rows":5373574,"rows":[ {"id":"simple","key":"simple","value":{"rev":"46-000aa59a2bb142860000000000000000"},"doc":{"meta":{"id":"simple","rev":"46-000aa59a2bb142860000000000000000","expiration":0,"flags":0},"json":{"click":"to edit","new in 2.0":"there are no reserved field names"}}} ] }

Volker Mische added a comment - 04/Oct/12 6:15 PM Simply upgrading ICU doesn't solve the full problem, I'll fix it differently. Though ICU should be updated nonetheless sooner or later. @Tommie: That is expected. The problem is that the startkey and endkey is checked if endkey is smaller than the startkey. That throws the given error. If you reverse the results it of course does work as then the endkey needs to be smaller than the startkey.

Tommie McAfee added a comment - 04/Oct/12 6:39 PM Thanks Volker, I'm just wondering if doing query other way is possible workaround? Agree fix is much better, but if we can't do anything in 2.0 scope probably changing query string in client would be better than autocomplete failing all together.

Farshid Ghods added a comment - 04/Oct/12 7:01 PM Hi Volker, we can upgrade icu library post 2.0 as much as possible to avoid upgrading any part of our server. it would be good to find a cheaper fix in the UI as suggested if possible

Volker Mische added a comment - 04/Oct/12 7:04 PM Hi Farshid, Tommie, as mentioned in my previous comment. I'll provide a fix that solves the whole issue that won't need an ICU upgrade.

Aleksey Kondratenko added a comment - 04/Oct/12 7:05 PM Please, note that Tommie's proposal is not ok with me. _Everybody else_ have fixed ICU version. 'Fixing' UI in order to rely on broken behavior is very very bad idea in my strong opinion. Alternatively we can consider using another 'max' character or even better find solution that reliably handles CJK chars with unicode code points greater than 0xffff.

Volker Mische added a comment - 04/Oct/12 7:47 PM Not fully tested yet if it breaks something else, but it seems to fix the problem: couchdb commit: http://review.couchbase.org/21359 testrunner test commit for verification: http://review.couchbase.org/21360

Filipe Manana added a comment - 05/Oct/12 9:37 AM Volker / Farshid, the fix, ported from Apache, only fixes it for the verification of startkey/endkey when querying _all_docs. For regular views, ICU upgrade is necessary.

Volker Mische added a comment - 05/Oct/12 9:44 AM - edited That's true, I'm fixing a slightly different issue. From the UI perspective you wouldn't be able to use \uFFFF for trailing the search, but you would need to use some other high character until ICU gets upgraded.

Dipti Borkar added a comment - 11/Oct/12 4:35 PM changes on gerrit

Thuan Nguyen added a comment - 12/Oct/12 8:49 PM Integrated in multi-nodes-2.0.x-windows-64-backup-cli #15 (See [ http://qa.hq.northscale.net/job/multi-nodes-2.0.x-windows-64-backup-cli/15/ ])      MB-6591 : Test cases to make sure the fix is valid (Revision 8535782ee0ffe7305ebbf3dc00df530b2350edf0)      Result = UNSTABLE IrynaM : Files : * pytests/viewtests.py * conf/py-viewquery.conf

Thuan Nguyen added a comment - 14/Oct/12 12:05 AM Integrated in github-couchdb-preview #516 (See [ http://qa.hq.northscale.net/job/github-couchdb-preview/516/ ])      MB-6591 : Use raw collation for _all_docs query params (Revision fd03c3a583904440d34bbc8033a826cbec446357)      Result = SUCCESS Farshid Ghods : Files : * src/couchdb/couch_httpd_view.erl * src/couch_index_merger/src/couch_view_merger.erl * src/couchdb/couch_httpd_db.erl * share/www/script/test/view_collation_raw.js * src/couchdb/couch_btree.erl * share/www/script/test/all_docs.js

Volker Mische added a comment - 14/Oct/12 5:14 AM - edited The problem from the second comment [1] is fixed. You won't get an error if you do a curl " http://10.3.3.62:8091/couchBase/default/_all_docs?startkey=%22simple%22&endkey=%22simple%EF%BF%BF%22&skip=0&include_docs=true&limit=6 " Though I think we still might have the problem with the \uFFFF, as the queries are on _all_docs and _all_docs uses raw collation. Please check whether \uFFFF really returns the expected range or not. If it doesn't work a high byte value might help, like e.g. %7F [1] http://www.couchbase.com/issues/browse/MB-6591?focusedCommentId=38353&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-38353

Thuan Nguyen added a comment - 16/Oct/12 4:07 AM Integrated in single-node-2.0.x-windows7-64-view #18 (See [ http://qa.hq.northscale.net/job/single-node-2.0.x-windows7-64-view/18/ ])      MB-6591 : Test cases to make sure the fix is valid (Revision 8535782ee0ffe7305ebbf3dc00df530b2350edf0)      Result = UNSTABLE IrynaM : Files : * conf/py-viewquery.conf * pytests/viewtests.py

Thuan Nguyen added a comment - 17/Oct/12 12:58 PM Integrated in multi-nodes-windows-64-viewtest #20 (See [ http://qa.hq.northscale.net/job/multi-nodes-windows-64-viewtest/20/ ])      MB-6591 : Test cases to make sure the fix is valid (Revision 8535782ee0ffe7305ebbf3dc00df530b2350edf0)      Result = SUCCESS IrynaM : Files : * conf/py-viewquery.conf * pytests/viewtests.py

Iryna Mironava added a comment - 23/Oct/12 6:15 AM verified build-1884