Skip to content
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

Add option for fieldnorm #1215

Merged
merged 3 commits into from
Apr 8, 2022
Merged

Add option for fieldnorm #1215

merged 3 commits into from
Apr 8, 2022

Conversation

infiniteregrets
Copy link
Contributor

Description

This PR makes the fieldnorm feature optional as it was done in the tantivy library (refer: quickwit-oss/tantivy#1001). The index configuration now supports fieldnorms field which can be set to a boolean value and by default is set to false.
Closes #1134

How was this PR tested?

  • Created a custom wikipedia index with the fieldnorms option enabled and found that the configuration was being serialized without any errors.
  • Wrote a test to check if the case when the FieldMappingType is Text, whether the index options for fieldnorms are set to true or not for the passed json with fieldnorms set to true.

Signed-off-by: Mehul Arora aroram18@mcmaster.ca

@fmassot
Copy link
Contributor

fmassot commented Mar 23, 2022

@infiniteregrets cool! To test it completely, that would be awesome to follow these steps:

  • index Wikipedia datasets with fieldnorms
  • index Wikipedia datasets without fieldnorms
  • use the CLI to extract a split and look at the different tantivy files. You should find a .fieldnorm file and the size should be almost 0 without fieldnorms.

@infiniteregrets
Copy link
Contributor Author

Okay sure, I will test it out. I am actually missing the default fieldnorm setting for different types like i64, u64 etc. so I will fix that first

@infiniteregrets infiniteregrets force-pushed the fieldnorm-patch branch 3 times, most recently from 6138339 to 4bb0797 Compare March 24, 2022 17:56
@fmassot
Copy link
Contributor

fmassot commented Mar 26, 2022

sounds good. @infiniteregrets did you try to index with and without fieldnorms?

Signed-off-by: Mehul Arora <aroram18@mcmaster.ca>
@fulmicoton fulmicoton merged commit d1f281e into main Apr 8, 2022
@fulmicoton fulmicoton deleted the fieldnorm-patch branch April 8, 2022 03:08
@fulmicoton
Copy link
Contributor

Thank you @infiniteregrets

This was referenced Apr 18, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove fieldnorm
3 participants