jpountz commented on a change in pull request #1440:
URL: https://github.com/apache/lucene-solr/pull/1440#discussion_r412312883
##########
File path:
lucene/core/src/java/org/apache/lucene/index/DefaultIndexingChain.java
##########
@@ -94,29 +89,100 @@ public DefaultIndexingChain(DocumentsWriterPerThread
docWriter) {
termsHash = new FreqProxTermsWriter(docWriter, termVectorsWriter);
}
+ private LeafReader getDocValuesReader(int maxDoc) {
+ return new DocValuesReader() {
+ @Override
+ public NumericDocValues getNumericDocValues(String field) throws
IOException {
+ PerField pf = getPerField(field);
+ if (pf == null) {
+ return null;
+ }
+ if (pf.fieldInfo.getDocValuesType() == DocValuesType.NUMERIC) {
+ return (NumericDocValues) pf.docValuesWriter.getDocValues();
+ }
+ return null;
+ }
+
+ @Override
+ public BinaryDocValues getBinaryDocValues(String field) throws
IOException {
+ PerField pf = getPerField(field);
+ if (pf == null) {
+ return null;
+ }
+ if (pf.fieldInfo.getDocValuesType() == DocValuesType.BINARY) {
+ return (BinaryDocValues) pf.docValuesWriter.getDocValues();
+ }
+ return null;
+ }
+
+ @Override
+ public SortedDocValues getSortedDocValues(String field) throws
IOException {
+ PerField pf = getPerField(field);
+ if (pf == null) {
+ return null;
+ }
+ if (pf.fieldInfo.getDocValuesType() == DocValuesType.SORTED) {
+ return (SortedDocValues) pf.docValuesWriter.getDocValues();
+ }
+ return null;
+ }
+
+ @Override
+ public SortedNumericDocValues getSortedNumericDocValues(String field)
throws IOException {
+ PerField pf = getPerField(field);
+ if (pf == null) {
+ return null;
+ }
+ if (pf.fieldInfo.getDocValuesType() == DocValuesType.SORTED_NUMERIC) {
+ return (SortedNumericDocValues) pf.docValuesWriter.getDocValues();
+ }
+ return null;
+ }
+
+ @Override
+ public SortedSetDocValues getSortedSetDocValues(String field) throws
IOException {
+ PerField pf = getPerField(field);
+ if (pf == null) {
+ return null;
+ }
+ if (pf.fieldInfo.getDocValuesType() == DocValuesType.SORTED_SET) {
+ return (SortedSetDocValues) pf.docValuesWriter.getDocValues();
+ }
+ return null;
+ }
+
+ @Override
+ public FieldInfos getFieldInfos() {
+ return fieldInfos.finish();
+ }
+
+ @Override
+ public int maxDoc() {
+ return maxDoc;
+ }
+ };
+ }
+
private Sorter.DocMap maybeSortSegment(SegmentWriteState state) throws
IOException {
Sort indexSort = state.segmentInfo.getIndexSort();
if (indexSort == null) {
return null;
}
- List<Sorter.DocComparator> comparators = new ArrayList<>();
+ LeafReader docValuesReader =
getDocValuesReader(state.segmentInfo.maxDoc());
+
+ List<IndexSorter.DocComparator> comparators = new ArrayList<>();
for (int i = 0; i < indexSort.getSort().length; i++) {
SortField sortField = indexSort.getSort()[i];
- PerField perField = getPerField(sortField.getField());
- if (perField != null && perField.docValuesWriter != null &&
- finishedDocValues.contains(perField.fieldInfo.name) == false) {
- perField.docValuesWriter.finish(state.segmentInfo.maxDoc());
- Sorter.DocComparator cmp =
perField.docValuesWriter.getDocComparator(state.segmentInfo.maxDoc(),
sortField);
- comparators.add(cmp);
- finishedDocValues.add(perField.fieldInfo.name);
- } else {
- // safe to ignore, sort field with no values or already seen before
+ IndexSorter sorter = sortField.getIndexSorter();
+ if (sorter == null) {
+ throw new UnsupportedOperationException("Cannot sort index using sort
field " + sortField);
}
+ comparators.add(sorter.getDocComparator(docValuesReader));
}
Sorter sorter = new Sorter(indexSort);
// returns null if the documents are already sorted
- return sorter.sort(state.segmentInfo.maxDoc(), comparators.toArray(new
Sorter.DocComparator[comparators.size()]));
+ return sorter.sort(state.segmentInfo.maxDoc(), comparators.toArray(new
IndexSorter.DocComparator[0]));
Review comment:
There is an even nicer syntax now.
```suggestion
return sorter.sort(state.segmentInfo.maxDoc(),
comparators.toArray(IndexSorter.DocComparator[]::new));
```
##########
File path: lucene/core/src/java/org/apache/lucene/index/DocValuesWriter.java
##########
@@ -21,12 +21,10 @@
import org.apache.lucene.codecs.DocValuesConsumer;
import org.apache.lucene.search.DocIdSetIterator;
-import org.apache.lucene.search.SortField;
-abstract class DocValuesWriter {
- abstract void finish(int numDoc);
+abstract class DocValuesWriter<T> {
abstract void flush(SegmentWriteState state, Sorter.DocMap sortMap,
DocValuesConsumer consumer) throws IOException;
- abstract Sorter.DocComparator getDocComparator(int numDoc, SortField
sortField) throws IOException;
+ abstract T getDocValues();
abstract DocIdSetIterator getDocIdSet();
Review comment:
can we remove this since `getDocValues` already returns an iterator? (we
might need to do `T extends DocIdSetIterator` above)
##########
File path: lucene/core/src/java/org/apache/lucene/index/DocValuesReader.java
##########
@@ -0,0 +1,84 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.lucene.index;
+
+import java.io.IOException;
+
+import org.apache.lucene.util.Bits;
+
+abstract class DocValuesReader extends LeafReader {
Review comment:
when I first saw this class I thought it would be similar to
DocValuesProducer, maybe rename to DocValuesLeafReader to clear potential
confusion?
##########
File path: lucene/core/src/java/org/apache/lucene/index/IndexSorter.java
##########
@@ -0,0 +1,500 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one or more
+ * contributor license agreements. See the NOTICE file distributed with
+ * this work for additional information regarding copyright ownership.
+ * The ASF licenses this file to You under the Apache License, Version 2.0
+ * (the "License"); you may not use this file except in compliance with
+ * the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+package org.apache.lucene.index;
+
+import java.io.IOException;
+import java.util.Arrays;
+import java.util.Comparator;
+import java.util.List;
+
+import org.apache.lucene.search.FieldComparator;
+import org.apache.lucene.search.SortField;
+import org.apache.lucene.store.DataInput;
+import org.apache.lucene.store.DataOutput;
+import org.apache.lucene.util.LongValues;
+import org.apache.lucene.util.NumericUtils;
+import org.apache.lucene.util.packed.PackedInts;
+
+import static org.apache.lucene.search.DocIdSetIterator.NO_MORE_DOCS;
+
+/**
+ * Handles how documents should be sorted in an index, both within a segment
and between
+ * segments.
+ *
+ * Implementers must provide the following methods:
+ * {@link #getDocComparator(LeafReader)} - an object that determines how
documents within a segment are to be sorted
+ * {@link #getComparableProviders(List)} - an array of objects that return a
sortable long value per document and segment
+ * {@link #serialize(DataOutput)} - how the sort should be written into the
segment header
+ * {@link #getProviderName()} - the SPI-registered name of a {@link
SortFieldProvider} to deserialize the sort
+ *
+ * The companion {@link SortFieldProvider} should be registered with SPI via
{@code META-INF/services}
+ */
+public interface IndexSorter {
+
+ /** Used for sorting documents across segments */
+ public interface ComparableProvider {
+ /**
+ * Returns a long so that the natural ordering of long values matches the
+ * ordering of doc IDs for the given comparator
+ */
+ long getAsComparableLong(int docID) throws IOException;
+ }
+
+ /** A comparator of doc IDs, used for sorting documents within a segment */
+ public interface DocComparator {
+ /** Compare docID1 against docID2. The contract for the return value is the
+ * same as {@link Comparator#compare(Object, Object)}. */
+ int compare(int docID1, int docID2);
+ }
+
+ /**
+ * Get an array of {@link ComparableProvider}, one per segment, for merge
sorting documents in different segments
+ * @param readers the readers to be merged
+ */
+ public abstract ComparableProvider[] getComparableProviders(List<? extends
LeafReader> readers) throws IOException;
+
+ /**
+ * Get a comparator that determines the sort order of docs within a single
Reader.
+ *
+ * NB We cannot simply use the {@link FieldComparator} API because it
requires docIDs to be sent
+ * in-order. The default implementations allocate array[maxDoc] to hold
native values for comparison,
+ * but 1) they are transient (only alive while sorting this one segment) and
2) in the typical
+ * index sorting case, they are only used to sort newly flushed segments,
which will be smaller
+ * than merged segments
+ *
+ * @param reader the Reader to sort
+ */
+ public abstract DocComparator getDocComparator(LeafReader reader) throws
IOException;
+
+ /**
+ * Serializes the parent SortField. This is used to write Sort information
into the Segment header
+ *
+ * @see SortFieldProvider#loadSortField(DataInput)
+ */
+ public abstract void serialize(DataOutput out) throws IOException;
Review comment:
I wonder if it would work to move it to SortFieldProvider instead for
symmetry, so that the serialization and deserialization logic would be in the
same place.
----------------------------------------------------------------
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.
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
