[Feature] Adding a char_group tokenizer

[synhershko]

Char Group Tokenizer

The char_group tokenizer breaks text into terms whenever it encounters
a
character which is in a defined set. It is mostly useful for cases where
a simple
custom tokenization is desired, and the overhead of use of the
<<analysis-pattern-tokenizer, pattern tokenizer>>
is not acceptable.

Configuration

The char_group tokenizer accepts one parameter:

tokenize_on_chars::
A string containing a list of characters to tokenize the string on.
Whenever a character
from this list is encountered, a new token is started. Also supports
escaped values like \\n and \\f,
and in addition \\s to represent whitespace, \\d to represent
digits and \\w to represent letters.
Defaults to an empty list.

Example output

The 2 QUICK Brown-Foxes jumped over the lazy dog's bone for $2

When the configuration \\s-:<> is used for tokenize_on_chars, the
above sentence would produce the following terms:

[ The, 2, QUICK, Brown, Foxes, jumped, over, the, lazy, dog's, bone,
for, $2 ]

[elasticmachine]

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

[jimczi]

It is mostly useful for cases where
a simple
custom tokenization is desired, and the overhead of use of the
<<analysis-pattern-tokenizer, pattern tokenizer>>
is not acceptable.

Are you sure that there is an overhead when using a pattern tokenizer for single char tokenization ?
Is this the only reason why you need this specific tokenizer ? I guess that if the list of characters is big then this tokenizer is easier to use but I am not sure that it should be in the list of default tokenizers. A plugin seems a better fit because it is intended for a very specific usage ?

[synhershko]

@jimczi yes, at a client's site and it's measurable even for a simple regex pattern.

From experience, maintaining a plugin is a considerable overhead and this seems quite a legit and useful addition, considering the existence of the letter tokenizer

[jimczi]

Fair enough. I marked this PR with the discuss label. It is also unclear to me if this tokenizer should be added to Lucene first. @jpountz may have a better answer regarding this. And thanks @synhershko for submitting your PR.

[jpountz]

Sorry for the late reply, I had totally missed the ping. @synhershko I think this new tokenizer could help. The main question to me is how we should allow the list of token chars to be configured. We already have a token_chars option with the ngram tokenizer, I'm wondering whether we should try to be consistent. Also I'm curious what your actual use-case was with this customer? For instance do you think we need to expose the ability to tokenize on specific characters or would it be enough to just expose character classes like the ngram tokenizer does?

[jpountz]

To @jimczi 's question, I think Elasticsearch is the right place to put this tokenizer given that Lucene does not need more than its existing CharTokenizer?

[dakrone]

Ping @synhershko is this still something you're interested in pursuing?

[synhershko]

@dakrone sure, the PR with all required changes are here - not sure what you are asking? :)

[synhershko]

Ping @jpountz @dakrone - would love to have this in for 6.0, anything else you need from me here?

[dakrone]

@synhershko I think @jpountz's questions from his last comment are still unanswered, other than that I'm not sure what else needs to happen, @jpountz will likely have a better idea

[synhershko]

Aye, responding below

The main question to me is how we should allow the list of token chars to be configured. We already have a token_chars option with the ngram tokenizer, I'm wondering whether we should try to be consistent.

That's just a naming question - fine by me to change that.

Also I'm curious what your actual use-case was with this customer?

To be honest I don't remember the actual use-case, but I do remember it was important enough to call for this PR :) I assume this was an attempt to efficiently parse incoming user data with custom separators. We all know those type of systems...

The Pattern replace tokenizer would most likely help that cause but it does come with performance penalty.

For instance do you think we need to expose the ability to tokenize on specific characters or would it be enough to just expose character classes like the ngram tokenizer does?

Using character classes does look tempting for simplicity reasons, but in most likelihood this tokenizer will mostly be useful for it's granularity. Do note \s, \w , \d are supported ootb and can also be extended for more categories if needed.

[jpountz]

Maybe we could make it take eg. token_chars: [ "whitespace", "-", "_" ] as a way to say it should split on whitespaces, hypens and underscores? It is not ambiguous since all character classes use more than one char, and consistent with the current configuration of the ngram and edge_ngram tokenizers (which could later be improved to allow splitting on specific chars too)?

[synhershko]

Sounds good. You want me to go ahead and apply this change?

If so, what should we do if len(token_chars[i]) > 1 and token_chars[i] not in ["whitespace", "digit", "letter"] for any i? this subtlety is probably the only downside for this approach

[jpountz]

@synhershko Yes, something like that. I suspect your example didn't mean to be all-inclusive, but just in case, supporting symbol and punctuation would be nice too.

[jpountz]

Actually I just realized that reusing token_chars is a bad idea since this option of the ngram tokenizer is about characters to keep while here we are dealing with characters to split on. So let's keep tokenize_on_chars that you initially suggested? However I'd still like to go with an array notation which I find less ambiguous than a string, especially if there are supplementary chars in the string that can't be represented by a single Java char?

[synhershko]

Oh right, that might be the original reason why I chose that naming ;) been too long.

I pushed an update to the code - can you confirm structure and config look good? if so I will update tests

[romseygeek]

/cc @elastic/es-search-aggs

[elasticmachine]

Since this is a community submitted pull request, a Jenkins build has not been kicked off automatically. Can an Elastic organization member please verify the contents of this patch and then kick off a build manually?

[jpountz]

@elasticmachine test this please

[jpountz]

@synhershko I took the liberty of doing some minor changes like moving the code to analysis-common before merging this PR. Sorry for the long delay.