romseygeek commented on a change in pull request #1440:
URL: https://github.com/apache/lucene-solr/pull/1440#discussion_r413689133



##########
File path: 
lucene/core/src/java/org/apache/lucene/index/DefaultIndexingChain.java
##########
@@ -527,45 +589,63 @@ private void indexPoint(PerField fp, IndexableField 
field) throws IOException {
     fp.pointValuesWriter.addPackedValue(docState.docID, field.binaryValue());
   }
 
-  private void validateIndexSortDVType(Sort indexSort, String fieldName, 
DocValuesType dvType) {
+  private void validateIndexSortDVType(Sort indexSort, String fieldToValidate, 
DocValuesType dvType) throws IOException {
     for (SortField sortField : indexSort.getSort()) {
-      if (sortField.getField().equals(fieldName)) {
-        switch (dvType) {
-          case NUMERIC:
-            if (sortField.getType().equals(SortField.Type.INT) == false &&
-                  sortField.getType().equals(SortField.Type.LONG) == false &&
-                  sortField.getType().equals(SortField.Type.FLOAT) == false &&
-                  sortField.getType().equals(SortField.Type.DOUBLE) == false) {
-              throw new IllegalArgumentException("invalid doc value type:" + 
dvType + " for sortField:" + sortField);
-            }
-            break;
+      IndexSorter sorter = sortField.getIndexSorter();
+      if (sorter == null) {
+        throw new IllegalStateException("Cannot sort index with sort order " + 
sortField);
+      }
+      sorter.getDocComparator(new DocValuesLeafReader() {
+        @Override
+        public NumericDocValues getNumericDocValues(String field) {
+          if (Objects.equals(field, fieldToValidate) && dvType != 
DocValuesType.NUMERIC) {
+            throw new IllegalArgumentException("SortField " + sortField + " 
expected field [" + field + "] to be NUMERIC but it is [" + dvType + "]");
+          }
+          return DocValues.emptyNumeric();
+        }
 
-          case BINARY:
-            throw new IllegalArgumentException("invalid doc value type:" + 
dvType + " for sortField:" + sortField);
+        @Override
+        public BinaryDocValues getBinaryDocValues(String field) {
+          if (Objects.equals(field, fieldToValidate) && dvType != 
DocValuesType.BINARY) {
+            throw new IllegalArgumentException("SortField " + sortField + " 
expected field [" + field + "] to be BINARY but it is [" + dvType + "]");
+          }
+          return DocValues.emptyBinary();
+        }
 
-          case SORTED:
-            if (sortField.getType().equals(SortField.Type.STRING) == false) {
-              throw new IllegalArgumentException("invalid doc value type:" + 
dvType + " for sortField:" + sortField);
-            }
-            break;
+        @Override
+        public SortedDocValues getSortedDocValues(String field) {
+          if (Objects.equals(field, fieldToValidate) && dvType != 
DocValuesType.SORTED) {
+            throw new IllegalArgumentException("SortField " + sortField + " 
expected field [" + field + "] to be SORTED but it is [" + dvType + "]");
+          }
+          return DocValues.emptySorted();
+        }
 
-          case SORTED_NUMERIC:
-            if (sortField instanceof SortedNumericSortField == false) {
-              throw new IllegalArgumentException("invalid doc value type:" + 
dvType + " for sortField:" + sortField);
-            }
-            break;
+        @Override
+        public SortedNumericDocValues getSortedNumericDocValues(String field) {
+          if (Objects.equals(field, fieldToValidate) && dvType != 
DocValuesType.SORTED_NUMERIC) {
+            throw new IllegalArgumentException("SortField " + sortField + " 
expected field [" + field + "] to be SORTED_NUMERIC but it is [" + dvType + 
"]");
+          }
+          return DocValues.emptySortedNumeric(0);
+        }
 
-          case SORTED_SET:
-            if (sortField instanceof SortedSetSortField == false) {
-              throw new IllegalArgumentException("invalid doc value type:" + 
dvType + " for sortField:" + sortField);
-            }
-            break;
+        @Override
+        public SortedSetDocValues getSortedSetDocValues(String field) {
+          if (Objects.equals(field, fieldToValidate) && dvType != 
DocValuesType.SORTED_SET) {
+            throw new IllegalArgumentException("SortField " + sortField + " 
expected field [" + field + "] to be SORTED_SET but it is [" + dvType + "]");
+          }
+          return DocValues.emptySortedSet();
+        }
 
-          default:
-            throw new IllegalArgumentException("invalid doc value type:" + 
dvType + " for sortField:" + sortField);
+        @Override
+        public FieldInfos getFieldInfos() {
+          throw new UnsupportedOperationException();
         }
-        break;
-      }
+
+        @Override
+        public int maxDoc() {
+          return 0;

Review comment:
       `IndexSorter.getDocComparator(Reader)` calls `maxDoc()` on the reader to 
allocate its comparison arrays.  We could change the signature so that it takes 
`maxDoc` as well as the Reader, which would reduce the indirection a bit and 
make it easier to read?




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

Reply via email to