benwtrent commented on code in PR #13548:
URL: https://github.com/apache/lucene/pull/13548#discussion_r1669043121
##########
lucene/core/src/java/org/apache/lucene/codecs/KnnVectorsWriter.java:
##########
@@ -139,6 +142,54 @@ public int nextDoc() throws IOException {
}
}
+ /**
+ * Given old doc ids and an id mapping, maps old ordinal to new ordinal.
Note: this method return
+ * nothing and output are written to parameters
+ *
Review Comment:
```suggestion
* @param oldDocIds the old or current document ordinals. Must not be null.
* @param sortMap, the document sorting map for how to make the new
ordinals. Must not be null.
```
##########
lucene/core/src/java/org/apache/lucene/codecs/KnnVectorsWriter.java:
##########
@@ -139,6 +142,54 @@ public int nextDoc() throws IOException {
}
}
+ /**
+ * Given old doc ids and an id mapping, maps old ordinal to new ordinal.
Note: this method return
+ * nothing and output are written to parameters
+ *
+ * @param old2NewOrd int[] maps from old ord to new ord
+ * @param new2OldOrd int[] maps from new ord to old ord
+ * @param newDocsWithField set of new doc ids which has the value
+ */
+ public static void mapOldOrdToNewOrd(
+ DocsWithFieldSet oldDocIds,
+ Sorter.DocMap sortMap,
+ int[] old2NewOrd,
+ int[] new2OldOrd,
+ DocsWithFieldSet newDocsWithField)
+ throws IOException {
+ // TODO: a similar function exists in
IncrementalHnswGraphMerger#getNewOrdMapping
+ // maybe we can do a further refactoring
+ assert (old2NewOrd != null || new2OldOrd != null || newDocsWithField !=
null);
Review Comment:
```suggestion
Objects.requireNonNull(oldDocIds);
Objects.requireNonNull(sortMap);
assert (old2NewOrd != null || new2OldOrd != null || newDocsWithField !=
null);
```
##########
lucene/core/src/java/org/apache/lucene/codecs/lucene99/Lucene99FlatVectorsFormat.java:
##########
@@ -56,8 +56,8 @@
* <li><b>[vlong]</b> length of this field's vectors, in bytes
* <li><b>[vint]</b> dimension of this field's vectors
* <li><b>[int]</b> the number of documents having values for this field
- * <li><b>[int8]</b> if equals to -1, dense – all documents have values for
a field. If equals to
- * 0, sparse – some documents missing values.
+ * <li><b>[int8]</b> if equals to -2, empty - no vectory values. If equals
to -1, dense – all
Review Comment:
```suggestion
* <li><b>[int8]</b> if equals to -2, empty - no vector values. If equals
to -1, dense – all
```
##########
lucene/core/src/java/org/apache/lucene/codecs/KnnVectorsWriter.java:
##########
@@ -139,6 +142,54 @@ public int nextDoc() throws IOException {
}
}
+ /**
+ * Given old doc ids and an id mapping, maps old ordinal to new ordinal.
Note: this method return
+ * nothing and output are written to parameters
+ *
+ * @param old2NewOrd int[] maps from old ord to new ord
+ * @param new2OldOrd int[] maps from new ord to old ord
+ * @param newDocsWithField set of new doc ids which has the value
+ */
+ public static void mapOldOrdToNewOrd(
+ DocsWithFieldSet oldDocIds,
+ Sorter.DocMap sortMap,
+ int[] old2NewOrd,
+ int[] new2OldOrd,
+ DocsWithFieldSet newDocsWithField)
+ throws IOException {
+ // TODO: a similar function exists in
IncrementalHnswGraphMerger#getNewOrdMapping
+ // maybe we can do a further refactoring
+ assert (old2NewOrd != null || new2OldOrd != null || newDocsWithField !=
null);
+ IntIntHashMap newIdToOldOrd = new IntIntHashMap();
Review Comment:
Since this method isn't in charge of creating `old2NewOrd` or `new2OldOrd`
can we assert those array sizes they are provided?
##########
lucene/core/src/java/org/apache/lucene/index/FieldUpdatesBuffer.java:
##########
@@ -356,7 +356,7 @@ BufferedUpdate next() throws IOException {
}
}
- BytesRef nextTerm() throws IOException {
+ private BytesRef nextTerm() throws IOException {
Review Comment:
seems like an extra thing out of nowhere? Its probably ok in this commit,
just struck me as a strange inclusion :)
--
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
