ESQL: Remove AllReader

server/src/main/java/org/elasticsearch/index/mapper/AbstractShapeGeometryFieldMapper.java

@@ -92,12 +92,12 @@ protected BoundsBlockLoader(String fieldName) {
             }
 
             @Override
-            public BlockLoader.AllReader reader(CircuitBreaker breaker, LeafReaderContext context) throws IOException {
+            public BlockLoader.ColumnAtATimeReader reader(CircuitBreaker breaker, LeafReaderContext context) throws IOException {
                 breaker.addEstimateBytesAndMaybeBreak(BLOCK_LOADER_ESTIMATED_SIZE, "load blocks");
                 BinaryDocValues binaryDocValues = context.reader().getBinaryDocValues(fieldName);
                 if (binaryDocValues == null) {
                     breaker.addWithoutBreaking(-BLOCK_LOADER_ESTIMATED_SIZE);
-                    return ConstantNull.READER;
+                    return ConstantNull.COLUMN_READER;
                 }
                 return new BoundsReader(breaker, binaryDocValues);
             }
@@ -108,7 +108,7 @@ public BlockLoader.Builder builder(BlockLoader.BlockFactory factory, int expecte
             }
         }
 
-        private static class BoundsReader implements BlockLoader.AllReader {
+        private static class BoundsReader implements BlockLoader.ColumnAtATimeReader {
             private final GeometryDocValueReader reader = new GeometryDocValueReader();
             private final CircuitBreaker breaker;
             private final BinaryDocValues binaryDocValues;
@@ -129,11 +129,6 @@ public BlockLoader.Block read(BlockLoader.BlockFactory factory, BlockLoader.Docs
                 }
             }
 
-            @Override
-            public void read(int docId, BlockLoader.StoredFields storedFields, BlockLoader.Builder builder) throws IOException {
-                read(binaryDocValues, docId, (org.elasticsearch.index.mapper.BlockLoader.IntBuilder) builder);
-            }
-
             private void read(BinaryDocValues binaryDocValues, int doc, org.elasticsearch.index.mapper.BlockLoader.IntBuilder builder)
                 throws IOException {
                 if (binaryDocValues.advanceExact(doc) == false) {

server/src/main/java/org/elasticsearch/index/mapper/BlockLoader.java

@@ -146,14 +146,14 @@
  *     but to disable {@code _source} and {@code doc_values}. Nothing's perfect. Especially
  *     code.
  * </p>
- * <h2>Why is {@link AllReader}?</h2>
+ * <h2>Column-at-a-time vs row-stride</h2>
  * <p>
- *     When we described how to read from {@code doc_values} we said we <strong>prefer</strong>
- *     to use {@link ColumnAtATimeReader}. But some callers don't support reading column-at-a-time
- *     and need to read row-by-row. So we also need an implementation of {@link RowStrideReader}
- *     that reads from {@code doc_values}. Usually it's most convenient to implement both of those
- *     in the same {@code class}. {@link AllReader} is an interface for those sorts of classes, and
- *     you'll see it in the {@code doc_values} code frequently.
+ *     Readers may load {@link ColumnAtATimeReader column-at-a-time} or {@link RowStrideReader row-by-row}.
+ *     They need only return non-null from {@link #columnAtATimeReader} or {@link #rowStrideReader}.
+ *     {@link ColumnAtATimeReader}s have the lowest overhead and get access to a large batch of document
+ *     ids to load at once, making them must faster for loading dense on disk structures like doc values
+ *     or vector distances. {@link RowStrideReader}s can use {@link StoredFields} to access Lucene's
+ *     compressed {@code stored} fields, include Elasticsearch's {@code _source}.
  * </p>
  * <h2>Why is {@link #rowStrideStoredFieldSpec}?</h2>
  * <p>
@@ -294,13 +294,6 @@ interface RowStrideReader extends Reader {
         void read(int docId, StoredFields storedFields, Builder builder) throws IOException;
     }
 
-    /**
-     * @deprecated we no longer need to implement {@link RowStrideReader} when
-     *             storage prefers column-at-a-time
-     */
-    @Deprecated
-    interface AllReader extends ColumnAtATimeReader, RowStrideReader {}
-
     interface StoredFields {
         /**
          * The {@code _source} of the document.
@@ -431,31 +424,57 @@ public RowStrideReader rowStrideReader(CircuitBreaker breaker, LeafReaderContext
             if (preferLoader.rowStrideStoredFieldSpec().noRequirements() == false) {
                 return fallbackLoader.rowStrideReader(breaker, context);
             }
-            RowStrideReader preferReader = preferLoader.rowStrideReader(breaker, context);
-            if (canUsePreferLoaderForLeaf(context)) {
-                return preferReader;
-            }
-            RowStrideReader fallbackReader = fallbackLoader.rowStrideReader(breaker, context);
-            return new RowStrideReader() {
-                @Override
-                public void read(int docId, StoredFields storedFields, Builder builder) throws IOException {
-                    if (storedFields.loaded() == false && canUsePreferLoaderForDoc(docId)) {
-                        preferReader.read(docId, storedFields, builder);
-                    } else {
-                        fallbackReader.read(docId, storedFields, builder);
-                    }
+            RowStrideReader preferReader = null;
+            RowStrideReader fallbackReader = null;
+            boolean success = false;
+            try {
+                preferReader = preferLoader.rowStrideReader(breaker, context);
+                if (preferReader == null) {
+                    throw new IllegalStateException(
+                        "ConditionalBlockLoader requires sub-readers to support both row-at-a-time and column-at-a-time: " + preferLoader
+                    );
                 }
-
-                @Override
-                public boolean canReuse(int startingDocID) {
-                    return fallbackReader.canReuse(startingDocID) && preferReader.canReuse(startingDocID);
+                if (canUsePreferLoaderForLeaf(context)) {
+                    success = true;
+                    return preferReader;
                 }
-
-                @Override
-                public void close() {
+                fallbackReader = fallbackLoader.rowStrideReader(breaker, context);
+                success = true;
+                return new ConditionalRowStrideReader(preferReader, fallbackReader);
+            } finally {
+                if (success == false) {
                     Releasables.close(preferReader, fallbackReader);
                 }
-            };
+            }
+        }
+
+        class ConditionalRowStrideReader implements RowStrideReader {
+            private final RowStrideReader preferReader;
+            private final RowStrideReader fallbackReader;
+
+            ConditionalRowStrideReader(RowStrideReader preferReader, RowStrideReader fallbackReader) {
+                this.preferReader = preferReader;
+                this.fallbackReader = fallbackReader;
+            }
+
+            @Override
+            public void read(int docId, StoredFields storedFields, Builder builder) throws IOException {
+                if (storedFields.loaded() == false && canUsePreferLoaderForDoc(docId)) {
+                    preferReader.read(docId, storedFields, builder);
+                } else {
+                    fallbackReader.read(docId, storedFields, builder);
+                }
+            }
+
+            @Override
+            public boolean canReuse(int startingDocID) {
+                return fallbackReader.canReuse(startingDocID) && preferReader.canReuse(startingDocID);
+            }
+
+            @Override
+            public void close() {
+                Releasables.close(preferReader, fallbackReader);
+            }
         }
 
         @Override

server/src/main/java/org/elasticsearch/index/mapper/BlockSourceReader.java

@@ -139,7 +139,7 @@ public final RowStrideReader rowStrideReader(CircuitBreaker breaker, LeafReaderC
             DocIdSetIterator iter = lookup.lookup(context);
             if (iter == null) {
                 breaker.addWithoutBreaking(-ESTIMATED_SIZE);
-                return ConstantNull.READER;
+                return ConstantNull.ROW_READER;
             }
             return rowStrideReader(breaker, context, iter);
         }

server/src/main/java/org/elasticsearch/index/mapper/IdFieldMapper.java

@@ -135,8 +135,7 @@ public Query existsQuery(SearchExecutionContext context) {
 
         @Override
         public BlockLoader blockLoader(BlockLoaderContext blContext) {
-            return IdLoader.create(blContext.indexSettings(), blContext.mappingLookup())
-                .blockLoader(blContext.ordinalsByteSize().getBytes());
+            return IdLoader.create(blContext.indexSettings(), blContext.mappingLookup()).blockLoader(blContext.ordinalsByteSize());
         }
 
         @Override