leaves12138 commented on code in PR #7:
URL: https://github.com/apache/paimon-full-text/pull/7#discussion_r3524514631
##########
core/src/tokenizer.rs:
##########
@@ -36,19 +37,19 @@ impl Default for TokenizerConfig {
fn default() -> Self {
Self {
tokenizer: TokenizerKind::Default,
- ngram_min_gram: 2,
- ngram_max_gram: 2,
+ ngram_min_gram: 3,
+ ngram_max_gram: 3,
ngram_prefix_only: false,
jieba_search_mode: true,
jieba_ordinal_position: true,
lower_case: true,
max_token_length: 40,
- ascii_folding: false,
- stem: false,
+ ascii_folding: true,
+ stem: true,
language: "english".to_string(),
- remove_stop_words: false,
+ remove_stop_words: true,
stop_words: Vec::new(),
- with_position: true,
+ with_position: false,
Review Comment:
This changes the library defaults from the previous API/Paimon adapter
assumptions (`ngram` 2/2, `ascii-folding=false`, `stem=false`,
`remove-stop-words=false`, `with-position=true`) to English-analysis defaults
with `with_position=false`. That is not just a local default change: the Java
Paimon adapter in apache/paimon PR #8463 serializes sparse native options by
omitting values that are default on the Paimon side. With these new defaults, a
default Paimon full-text index would be built without positions, so phrase
queries fail with `phrase query requires positions`; `ngram` without explicit
grams also becomes 3/3 instead of Paimon's 2/2, and
stemming/stop-word/ascii-folding semantics silently change. Please either keep
the standalone 0.1 defaults aligned with the Paimon adapter defaults, or update
the adapter/integration contract so it always passes the full intended defaults
(and add a regression test covering default phrase search/ngram defaults).
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
