-
-
Notifications
You must be signed in to change notification settings - Fork 709
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
fix: #2078 return error when tokenizer not found while indexing #2093
fix: #2078 return error when tokenizer not found while indexing #2093
Conversation
src/indexer/segment_writer.rs
Outdated
let index = Index::create_in_dir(&tempdir_path, schema).unwrap(); | ||
index | ||
.tokenizers() | ||
.register("custom_en", custom_en_tokenizer); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that can be removed, it's unused
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess we can remove the tokenizer registration but we may need the index creation else open_in_dir
will fail
src/indexer/segment_writer.rs
Outdated
let mut document = Document::default(); | ||
document.add_text(title, "The Old Man and the Sea"); | ||
index_writer.add_document(document).unwrap(); | ||
match index_writer.commit() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
you can use unwrap_err()
which will panic if it's not an error
src/indexer/segment_writer.rs
Outdated
@@ -900,4 +911,42 @@ mod tests { | |||
postings.positions(&mut positions); | |||
assert_eq!(positions, &[4]); //< as opposed to 3 if we had a position length of 1. | |||
} | |||
|
|||
// ISSUE-#2078 - writing and searching shall throw error when the field tokenizer is missing |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
the comment implies this also tests search which is not the case
Codecov Report
❗ Your organization is not using the GitHub App Integration. As a result you may experience degraded service beginning May 15th. Please install the Github App Integration for your organization. Read more. @@ Coverage Diff @@
## main #2093 +/- ##
=======================================
Coverage 94.38% 94.38%
=======================================
Files 321 321
Lines 60583 60612 +29
=======================================
+ Hits 57179 57210 +31
+ Misses 3404 3402 -2
|
This is to fix #2078