msokolov commented on code in PR #12829:
URL: https://github.com/apache/lucene/pull/12829#discussion_r1424209861


##########
lucene/core/src/java/org/apache/lucene/index/IndexingChain.java:
##########
@@ -219,15 +222,33 @@ private Sorter.DocMap maybeSortSegment(SegmentWriteState 
state) throws IOExcepti
     }
 
     LeafReader docValuesReader = getDocValuesLeafReader();
-
+    Function<IndexSorter.DocComparator, IndexSorter.DocComparator> 
comparatorWrapper = in -> in;
+
+    if (state.segmentInfo.getHasBlocks() && indexSort.getParentField() != 
null) {
+      final DocIdSetIterator readerValues =
+          docValuesReader.getNumericDocValues(indexSort.getParentField());
+      BitSet parents = BitSet.of(readerValues, state.segmentInfo.maxDoc());
+      comparatorWrapper =
+          in ->
+              (docID1, docID2) ->
+                  in.compare(parents.nextSetBit(docID1), 
parents.nextSetBit(docID2));
+    }
+    assert state.segmentInfo.getHasBlocks() == false
+            || indexSort.getParentField() != null
+            || indexCreatedVersionMajor < Version.LUCENE_10_0_0.major
+        : "parent field is not set but the index has blocks. 
indexCreatedVersionMajor: "
+            + indexCreatedVersionMajor;
     List<IndexSorter.DocComparator> comparators = new ArrayList<>();
     for (int i = 0; i < indexSort.getSort().length; i++) {
       SortField sortField = indexSort.getSort()[i];
       IndexSorter sorter = sortField.getIndexSorter();
       if (sorter == null) {
         throw new UnsupportedOperationException("Cannot sort index using sort 
field " + sortField);
       }
-      comparators.add(sorter.getDocComparator(docValuesReader, 
state.segmentInfo.maxDoc()));
+
+      IndexSorter.DocComparator docComparator =

Review Comment:
   Thanks for addl info @simonw. your point about the complication of arbitrary 
query execution from inside indexing chain makes sense. I think the approach 
makes sense to me
   



##########
lucene/core/src/java/org/apache/lucene/index/IndexingChain.java:
##########
@@ -1512,4 +1557,77 @@ void assertSameSchema(FieldInfo fi) {
       assertSame("point num bytes", fi.getPointNumBytes(), pointNumBytes);
     }
   }
+
+  /**
+   * Wraps the given field in a reserved field and registers it as reserved. 
Only DWPT should do
+   * this to mark fields as private / reserved to prevent this fieldname to be 
used from the outside

Review Comment:
   I get that we are recording the name so it cannot be created with a 
different type, and indeed prevent it from being added to a document except by 
IndexingChain -- are there any other restrictions on the use of the field? 
Could we clear up the javadoc to be more explicit? I think eg I can still get a 
DV iterator over this field and read its values from "outside". Maybe we could 
say "prevent this fieldname from being used when writing documents ..."?



-- 
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

Reply via email to