-
Notifications
You must be signed in to change notification settings - Fork 973
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
LUCENE-9353: revise format documentation of Lucene90BlockTreeTermsWriter #90
LUCENE-9353: revise format documentation of Lucene90BlockTreeTermsWriter #90
Conversation
@jpountz Could you take a look at this. |
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.
Thank you @mocobeta for fixing this!
.../core/src/java/org/apache/lucene/codecs/lucene90/blocktree/Lucene90BlockTreeTermsWriter.java
Outdated
Show resolved
Hide resolved
* <li>TermsMeta (.tmd) --> Header, NumFields, <FieldStats><sup>NumFields</sup>, | ||
* TermIndexLength, TermDictLength, Footer | ||
* <li>FieldStats --> FieldNumber, NumTerms, RootCodeLength, Byte<sup>RootCodeLength</sup>, | ||
* SumTotalTermFreq?, SumDocFreq, DocCount, MinTerm, MaxTerm, IndexStartFP, FSTHeader, |
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 it is SumDocFreq which is not always specified rather than SumTotalTermFreq?
* SumTotalTermFreq?, SumDocFreq, DocCount, MinTerm, MaxTerm, IndexStartFP, FSTHeader, | |
* SumTotalTermFreq, SumDocFreq?, DocCount, MinTerm, MaxTerm, IndexStartFP, FSTHeader, |
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'm actually not the author of the line (I just moved it from the above section to here), but the specification seems to be correct to me.
Lines 1108 to 1111 in 5592d58
if (fieldInfo.getIndexOptions() != IndexOptions.DOCS) { | |
metaOut.writeVLong(sumTotalTermFreq); | |
} | |
metaOut.writeVLong(sumDocFreq); |
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.
Woops I had misread!
Co-authored-by: Adrien Grand <jpountz@gmail.com>
It seems that the javadocs needs to be revised. I tried to reflect the changes in apache/lucene-solr#1473 to the format description.
latest javadocs
generated javadocs with the patch