gsmiller commented on a change in pull request #442:
URL: https://github.com/apache/lucene/pull/442#discussion_r751355800
##########
File path:
lucene/facet/src/java/org/apache/lucene/facet/taxonomy/directory/TaxonomyIndexArrays.java
##########
@@ -130,15 +125,49 @@ private void initParents(IndexReader reader, int first)
throws IOException {
return;
}
+ if (tryLoadParentUsingTermPosition(reader, first)) {
Review comment:
The major version used to create each segment is available. Can you
leverage that? It's consistent with our approach in #443 for what it's worth.
For example:
`reader.leaves().get(0).reader().getMetaData().getCreatedVersionMajor() <= 8`
##########
File path:
lucene/facet/src/java/org/apache/lucene/facet/taxonomy/directory/DirectoryTaxonomyWriter.java
##########
@@ -92,15 +93,18 @@
private final Directory dir;
private final IndexWriter indexWriter;
private final boolean useOlderStoredFieldIndex;
+ private final boolean useOlderTermPositionForParentOrdinal;
Review comment:
Could you have a quick look at what I did in #443 and consider copying
the same idea? It just collapses these two booleans into a single one that
basically says "am I working with an 8.x or earlier index?" I need the same
idea in #443 so it might be nice to collapse everything down to this one
boolean for back-compat. Alternatively we could have three separate ones if you
think that makes more sense? Either way, let's reconcile this.
##########
File path:
lucene/facet/src/java/org/apache/lucene/facet/taxonomy/directory/DirectoryTaxonomyWriter.java
##########
@@ -466,18 +476,22 @@ protected final void ensureOpen() {
* effectively synchronized as well.
*/
private int addCategoryDocument(FacetLabel categoryPath, int parent) throws
IOException {
- // Before Lucene 2.9, position increments >=0 were supported, so we
- // added 1 to parent to allow the parent -1 (the parent of the root).
- // Unfortunately, starting with Lucene 2.9, after LUCENE-1542, this is
- // no longer enough, since 0 is not encoded consistently either (see
- // comment in SinglePositionTokenStream). But because we must be
- // backward-compatible with existing indexes, we can't just fix what
- // we write here (e.g., to write parent+2), and need to do a workaround
- // in the reader (which knows that anyway only category 0 has a parent
- // -1).
- parentStream.set(Math.max(parent + 1, 1));
Document d = new Document();
- d.add(parentStreamField);
+ if (useOlderTermPositionForParentOrdinal) {
+ // Before Lucene 2.9, position increments >=0 were supported, so we
+ // added 1 to parent to allow the parent -1 (the parent of the root).
+ // Unfortunately, starting with Lucene 2.9, after LUCENE-1542, this is
+ // no longer enough, since 0 is not encoded consistently either (see
+ // comment in SinglePositionTokenStream). But because we must be
+ // backward-compatible with existing indexes, we can't just fix what
+ // we write here (e.g., to write parent+2), and need to do a workaround
+ // in the reader (which knows that anyway only category 0 has a parent
+ // -1).
+ parentStream.set(Math.max(parent + 1, 1));
Review comment:
Maybe `assert parentStreamField != null`?
--
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
