472764 - gloda full-text search always uses SQLite's porter stemmer without regard to effective locale, etc.

Status: RESOLVED FIXED

(MailNews Core :: Database, defect)

Description

[Andrew Sutherland [:asuth] (he/him)]

The SQLite FTS3 engine supports per-column tokenizers.  We currently always use the Porter stemmer.  This is only appropriate for messages written in English.

It's a tricky situation because each column only gets one tokenizer, and there need not be a correlation between the user's locale and the languages they receive messages in.  Using a unicode tokenizer might get us pretty far, but stemming is also very useful for English.  The good news is that sqlite at least allows easily pluggable tokenizers.

Marking this blocking pref-ing gloda on, at least until we figure out what our game-plan is.  (Once we have a plan, it may no longer be blocking.)

Comment 1

[Andrew Sutherland [:asuth] (he/him)]

Good news, everyone!  Apparently we already have a fairly clever tokenization function added for the spam filter!  nsISemanticUnitScanner!  Examples of results on the adding bug at 176528#c13 and the subsequent comment.  The logic seems consistent with my frantic wikipedia & friends research.  The usage is also compatible with the FTS3 tokenizer API.

So, that gets us what we want, except that we would also really like to use the built-in porter stemmer for English.  As such, I propose we implement 2 tokenizers:

1) nsISemanticUnitScanner-based tokenizer.  This is the default.

2) English-biased tokenizer, for use when the user's locale is en_*.  It sounds wacky, but the idea is that we conceptually apply the porter stemmer to the output of every word emitted by the nsISemanticUnitScanner tokenizer.

My first instinct was to say "apply the porter stemmer only if we think the message contains only english."  Unfortunately, this can result in making some queries intuitively impossible to formulate.  For example, let's say I have an indexed string containing the word "dancing" and another word with an accent, causing the porter stemmer to not be used.  Then the term will be indexed as "dancing".  But when I go to formulate a query for "dancing", because my query doesn't have anything to convince the tokenizer that it's not English, it will stem my query term to "danc".  Bad news.

So, the English-biased tokenizer will use the porter stemmer against anything that the porter stemmer would stem.  (The porter stemmer only stems [a-zA-z], punting to copying otherwise.)  This has the potential to result in false positives where a non-english word that lacks accents will be stemmed to collide with other words (possibly of a different language), resulting in user confusion.  It seems like there has to be an awesome and amusing example of this, but none spring to mind.

Note: I use the words "conceptually" and "would" because there may be efficiency dancing going on, not to mention odd implementation choices required.  Namely, the tokenizer actually gets its strings as utf-8, but nsISemanticUnitScanner consumes PRUnichars.  For efficiency reasons we might want to avoid crossing the conversion bridge until we have to.  At the same time, the porter stemmer implementation has some private state we probably don't want to much with.  A 'minimal' implementation might always run the porter stemmer, but double-check the input stream and re-process any text span consumed by the porter stemmer that had 0x80 or higher in it.  The book-keeping would be fairly reasonable, and would allow for on-demand instantiation of the semantic scanner.

dmose, if you would comment on the sanity of this plan, it would be appreciated.  Assuming it is deemed sane, we can remove the blocking status of this bug.  Whether the semantic scanner implementation targets beta 2 or beta 3 I suppose would depend on whether we want to try and put the implementation in mozilla-central or not.  There is no technical reason for the code to live in mozilla-central; the tokenizer registration can all be accomplished without any changes to the storage component.  I am inclined to put the code in comm-central, the most noble reasons being that it involves arbitrary heuristics and the test code makes the most sense to exist in comm-central.  (No sense slowing down mozilla-central for a thunderbird feature, etc.)

Comment 2

[Dan Mosedale (:dmosedale, :dmose)]

Wow, great find!  I'd completely forgotten about that code.  This seems sane to me, but I'd like to hear thoughts from other folks whom I've added to the CC list (most especially smontagu, as he's likely to have the lion's share of expertise).  I don't have a terribly strong opinion on where this code would want to live; perhaps sdwilsh and myk have thoughts there.

Comment 3

[Simon Paquet [:sipaq]]

What is the ETA for this bug? 

Tomorrow at 23:59 PST is our string freeze and it would be great if this blocking bug could be fixed by that time as mentioned in https://wiki.mozilla.org/Thunderbird/Driver_Meetings/2009-02-03

Comment 4

[Bryan Clark (DevTools PM) [:clarkbw]]

Updating the status whiteboard as this doesn't actually affect strings.  I'm going to add the relnote keyword for now because if this lands I have the feeling we'll want to make a note about the behaviour effecting locales.

Comment 5

[Bryan Clark (DevTools PM) [:clarkbw]]

nm for now, bumping off the b2 list and aiming for b3

Comment 7

[dynamis (Tomoya ASAI)]

in a sentence:
We cannot use nsISemanticUnitScanner (at least as is) and we should use some other library or implement (port) tokenizer (or indexer as a whole) for sqlite.

and details:
nsISemanticUnitScanner is not so bad for spam mail filter but as for full text search, we cannot use it (at least for Japanese and some other non-space-separated languages).
# actually nsISemanticUnitScanner is not good enough for such languages too
# I hope you already know and so you write "-based".

As a tokenizer for full text search, there are following problems with nsISemanticUnitScanner at least:
# examples are taken from bug 176528#c14
 1. full-width symbols are not split well
    ex. [”（AIM）。] should be split as [”] [（] [AIM] [）] [。]
 2. some tokens in (at least) Japanese hiragana/katakana and Korea (both use phonogram) are not split well.
    ex. [のすべてを] should be split as [の] [すべて] [を]
    ex. [여러분이] should be split as [여러분] [이]
        [5시까지] should be split as [5] [시] [까지]
    # there must be other languages who have same problem
 3. simple uni-gram may not fast enough
    in general, we use bi-gram, tri-gram or m.n-gram for fast search

note: Korea is not always written with spaces but acutually usually without spaces and in that case almost tokens will be garbage.

Third problem is performance issue (will be serious for GB data) but first/second problems will cause (quite many) drop of search words and are really critical. With current nsISemanticUnitScanner, full text search will only be useful for space-separated non multi-byte languages.
# even as for space-separated non multi-byte lang, some probably have problems

So, we should use much better tokenizer for full-text search.
# of course better to have for spam filter too, not only full-text search
To solve performance issue too, we should reuse some production level library I think.

Some fine implementation example of well-known production level full text search engines are for example:
http://hyperestraier.sourceforge.net/
http://lucene.apache.org/
http://qwik.jp/senna/
http://projects.netlab.jp/rast/

But as far as I read some documents I cannot find how to use them with sqlite.
I don't know yet if we can reuse them for tokenizer of sqlite's fts3 or not.
I'm also not sure we can solve all the problem just using custom tokenizer for fts3 or we should customize fts3 indexer itself.
# some are used with mysql, postgresql etc or standalone but not with sqlite...
# this means we cannot reuse, just I haven't find howto document with sqlite yet

I cannot say which algorithm/library we should use but I hope this help you.

Comment 8

[Andrew Sutherland [:asuth] (he/him)]

The FTS3 tokenizer implementation is fairly pluggable (see the interface definition at http://www.sqlite.org/cvstrac/fileview?f=sqlite/ext/fts3/fts3_tokenizer.h&v=1.2).  If using something like an overlapping bi-gram tokenizer, we might have to help it out a little bit on the queries to get the desired results, but nothing that we should need to modify the core for.

We could certainly use something like the bi-gram tokenizer patch on bug 414102, but augmented to only do bi-gram tokenization when in the CJK character spaces.  To deal with characters that should not be part of a bi-gram, we could simply transform single-CJK-character queries into wildcarded prefix queries (ex: "の*" for the query "の").  It sounds like this might achieve a more correct result.

What are you thoughts on this strategy?  I would be very happy to have a better solution, but from a resource perspective, I will not be able to implement a better solution in the beta 3 timeframe, so I need a fallback plan if someone else doesn't step up with a better implementation.  You seem very informed and interested in this problem, perhaps you would like to take this on? :)

In terms of other full-text engines, for the current time-frame, I suspect it's only feasible to port tokenizers from other implementations.  Unfortunately, we also need to keep licensing issues in mind.  Both hyperestraier and senna are LGPL, which I think means we cannot use them unless the authors/copyright holders of the code we port consent to have the code re-licensed under the MPL as well.

Thanks again for your input!  As you may surmise, (human) languages are not my forte.

Comment 9

[Andrew Sutherland [:asuth] (he/him)]

(I am not actively working this yet, dropping assignment so as to encourage other people with an interest to consider working it...)

Comment 10

[Dan Mosedale (:dmosedale, :dmose)]

Re LGPL dual-licensing, Mozilla has convinced other projects to do this before, so if porting one of those tokenizers seems like the best option, we could certainly ask.

Comment 11

[David :Bienvenu]

-> asuth - since it's not ASSIGNED, we hope people will figure out they can help, but better not to be an unowned blocker.

Comment 12

[dynamis (Tomoya ASAI)]

Attachment: bigram tokenizer (based on attachment 321769)

simple/poor bi-gram tokenizer patch based on attachment 321769 [details] [diff] [review] of bug 414102.
just to check if bi-gram tokenizer with fts3 will be useful or not

Comment 13

[dynamis (Tomoya ASAI)]

As Andrew say, IMHO we should support simple N-gram first to make gloda works correctly even if it's not have high performance. And then we should work for faster tokenizer (and other parts not only tokenizer)
# much faster than existing search without gloda

In thunderbird, target data size will be (currently) ~XX GB, not XX TB and we need not support parallel machine indexer nor complex optimization just to get a little faster indexer/searcher or a little smaller database size.

I tested simple bi-gram tokenizer based on the patch on bug 414102.
Tha bi-gram patch itself was too poor (not supporting Hiragana/Katakana/Korean etc!) and I just commented out if statement for puncutations to avoid it.
# still too poor, just to check if simple bi-gram is acceptable or not

Then I can search Japanese words (including single char).
As for the speed, in case the search results will be few items, it takes a few seconds in the exptoolbar to show results in the new tab.
 - about 1,2 secondes for "浅井", "井上", "東京", "開発", "wordcamp" etc
On the other hand in case the search results will be hundreds of mails, I have to wait long time.
 - about 8 secondes for "bug"
 - about 18 secondes for "mozilla" - too slow :(
# with 1.8GM mail data on Intel iMac Core 2 Duo 3.06GHz

Not really fast but this speed is similar to current search with porter tokenizer which don't support multibyte chars.
# And this is maybe also problem of exptoolbar front-end, which should get
# search results incrementally but maybe currently not (I guess).

When we will support incremental search against message body within the thunderbar like awesomebar of Fx, it may be the problem and we should consider faster one but we should accept at first.
# we have too many thing around gloda before tb3...

note: current problem with the patch on bug 414102:
 - should based on porter tokenizer, not on simple one
   # at least Japanese and other languages which often use English words
   # within their language
 - should detect and skip invalid byte as utf-8
 - should define correct delimiter for multibyte chars
   # currently, all hiragana/katakana/Korean etc are treated as delimiters!
 - should support search words including delimiters like "utf-8", which
   include "-" and cannot search from exptoolbar now
   # this may be the bug of front-end, not tokenizer but not sure yet

Even when we use simple N-gram, there are many complex problems and it may have to import some robust/fast one finally. But not sure yet now and should start with simple N-gram...


 - p.s.
I'm really interested in this problem since I never want to say "Thunderbird 3 is great but not for Japanese..." to Japanese users.
# I'm technical marketing/evangelist staff in Mozilla Japan
I read some documents/papers/codes about full-text search but still not enough good to write custom fine tokenizer in this time frame. I'll do my best for this bug but not accepting this bug so far, sorry.

Comment 14

[dynamis (Tomoya ASAI)]

note: to test bi-gram tokenizer based on the simple tokenizer, you need edit
Shredder.app/Contents/MacOS/modules/gloda/datastore.js line 909 (or around)
       let createFulltextSQL = "CREATE VIRTUAL TABLE " + aTableName + "Text" +
-        " USING fts3(tokenize porter, " +
+        " USING fts3(tokenize simple, " +
         [(coldef[0] + " " + coldef[1]) for each

if you need binary of sqlite dynamic library, you can use (replace with):
http://people.mozilla.com/~dynamis/thunderbird/sqlite3/bigramtokenizer/1st/
# only tested with Mac (libsqlite3.dylib)

Comment 15

[Andrew Sutherland [:asuth] (he/him)]

Thank you for the testing.  I am not too surprised that exptoolbar is being slow, especially in cases with lots of results.  exptoolbar doesn't bother to use a "limit" on its query and gloda returns the results all at once (a current gloda limitation).  This turns out extra poorly, as you get to see.  I would not be surprised if Thunderbird even becomes non-responsive for a few seconds as a massive garbage collection triggers.

I hope to have up a revised patch on bug 474701 soon that exposes the new, simpler gloda search interface with much better performance.

Comment 16

[Andrew Sutherland [:asuth] (he/him)]

Not sure what to do about this bug and beta 3.  Although it does not require changes to the user interface (and so in theory could slip to rc1), we cannot have gloda-search replace quick-search for all intents and purposes if it majorly regresses the ability to search CJK.

Only enabling gloda search for english or non-CJK locales for beta 3 is problematic because it seems confusing to have beta 3 have major differences in apparent feature sets for different locales.  On the other hand, I don't know what else to do short of fairly serious slippage; I expect this to take at least a week of solid work, and I already have other things with higher priorities at serious risk.

Comment 17

[Ludovic Hirlimann [:Usul]]

Would be nice to have land this soon enough so we can get testing from people using non latin charsets.

Comment 18

[Dan Mosedale (:dmosedale, :dmose)]

After discussion with drivers, we decided that while it would be exceedingly unfortunate to not have good support for locales in Gloda in 3.0, we'd still rather get good GloDa support into the hands of some people rather than no people at all, so we wouldn't block on this bug if it were the last bug standing.

Comment 19

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

(In reply to comment #18)
> After discussion with drivers, we decided that while it would be exceedingly
> unfortunate to not have good support for locales in Gloda in 3.0, we'd still
> rather get good GloDa support into the hands of some people rather than no
> people at all, so we wouldn't block on this bug if it were the last bug
> standing.

I'm guessing this should probably be release-noted for TB3 - searching (and trying to make use of faceted search / gloda) in UTF-8 languages is a very natural thing to do for users of those languages (e.g. zh_CN, zh_TW and many more).

cc'ing Support & Marketing gurus...

Comment 20

[Wayne Mery (:wsmwk)]

In which case, if the search widget combining gloda search and quick search of asuth's tryserver build is carried forward, then for certain localized builds do we want the default search to be quick search and not gloda search?

OT but I'm not sure there's a bug/checkin to blame: I'm not sure I am keen on having the two searches combined for my workflow (extra clicks, etc), although it definitely simplifies the UI which is good for most users

Comment 21

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

(In reply to comment #20)
> In which case, if the search widget combining gloda search and quick search of
> asuth's tryserver build is carried forward, then for certain localized builds
> do we want the default search to be quick search and not gloda search?

Not even necessary to just be localized builds - I had been trying to search Chinese characters in emails even in the en_US localization.

Comment 22

[Makoto Kato [:m_kato]]

Attachment: WIP v0.9

Add FTS3 module to search CJK text.  Because we cannot modify sqlite3 code (see bug 414102 commnet #4).

Comment 23

[Andrew Sutherland [:asuth] (he/him)]

Woohoo!  Thank you, Makoto.  I need to add some unit tests for the gloda msg_search.js file, and then I think we can also use that as a basis for testing the indexer.  If you can come up with some simple test examples for strings we should be able to put in the message and then queries that should (and should not) match those messages, that would be also be very helpful.

I need to quickly try and finish up some beta-4 related things for the next few days, but I should be able to get to this after that and I think we definitely want to get this into 3.0 rc1.  Re-nominating for 3.0 blocker since your work has now made this feasible.

Comment 24

[Andrew Sutherland [:asuth] (he/him)]

(In reply to comment #23)
> msg_search.js file, and then I think we can also use that as a basis for
> testing the indexer.  If you can come up with some simple test examples for

Er, I mean tokenizer.

Comment 25

[Makoto Kato [:m_kato]]

Also, I will post to try-server after I test mac and linux.

This patch has unnecessary ifdef XP_WIN since I test on Windows only now.

Comment 26

[Makoto Kato [:m_kato]]

Attachment: WIP v0.91

Comment 27

[Makoto Kato [:m_kato]]

Also, It is necessary to update schema version.

Comment 28

[Makoto Kato [:m_kato]]

Attachment: patch v1

add test.
After testing on CentOS 5, I will send a review...

Comment 29

[David Ascher (:davida)]

Getting some resolution of this issue for TB3 is a blocker.

Comment 30

[David :Bienvenu]

Comment on attachment 401868 [details] [diff] [review]
patch v1

Thx for doing this!

Andrew is going to need to look at this, though he's on vacation at the moment.

I wonder if the sqlite code should be part of the sqlite lib, though I understand it might be hard for us to get that through mozilla-central at the moment.

The license for +++ b/mailnews/extensions/fts3/Makefile.in

looks to be old - it's 1998.

This might be a good thing to generate try server builds for, if we have testers available to try it out.

Comment 31

[Philippe M. Chiasson (:gozer)]

Submitted a try build, tagged 'bug472764-att401868'.

Comment 32

[Shawn Wilsher :sdwilsh]

(In reply to comment #30)
> I wonder if the sqlite code should be part of the sqlite lib, though I
> understand it might be hard for us to get that through mozilla-central at the
> moment.
It won't likely be hard to get an updated SQLite into mozilla-central.  I'll be sure to be quick on reviews!

Comment 33

[David :Bienvenu]

(In reply to comment #32)

> It won't likely be hard to get an updated SQLite into mozilla-central.  I'll be
> sure to be quick on reviews!

Cool, would it make sense to add the sqlite parts of this patch to sqlite.c? I'm not sure what all else that would entail w.r.t. exporting symbols, etc.

Comment 34

[David Ascher (:davida)]

sdwilsh: all the way back into 1.9.1?  

Given our past experience in the delays in getting simple changes back from trunk to 1.9.2 to 1.9.1, I think for this bug it'd be good to go with a tb-specific patch first, given our very short timelines, and a parallel m-c patch.

Comment 35

[David :Bienvenu]

speaking from almost complete ignorance, it looks like this would be just adding the contents of a file to sqlite.c (which is already the concatenation of a bunch of files). But a parallel m-c trunk patch and then fixing this when we move to the trunk probably is a lot easier.

Comment 36

[Shawn Wilsher :sdwilsh]

(In reply to comment #33)
> Cool, would it make sense to add the sqlite parts of this patch to sqlite.c?
> I'm not sure what all else that would entail w.r.t. exporting symbols, etc.
We don't take changes to the SQLite source.  We only take files from SQLite releases.

(In reply to comment #34)
> sdwilsh: all the way back into 1.9.1?
We'd have to ship it in a 1.9.2 beta first, but that's coming up soon.  I'm not sure I'll have cycles to do the SQLite upgrade, but I'll be quick with reviews.

Comment 37

[Philippe M. Chiasson (:gozer)]

Failed on Win32 with:

e:\buildbot\sendchange-win32-hg\mozilla\mailnews\extensions\fts3\src\fts3_tokenizer.h(27) : fatal error C1083: Cannot open include file: 'sqlite3.h': No such file or directory

Windows Log: <http://tinderbox.mozilla.org/showlog.cgi?log=ThunderbirdTry/1254154576.1254159607.21523.gz>

Mac Build: <http://s3.mozillamessaging.com/build/try-server/2009-09-28_09:[e-mail]-bug472764-att401868/[e-mail]>

Linux Build: <http://s3.mozillamessaging.com/build/try-server/2009-09-28_09:[e-mail]-bug472764-att401868/[e-mail]2>

Comment 38

[Makoto Kato [:m_kato]]

Attachment: patch v2

fix build issue.

If I can use http://hg.mozilla.org/try-comm-central/ as try server of comm-central tree, I will push it.

Comment 39

[David Ascher (:davida)]

Makoto-san: The page https://wiki.mozilla.org/Thunderbird/Infrastructure/TryServer describes how to get a certificate to use with the try server.

In the meantime, I've spun up some try server builds with your second patch, tagged 472764v2.

They should show up on http://tinderbox.mozilla.org/showbuilds.cgi?tree=ThunderbirdTry

Comment 40

[David Ascher (:davida)]

os x build failed for mysterious reasons, so trying again.

Comment 41

[David Ascher (:davida)]

windows build: http://s3.mozillamessaging.com/build/try-server/2009-09-28_21:[e-mail]-472764v2/[e-mail]

linux build: http://s3.mozillamessaging.com/build/try-server/2009-09-28_21:[e-mail]-472764v2/[e-mail]2

mac build will be a few hours.

Comment 42

[David Ascher (:davida)]

The next batch worked fine:

linux: http://s3.mozillamessaging.com/build/try-server/2009-09-28_22:[e-mail]-cjktry3/[e-mail]2


mac: http://s3.mozillamessaging.com/build/try-server/2009-09-28_22:[e-mail]-cjktry3/[e-mail]

windows: http://s3.mozillamessaging.com/build/try-server/2009-09-28_22:[e-mail]-cjktry3/[e-mail]
or http://s3.mozillamessaging.com/build/try-server/2009-09-28_22:[e-mail]-cjktry3/[e-mail]

Comment 43

[Makoto Kato [:m_kato]]

Attachment: patch v3

patch v2 has no xpt file for tokenizer, so I add it to package-static.

Comment 44

[Makoto Kato [:m_kato]]

(In reply to comment #39)
> Makoto-san: The page
> https://wiki.mozilla.org/Thunderbird/Infrastructure/TryServer describes how to
> get a certificate to use with the try server.

Thanks, David.  I file a request to bugzilla.

Comment 45

[David Ascher (:davida)]

spun up new try server builds, labeled 'cjkv3' or something like that =)

Comment 46

[David Ascher (:davida)]

v3 patch failed on windows:

http://tinderbox.mozilla.org/showlog.cgi?log=ThunderbirdTry/1254324631.1254328669.30206.gz&fulltext=1

======= making ./nsldif32v60.dll
link -MANIFEST:NO -OUT:"nsldif32v60.dll" -DEBUG -OPT:REF -MAP -nologo -DLL -SUBSYSTEM:WINDOWS  -SUBSYSTEM:CONSOLE    -out:"nsldif32v60.dll" ./line64.obj  -DEF:e:/buildbot/sendchange-win32-hg/mozilla/directory/c-sdk/ldap/libraries/msdos/winsock/nsldif32.def  
e:/buildbot/sendchange-win32-hg/mozilla/directory/c-sdk/ldap/libraries/msdos/winsock/nsldif32.def(39) : warning LNK4017: DESCRIPTION statement not supported for the target platform; ignored
   Creating library nsldif32v60.lib and object nsldif32v60.exp
LINK : fatal error LNK1104: cannot open file 'nsldif32v60.lib'
make[6]: Leaving directory `/e/buildbot/sendchange-win32-hg/mozilla/objdir/directory/c-sdk/ldap/libraries/libldif'
make[5]: Leaving directory `/e/buildbot/sendchange-win32-hg/mozilla/objdir/directory/c-sdk/ldap/libraries'
make[4]: Leaving directory `/e/buildbot/sendchange-win32-hg/mozilla/objdir/directory/c-sdk/ldap'
make[3]: Leaving directory `/e/buildbot/sendchange-win32-hg/mozilla/objdir/directory/c-sdk'
make[6]: *** [nsldif32v60.dll] Error 80
make[6]: *** Waiting for unfinished jobs....
make[5]: *** [export] Error 2
make[4]: *** [export] Error 2
make[3]: *** [export] Error 2
make[2]: Leaving directory `/e/buildbot/sendchange-win32-hg/mozilla/objdir'
make[1]: Leaving directory `/e/buildbot/sendchange-win32-hg/mozilla/objdir'
make[2]: *** [tier_app] Error 2
make[1]: *** [default] Error 2
make: *** [build] Error 2
program finished with exit code 2

Comment 47

[Dan Mosedale (:dmosedale, :dmose)]

That's an error linking LDAP, I wonder if there's some other tree coincidental with that try build that's causing it...

Comment 48

[Dan Mosedale (:dmosedale, :dmose)]

"tree bustage coincidental" is what I meant to say.

Comment 49

[David Ascher (:davida)]

just for fun, tried again.

Comment 50

[David Ascher (:davida)]

The try server is very brittle it seems. The latest run worked on windows & linux:

windows: http://s3.mozillamessaging.com/build/try-server/2009-09-30_12:[e-mail]-cjkv3try2/[e-mail]

and linux: http://s3.mozillamessaging.com/build/try-server/2009-09-30_12:[e-mail]-cjkv3try2/[e-mail]2

and the older successful mac build (also w/ 3rd patch): http://s3.mozillamessaging.com/build/try-server/2009-09-30_08:[e-mail]-cjkv3/[e-mail]

It'd be great if people could report whether these builds work a) with CJK corpora, and b) with non-CJK corpora)

Comment 51

[Gary Kwong [:gkw] [:nth10sd] (NOT official MoCo now)]

(In reply to comment #50)
> The try server is very brittle it seems. The latest run worked on windows &
> linux:
> 
> windows:
> http://s3.mozillamessaging.com/build/try-server/2009-09-30_12:[e-mail]-cjkv3try2/[e-mail]
> 
> It'd be great if people could report whether these builds work a) with CJK
> corpora, and b) with non-CJK corpora)

Just had some time (not much) to test on CJK searching on this en_US WinXP build. I had a test message with the "十" character (means "ten" in Chinese), gloda is on, the message is onDisk and has a gloda ID, but when I search all messages at the top, it still says "no messages match your search".

:(

ID of this tryserver build on WinXP:
Mozilla/5.0 (Windows; U; Windows NT 5.1; en-US; rv:1.9.1.4pre) Gecko/20090930 Shredder/3.0pre

Comment 52

[Makoto Kato [:m_kato]]

(In reply to comment #51)

> build. I had a test message with the "十" character (means "ten" in Chinese),
> gloda is on, the message is onDisk and has a gloda ID, but when I search all
> messages at the top, it still says "no messages match your search".

This implementation is bi-gram.  At least, 2 characters are necessary as search string.  So How about at least 2 character as search string??

If we allow 1 character, too, database size will becomes too big.

Comment 53

[Ludovic Hirlimann [:Usul]]

(In reply to comment #50)

> 
> It'd be great if people could report whether these builds work a) with CJK
> corpora, and b) with non-CJK corpora)

I've send a few emails to the contacts I made back in may to try the builds and give us some feedback on it.

Comment 54

[Makoto Kato [:m_kato]]

Attachment: patch v4

fix comment #51 (I tests on Japanese mail only)

Comment 55

[Makoto Kato [:m_kato]]

Attachment: patch v4.1

typo

Comment 56

[David Ascher (:davida)]

Setup try server builds w/ patch 4.1

Comment 57

[Makoto Kato [:m_kato]]

Attachment: patch v4.2

fix seamonkey installer code (maybe need *.xpt for tokenizer)

Windows
http://s3.mozillamessaging.com/build/try-server/2009-10-01_21:[e-mail]-1254456162/[e-mail]
http://s3.mozillamessaging.com/build/try-server/2009-10-01_21:[e-mail]-1254456162/[e-mail]

Linux
http://s3.mozillamessaging.com/build/try-server/2009-10-01_21:[e-mail]-1254456162/[e-mail]2

I will rebuild mac version (due to infra error?)

Comment 58

[Makoto Kato [:m_kato]]

Mac
http://s3.mozillamessaging.com/build/try-server/2009-10-02_02:[e-mail]-cjk-42/[e-mail]

Comment 59

[Ludovic Hirlimann [:Usul]]

Those builds seems to work correctly on ASCII latin usage.

Comment 60

[Makoto Kato [:m_kato]]

Attachment: patch v5

I got feedback about this patch.  A mixed content that has Japanese and ascii without break space causes that we may not search ascii word well.

Fixed binary is here.

Windows
http://s3.mozillamessaging.com/build/try-server/2009-10-08_02:[e-mail]-bug472764-v5/[e-mail]
http://s3.mozillamessaging.com/build/try-server/2009-10-08_02:[e-mail]-bug472764-v5/[e-mail]

Mac
http://s3.mozillamessaging.com/build/try-server/2009-10-08_02:[e-mail]-bug472764-v5/[e-mail]

Linux
http://s3.mozillamessaging.com/build/try-server/2009-10-08_02:[e-mail]-bug472764-v5/[e-mail]2

Comment 61

[Andrew Sutherland [:asuth] (he/him)]

Attachment: asuth cjk mods to go on top of the v5 patch

Here's a patch that goes on top of the v5 patch, it:

- Adds a lot of comments.  In order to verify the correctness of the code I needed to understand it, and I am nowhere near clever enough to keep all of the balls in the air in just my head... and my memory is certainly not good enough to remember it for next time if I did somehow manage it (caffeine?).

- Treats non-ASCII things in the generic script page like ASCII rather than punctuation/whitespace.  Renamed BIGRAM_ASCII to BIGRAM_ALPHA (and used that) accordingly.

- No longer emits single-character tokens (CJK or ALPHA), generally speaking (see next point).  The emission of single-character CJK tokens seems like it was probably an unintended bug.  Given that we don't specialize for anything in the general punctuation page except whitespace, this seems advisable.

- Loses the special logic in msg_search.js that tries to generate bi-grams.  The tokenizer is used on the queries too so there is no need to do this and we're likely to get burned if try.  The logic to add the wildcard in the case of a single CJK character actually did not work; it was depending on the prior bug where we generate only a single CJK character.  Namely if '&' is a CJK character, then the wildcarded string would end up as "&*".  This does not invoke the FTS3 wildcard logic at all; it has to be an unquoted term and the tokenizer has to eat a single token and leave '*' as the next character.  It just happened that "&*" tokenized down to "&" and if the source message had just a plain "&" somewhere, it would have put it in the database like that.  So to make the wildcard stuff work I changed the JS code to emit an unquoted character with the wildcard and special-cased the tokenizer to detect when it was in such a situation and emit the single CJK character in that case.  If someone were to have an e-mail message where the body or the subject was a single CJK character followed by an asterisk and absolutely no whitespace, we would get tricked in that case too, but I don't believe it to be likely.

- Added some more unit test cases.  These just test that the tokenizer works when multiple bigrams are involved.

Comment 62

[Andrew Sutherland [:asuth] (he/him)]

Attachment: v6 patch; v5 patch with asuth mods

Patch v5 is r=asuth with the changes from the previous patch.  I have folded my changes into v5 to produce v6.

Requesting review from Makoto to make sure my changes do not break or alter the intent of the patch.  And keeping the bienvenu sr request around.

I will spin try server builds as "cjk-v6" but will not be awake to see when they complete.  If someone posts the links here when they complete (or fail to complete), I won't complain.

Per discussion, the plan is currently _not to land the patch_ once the reviews are finished.  We want to try and time things with some other gloda patches I am hoping to land soon so that we avoid blowing away people's gloda databases multiple times in the span of a few days.

Comment 63

[Andrew Sutherland [:asuth] (he/him)]

Comment on attachment 406850 [details] [diff] [review]
v6 patch; v5 patch with asuth mods

(r=asuth :)

Comment 64

[Wayne Mery (:wsmwk)]

tryserver builds are done and ready for testing

http://s3.mozillamessaging.com/build/try-server/2009-10-16_23:[e-mail]-cjk-v6/[e-mail]2

http://s3.mozillamessaging.com/build/try-server/2009-10-16_23:[e-mail]-cjk-v6/[e-mail]

http://s3.mozillamessaging.com/build/try-server/2009-10-16_23:[e-mail]-cjk-v6/[e-mail]

http://s3.mozillamessaging.com/build/try-server/2009-10-16_23:[e-mail]-cjk-v6/[e-mail]

Comment 65

[Andrew Sutherland [:asuth] (he/him)]

uh, and I probably should have mentioned that if before using one of these builds you were using some of the other CJK builds, then you will need to blow away your global-messages-db.sqlite file in your profile directory.  (My changes do not bump the schema so this will not be done automatically unless you are coming from a nightly/beta4 build.)

Comment 66

[Gen Kanai [:gen]]

Channy, can you do some testing with the builds Wayne Mery created for Korean/Hangul?

Comment 67

[David :Bienvenu]

Comment on attachment 406850 [details] [diff] [review]
v6 patch; v5 patch with asuth mods

no need for temp var code here, since it's only used once:

+        let code = term.charCodeAt(0);
+        // Our tokenizer treats anything at/above 0x2000 as CJK for now.
+        if (code >= 0x2000)
+          magicWild = true;

so this can be simply:

let magicWild = term.length == 1 && term.charCodeAt(0) >= 0x2000)

in fact, magicWild is only used once, so the whole thing can be in-lined.

move decl of rv to where it's first used here:

+nsFts3Tokenizer::RegisterTokenizer(mozIStorageConnection *connection)
+{
+    nsresult rv;
+    nsCOMPtr <mozIStorageStatement> selectStatement;
+
+    rv = connection->CreateStatement(NS_LITERAL_CSTRING(

you're ignoring the first rv here: 

+    rv = selectStatement->BindStringParameter(0, NS_LITERAL_STRING("mozporter"));
+    rv = selectStatement->BindBlobParameter(1, (PRUint8*)&module, sizeof(module));
+    NS_ENSURE_SUCCESS(rv, rv);

either add an NS_ENSURE_SUCCESS(rv, rv); between the two statements, or don't assign to rv, whichever is more appropriate.

just return the value of selectStatement directly here, instead of assigning to rv:

+    PRBool hasMore;
+    rv = selectStatement->ExecuteStep(&hasMore);
+
+    return rv;

sr=me, with those things fixed.

Comment 68

[Andrew Sutherland [:asuth] (he/him)]

The plan is that I am now going to:

1) Make bienvenu's requested changes.
2) Remove the schema bumping.
3) Smoke-test that things work (still using the old porter stemmer tokenizer) for a gloda database created by a pre-CJK-aware build.
4) Land it.
5) Post about the landing as well as instructions on how to blow away your database to get the new CJK-aware-tokenizer.  This may also get some people out of the woodwork to further improve the tokenizer for non-English, non-CJK languages.  (Simple case/accent-folding seems reasonable and likely low risk assuming they are able to use libintl.)

Comment 69

[Andrew Sutherland [:asuth] (he/him)]

1-4 completed:

I did this as 1 push of 3 commits:

pushed Makoto's v5 patch:
http://hg.mozilla.org/comm-central/rev/4aab2fb7d9e1

pushed my modifications to the patch mainly adding comments:
http://hg.mozilla.org/comm-central/rev/1ef3fd69325a

pushed the review modifications and the removal of the schema bumping:
http://hg.mozilla.org/comm-central/rev/810cd547ef75

I'm off to post about this.

Marking resolved fixed since the fix is on the trunk.

Super thanks once again to Makoto Kato for the patch!

Comment 70

[Andrew Sutherland [:asuth] (he/him)]

I posted to m.d.a.t and m.d.l10n about this:
http://groups.google.ca/group/mozilla.dev.apps.thunderbird/browse_thread/thread/bf4fe06f0e0b20aa#

Once the nightlies are available we should probably reply to the message with an update.  Not sure if we should make a blog post about it or not after the nightlies are spun.  My message leaves it up to the reader to figure out how to locate their profile directory; that may be setting the bar too high.