feat: add a simple way to chain two tokenizers

[ctron]

No description provided.

[ctron]

I applied nightly rustfmt.

[fulmicoton]

@ctron can you describe your use case?

[ctron]

My use case is to have all simple tokens plus all ngrams.

[adamreichold]

The Token::position fields would need updating though, wouldn't they? Meaning the positions of the second stream should be offset by the number of tokens yielded by the first one?

[ctron]

I was able to incorporate most of the feedback you mentioned. It's less explicit without the enum, but works the same way. There was just one call to second.advance() missing, as a TokenStream seems to start before the first item.

I am not sure about the position topic, but I also don't fully understand it: If the idea is to enumerate/count all tokens, then yes that should be offset but the length of first. If that reflects the position of the token as part of the original input, then that might not be correct.

I'll admit that the whole API around tokenization feels a bit confusing.

[adamreichold]

I was able to incorporate most of the feedback you mentioned. It's less explicit without the enum, but works the same way. There was just one call to second.advance() missing, as a TokenStream seems to start before the first item.

Agreed, it is more implicit. But I was mainly suggesting it for efficiency, i.e. keep the state as small as possible and drop the first tokenizer as soon as we are done with it. If you feel like code-golfing, I think those two calls to second.advance() could even be merged:

fn advance(&mut self) -> bool {
    if let Some(first) = &mut self.first {
        if first.advance() {
            return true;
        } else {
            self.first = None;
        }
    }
    
    self.second.advance()
}

[ctron]

If you feel like code-golfing, I think those two calls to second.advance() could even be merged

I like that, pushed.

So, the remaining thing seems to be the position. I am just not sure what to do with it.

[adamreichold]

So, the remaining thing seems to be the position. I am just not sure what to do with it.

I'd say let's wait for input from @fulmicoton on that. I am myself unsure what downstream consumers expect of the position field. I suspect that this is mainly used for phrase queries with slop which I think would make the current implementation correct, i.e. have position still refer to the original input.

[fulmicoton]

@PSeitz can you review ?

[ctron]

Fixed the test issue.

[PSeitz]

There is a hidden contract currently on the tokenizer API which expects positions to be sorted incrementally. This happens in the serialization part in recorder.rs, when positions are delta encoded.

There are two options:

• Handle unsorted positions in the serialization (allow unsorted positions from the tokenizer)
• Return the tokens in a sorted way

I think handling unsorted positions is not really favorable, since it would carry some performance overhead.

[adamreichold]

Return the tokens in a sorted way

So in this case, we'd need to interleave the output of the two tokenizer dynamically to ensure that one does not outpace the other. Or could just make positions up and offset all positions returned by the second tokenizer?