dungba88 commented on code in PR #12624:
URL: https://github.com/apache/lucene/pull/12624#discussion_r1400663661
##########
lucene/core/src/java/org/apache/lucene/util/fst/FSTCompiler.java:
##########
@@ -827,22 +910,24 @@ void setEmptyOutput(T v) {
}
void finish(long newStartNode) {
- assert newStartNode <= bytes.size();
+ assert newStartNode <= numBytesWritten;
if (fst.metadata.startNode != -1) {
throw new IllegalStateException("already finished");
}
if (newStartNode == FINAL_END_NODE && fst.metadata.emptyOutput != null) {
newStartNode = 0;
}
fst.metadata.startNode = newStartNode;
- fst.metadata.numBytes = bytes.getPosition();
+ fst.metadata.numBytes = numBytesWritten;
}
private boolean validOutput(T output) {
return output == NO_OUTPUT || !output.equals(NO_OUTPUT);
}
/** Returns final FST. NOTE: this will return null if nothing is accepted by
the FST. */
+ // TODO: make this method to only return the FSTMetadata and user needs to
construct the FST
+ // themselves
public FST<T> compile() throws IOException {
Review Comment:
Currently if we try to read from it, the reader would return null (and be
changed to throw an IllegalStateException). If we change this method to just
return the FSTMetadata the user will be mindful that the FST might not be
readable and we will never return an unusable FST. But yeah it will touch a lot
of codes.
I also put together this PR for the migration later:
https://github.com/dungba88/lucene/pull/19. It will ensure that we will never
return an invalid/unusable FST.
--
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: issues-unsubscr...@lucene.apache.org
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: issues-unsubscr...@lucene.apache.org
For additional commands, e-mail: issues-h...@lucene.apache.org
