WhitespaceTokenizer should tokenize on NBSP [LUCENE-6874]

[asfimport]

WhitespaceTokenizer uses Character.isWhitespace to decide what is whitespace. Here's a pertinent excerpt:

It is a Unicode space character (SPACE_SEPARATOR, LINE_SEPARATOR, or PARAGRAPH_SEPARATOR) but is not also a non-breaking space ('\u00A0', '\u2007', '\u202F')

Perhaps Character.isWhitespace should have been called isLineBreakableWhitespace?

I think WhitespaceTokenizer should tokenize on this. I am aware it's easy to work around but why leave this trap in by default?

---

Migrated from LUCENE-6874 by David Smiley (@dsmiley), resolved Nov 14 2015
Attachments: icu-datasucker.patch, LUCENE_6874_jflex.patch, LUCENE-6874.patch, LUCENE-6874-chartokenizer.patch (versions: 3), LUCENE-6874-jflex.patch, unicode-ws-tokenizer.patch (versions: 3)
Linked issues:

• WhitespaceTokenizer supports Java whitespace, should also support Unicode whitespace [LUCENE-5096] #6160
• Allow to define custom CharTokenizer using Java 8 Lambdas/Method references [LUCENE-6879] #7937

[asfimport]

Dawid Weiss (@dweiss) (migrated from JIRA)

Depends what you consider a trap.

A non-breakable whitespace could be a legitimate way to prevent two tokens from being separated if they need to be tokenized together. An example that comes to my mind is the special "zero-width" space or the hyphenation marker... which even on its own poses a problem [1]...

Ultimately it should be probably the question of whether we want to tokenize on "whitespace as in formatted text" or "whitespace as in logical codepoint units" and it doesn't apply to the WhitespaceTokenizer only, but to any tokenizer in general?

I think WhitespaceTokenizer should tokenize on this.

Seems like majority of people would want it to be tokenized, I agree. But if you change this then there is no way to go back to previous behavior. Currently it's relatively easy to wrap your input in a reader that replaces those problematic codepoints on the fly before they're fed to the tokenizer?

[1] https://www.cs.tut.fi/\~jkorpela/shy.html

[asfimport]

Adrien Grand (@jpountz) (migrated from JIRA)

So maybe we should solve this problem by adding some documentation?

[asfimport]

Dawid Weiss (@dweiss) (migrated from JIRA)

Any improvement to the docs that clarify what the software does would be great :)

[asfimport]

Uwe Schindler (@uschindler) (migrated from JIRA)

My personal opinion on this:

• The thing is called WhitespaceTokenizer, so it should do what the name says (split on isWhitespace).
• If we want something else, maybe provide a separate CharTokenizer implementation that also splits on NBSP

In general, whitespace tokenizer is not used for "classical" fulltext. For this type of text one would better use StandardTokenizer, ICU's Tokenizers or the language specific ones for Chinese or Japan. People using WhitespaceTokenizer are more those people which have very special types of fields, like a list of whitespace-separated tokens used for facetting or stuff like a list of product numbers. These types of tokens were always good to handle with WhitespaceTokenizer. If you wanted to keep your facet tokens together, you were able to use NBSP! So a change here would be a break for those apps :-)

So I would just update documentation to explain what this thing does (splitting on whitespace and not on spaces in general).

[asfimport]

Jack Krupansky (migrated from JIRA)

+1 for using the Unicode definition of white space rather than the (odd) Java definition. From a Solr user perspective, the fact that Java is used for implementation under the hood should be irrelevant. That said, the Javadoc for WhitespaceTokenizer#isTokenChar does explicitly refer to isWhitespace already.

The term "non-breaking white space" explicitly refers to line breaking and has no mention of tokens in either Unicode or traditional casual usage.

From a Solr user perspective, there is like zero value to having NBSP from HTML web pages being treated as if it were not traditional white space.

From a Solr user perspective, the primary use of whitespace tokenizer is to avoid the fact that standard tokenizer breaks on various special characters such as occur in product numbers.

One of the ongoing problems in the Solr community is the sheer amount of time spent explaining nuances and gotchas, even if they do happen to be documented somewhere in the fine print - no sane user reads the fine print anyway. No Solr user actually uses WhitespaceTokenizer directly - they reference WhitespaceTokenizerFactory, and then having to drop down to Lucene and Java for doc is way too much to ask a typical Solr user. Our collective goal should be to minimize nuances and gotchas (IMHO.)

In short, the benefits to Solr users for NBSP being tokenized as white space seem to outweigh any minor use cases for treating it as non-white space. A compatibility mode can be provided if those minor use cases are considered truly worthwhile.

Ugh... there are plenty of other places in doc for other tokenizers and filters that refer to "whitespace" and need to address this same issue, either to treat NBSP as white space or doc the nuance/gotcha much more thoroughly and effectively.

OTOH... an alternative view... having so many un/poorly-documented nuances and gotchas is money in the pockets of consultants and a great argument in favor of Solr users maximizing the employment of Solr consultants.

[asfimport]

Robert Muir (@rmuir) (migrated from JIRA)

I don't think we should make yet another definition of whitespace for java, there are already effectively 3 (Java isWhiteSpace(), java isSpaceChar(), and unicode whitespace): I think it would be better to just expose "unicode whitespace space" for situations like this.

java isSpaceChar is impractical, lets not even go there: it does not include controls such as tabs. so isSpaceChar('\t') == false and so on.

unicode whitespace is probably more useful and already well-defined:

http://icu-project.org/apiref/icu4j/com/ibm/icu/lang/UCharacter.html#isUWhiteSpace%28int%29
http://unicode.org/cldr/utility/list-unicodeset.jsp?a=[:White_Space=Yes:]

[asfimport]

Uwe Schindler (@uschindler) (migrated from JIRA)

In short, the benefits to Solr users for NBSP being tokenized as white space seem to outweigh any minor use cases for treating it as non-white space. A compatibility mode can be provided if those minor use cases are considered truly worthwhile.

As said before. If we want to change this we need a new Tokenizer with new name and a new Factory. Please don't add new matchVersion constants for that because this is a huge break. The Tokenizer does what it should and what is documentes: This is not a bug.

And still this holds: Users should prefer StandardTokenizer, the wide usage of WhitespaceTokenizer is caused by tons of example configs from earlier Solr days that uses WhiteSpaceTokenizer together with broken WordDestroyerFilter. This is indeed only useful for product numbers, but not fulltext.

[asfimport]

David Smiley (@dsmiley) (migrated from JIRA)

So maybe we should solve this problem by adding some documentation?

If the vast majority (like 90%+) of users that currently use WhitespaceTokenizer would want to tokenize on it, then I don't think documentation is sufficient at all. Documenting something most people would want to change is very very easy to overlook. That's what I call a trap; not that there might be some uses for the current behavior. Lucene should do what most users want it do do by default. As Jack said, the users of the search platform don't care what Java's definition of Character.isWhitespace is.

I propose WhitespaceTokenizerFactory have a flag for this, and that it default to consider NBSP a space based on Lucene's Version.

I get Uwe's point that there are other Tokenizers. But I disagree that WhitespaceTokenizer shouldn't be used for "classical full text". For example StandardTokenizer tokenizes on hypthen and thus foils some of the benefit of WordDelimiterFilter. Maybe ICUTokenizer is an answer; I haven't checked it's interaction with WDF. But why can't we just have a tokenizer that just tokenizes simply on all whitespace?

I'll have to see the links Rob just posted; I haven't read them yet.

[asfimport]

Uwe Schindler (@uschindler) (migrated from JIRA)

unicode whitespace is probably more useful and already well-defined

I would solve this issue by adding a ICUWhiteSpaceTokenizer using this definition to the ICU module. Problem solved (it is in fact a small patch with a Tokenizer extends CharTokenizer and the factory).

[asfimport]

Robert Muir (@rmuir) (migrated from JIRA)

You can add a CharTokenizer to ICU analysis module that just looks like this:

  protected boolean isTokenChar(int c) {
    return !UCharacter.isUWhiteSpace(c);
  }

If you are not happy with it needing ICU library, the definition of this property in ICU is "Space characters+TAB+CR+LF-ZWSP-ZWNBSP" (http://icu-project.org/apiref/icu4j/com/ibm/icu/lang/UProperty.html#WHITE_SPACE) so it shouldn't be hard to implement with just the jdk, but I do not know about the efficiency of that.

Either way, I think it should just be a different tokenizer.

[asfimport]

Uwe Schindler (@uschindler) (migrated from JIRA)

@rmuir: I am already preparing a patch :-)

[asfimport]

Uwe Schindler (@uschindler) (migrated from JIRA)

Patch that adds the ICU variant of the tokenizer (it's just copypaste...). This is the only way to do this right: It conforms to Unicode and also uses Unicode's fast reference implementation.

I also added a test.

[asfimport]

David Smiley (@dsmiley) (migrated from JIRA)

Uwe,
I beg to differ on WDF but I think we can put that behind us. It's great to see a solution come together on using ICU's rules :-)

For something as trivial as detecting if the character is in a set, do we really need to depend on ICU? It would be so nice to not need it, even if it's internal implementation seems fast. Then we could consider deprecating WhitespaceTokenizer since, after all, why would one use it when ICUWhitespaceTokenizer exists? Anyone wanting atypical tokenization could easily subclass CharTokenizer or consider a MappingCharFilter.

[asfimport]

Robert Muir (@rmuir) (migrated from JIRA)

I don't think we need to deprecate whitespacetokenizer, i think its intuitive for a java developer, and lucene is a java API.

If you really must avoid ICU: I already mentioned, you can probably implement it yourself, but i just don't think it will be as fast. you will probably have to precompute and cache for latin1 and all kinds of stuff to make it competitive. and it will be messier and so on.

[asfimport]

Uwe Schindler (@uschindler) (migrated from JIRA)

Then we could consider deprecating WhitespaceTokenizer since, after all, why would one use it when ICUWhitespaceTokenizer exists?

Because the non-breaking space is useful for stuff (as explained above) where you want to keep tokens together (although usicode standard speaks about line wrapping, but in any case, like soft hyphen vs. hyphen, its just a matter what you want to do: the NBSP just tells the tokenizer or line-breaker or how you call it to keep tokens together). The problem are people that misuse   in their HTML shit (e.g. tables). But for stuff I had implemented very often, I used WhitepsaceTokenizer to split tokens and placed a non-breaking space to keep tokens together.

So there is no need to deprecate WhitespaceTokenizer. It does what it should do. ICUWhitespaceTokenizer is using same naming and does the same, just with different rules.

... or consider a MappingCharFilter

This is thing is slow like hell. If you want it faster, e.g. use PatternTokenizer.

It would be so nice to not need it, even if it's internal implementation seems fast

The problem is that you need to do additional 4-way branching: You have to check in isTokenChar() that it is !Whitespace() and also exclude all those 3 chars we listed in the description: '\u00A0', '\u2007', '\u202F'.

I agree with Robert: We should not change the default WhitespaceTokenizer and also not deprecate it. We should add a new one, which I did in supplied patch. If we want it in core, lets call it different and implement isTokenChar in a fast way without 3 additional branches.

I beg to differ on WDF

This is coming from the fact that Solr is often misused because users just give up to think about tokenization. WDF only makes sense in product catalogues, but it is definitely broken for fulltext. The product catalogues are of course some of our customers, but before I suggest to them that they should use WhiteSpaceTokenizer with WordDestroyerFilter, I would analyzer their root problem (why is their tokenization broken). This is why I am against the broken example configs in Solr we had in the past. Because WST and WDF should really only be used as a last resort.