From 5e604e492648a95f7adef067aeb2b9fc2ca4a206 Mon Sep 17 00:00:00 2001 From: Nhat Nguyen Date: Thu, 21 Aug 2025 15:52:17 -0700 Subject: [PATCH 1/7] Skip iterating DISI when reading metric values --- .../mapper/extras/ScaledFloatFieldMapper.java | 2 +- .../es819/ES819TSDBDocValuesProducer.java | 78 ++++++++- .../index/mapper/BlockDocValuesReader.java | 161 ++++++++++++++++-- .../index/mapper/BlockLoader.java | 5 + .../index/mapper/NumberFieldMapper.java | 6 +- .../elasticsearch/index/mapper/TestBlock.java | 10 ++ .../read/DelegatingBlockLoaderFactory.java | 5 + 7 files changed, 250 insertions(+), 17 deletions(-) diff --git a/modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/extras/ScaledFloatFieldMapper.java b/modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/extras/ScaledFloatFieldMapper.java index eb3372e9b6e85..f6e5f17ccadf5 100644 --- a/modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/extras/ScaledFloatFieldMapper.java +++ b/modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/extras/ScaledFloatFieldMapper.java @@ -377,7 +377,7 @@ public BlockLoader blockLoader(BlockLoaderContext blContext) { return BlockLoader.CONSTANT_NULLS; } if (hasDocValues() && (blContext.fieldExtractPreference() != FieldExtractPreference.STORED || isSyntheticSource)) { - return new BlockDocValuesReader.DoublesBlockLoader(name(), l -> l / scalingFactor); + return new BlockDocValuesReader.DoublesBlockLoader(name(), new BlockDocValuesReader.LongToScaledFloat(scalingFactor)); } // Multi fields don't have fallback synthetic source. if (isSyntheticSource && blContext.parentField(name()) == null) { diff --git a/server/src/main/java/org/elasticsearch/index/codec/tsdb/es819/ES819TSDBDocValuesProducer.java b/server/src/main/java/org/elasticsearch/index/codec/tsdb/es819/ES819TSDBDocValuesProducer.java index 3ddeca1cd25a5..c58c9122857fb 100644 --- a/server/src/main/java/org/elasticsearch/index/codec/tsdb/es819/ES819TSDBDocValuesProducer.java +++ b/server/src/main/java/org/elasticsearch/index/codec/tsdb/es819/ES819TSDBDocValuesProducer.java @@ -45,6 +45,7 @@ import org.apache.lucene.util.packed.PackedInts; import org.elasticsearch.core.IOUtils; import org.elasticsearch.index.codec.tsdb.TSDBDocValuesEncoder; +import org.elasticsearch.index.mapper.BlockDocValuesReader; import org.elasticsearch.index.mapper.BlockLoader; import java.io.IOException; @@ -515,7 +516,7 @@ BlockLoader.Block tryRead(BlockLoader.SingletonLongBuilder builder, BlockLoader. } } - abstract static class BaseSparseNumericValues extends NumericDocValues { + abstract static class BaseSparseNumericValues extends NumericDocValues implements BlockDocValuesReader.OptionalSingletonDoubles { protected final IndexedDISI disi; BaseSparseNumericValues(IndexedDISI disi) { @@ -546,6 +547,17 @@ public final int docID() { public final long cost() { return disi.cost(); } + + @Override + public BlockLoader.Block tryReadDoubles( + BlockLoader.BlockFactory factory, + BlockLoader.Docs docs, + int offset, + BlockDocValuesReader.ToDouble toDouble, + boolean nullsFiltered + ) throws IOException { + return null; + } } abstract static class BaseSortedSetDocValues extends SortedSetDocValues { @@ -1456,6 +1468,7 @@ static boolean isDense(int firstDocId, int lastDocId, int length) { ); return new BaseSparseNumericValues(disi) { private final TSDBDocValuesEncoder decoder = new TSDBDocValuesEncoder(ES819TSDBDocValuesFormat.NUMERIC_BLOCK_SIZE); + private IndexedDISI jumpDISI; private long currentBlockIndex = -1; private final long[] currentBlock = new long[ES819TSDBDocValuesFormat.NUMERIC_BLOCK_SIZE]; @@ -1479,6 +1492,69 @@ public long longValue() throws IOException { } return currentBlock[blockInIndex]; } + + @Override + public BlockLoader.Block tryReadDoubles( + BlockLoader.BlockFactory factory, + BlockLoader.Docs docs, + int offset, + BlockDocValuesReader.ToDouble toDouble, + boolean nullsFiltered + ) throws IOException { + if (nullsFiltered == false) { + return null; + } + final int firstDoc = docs.get(offset); + if (disi.advanceExact(firstDoc) == false) { + assert false : "nullsFiltered is true, but doc [" + firstDoc + "] has no value"; + throw new IllegalStateException("nullsFiltered is true, but doc [" + firstDoc + "] has no value"); + } + if (jumpDISI == null) { + jumpDISI = new IndexedDISI( + data, + entry.docsWithFieldOffset, + entry.docsWithFieldLength, + entry.jumpTableEntryCount, + entry.denseRankPower, + entry.numValues + ); + } + final int lastDoc = docs.get(docs.count() - 1); + if (jumpDISI.advanceExact(lastDoc) == false) { + assert false : "nullsFiltered is true, but doc [" + lastDoc + "] has no value"; + throw new IllegalStateException("nullsFiltered is true, but doc [" + lastDoc + "] has no value"); + } + // Assumes docIds are unique - if the number of value indices between the first + // and last doc equals the doc count, all values can be read and converted in bulk + // TODO: Pass docCount attr for enrich and lookup. + final int firstIndex = disi.index(); + final int lastIndex = jumpDISI.index(); + final int valueCount = lastIndex - firstIndex + 1; + // TODO: encode this via docCount + if (valueCount == docs.count()) { + final double[] values = new double[valueCount]; + int i = 0; + while (i < valueCount) { + final int index = firstIndex + i; + final int blockIndex = index >>> ES819TSDBDocValuesFormat.NUMERIC_BLOCK_SHIFT; + final int blockStartIndex = index & ES819TSDBDocValuesFormat.NUMERIC_BLOCK_MASK; + if (blockIndex != currentBlockIndex) { + assert blockIndex > currentBlockIndex : blockIndex + "<=" + currentBlockIndex; + if (currentBlockIndex + 1 != blockIndex) { + valuesData.seek(indexReader.get(blockIndex)); + } + currentBlockIndex = blockIndex; + decoder.decode(valuesData, currentBlock); + } + // bulk convert from the + final int count = Math.min(ES819TSDBDocValuesFormat.NUMERIC_BLOCK_SIZE - blockStartIndex, valueCount - i); + toDouble.convert(currentBlock, blockStartIndex, values, i, count); + i += count; + } + return factory.doubles(values, docs.count()); + } + return null; + } }; } } diff --git a/server/src/main/java/org/elasticsearch/index/mapper/BlockDocValuesReader.java b/server/src/main/java/org/elasticsearch/index/mapper/BlockDocValuesReader.java index 281087f703236..8ab6fe9c420e2 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/BlockDocValuesReader.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/BlockDocValuesReader.java @@ -19,8 +19,11 @@ import org.apache.lucene.index.SortedDocValues; import org.apache.lucene.index.SortedNumericDocValues; import org.apache.lucene.index.SortedSetDocValues; +import org.apache.lucene.sandbox.document.HalfFloatPoint; import org.apache.lucene.util.BytesRef; +import org.apache.lucene.util.NumericUtils; import org.elasticsearch.common.io.stream.ByteArrayStreamInput; +import org.elasticsearch.core.Nullable; import org.elasticsearch.index.IndexVersion; import org.elasticsearch.index.mapper.BlockLoader.BlockFactory; import org.elasticsearch.index.mapper.BlockLoader.BooleanBuilder; @@ -143,6 +146,7 @@ public BlockLoader.Block read(BlockFactory factory, Docs docs, int offset, boole if (numericDocValues.advanceExact(doc)) { builder.appendLong(numericDocValues.longValue()); } else { + assert nullsFiltered == false : "nullFiltered is true, but doc [" + doc + "] has no value"; builder.appendNull(); } } @@ -348,11 +352,133 @@ public String toString() { /** * Convert from the stored {@link long} into the {@link double} to load. - * Sadly, this will go megamorphic pretty quickly and slow us down, - * but it gets the job done for now. */ - public interface ToDouble { + public sealed interface ToDouble { double convert(long v); + + void convert(long[] src, int srcOffset, double[] dst, int dstOff, int count); + + BlockLoader.Block readThenConvert(BlockFactory factory, NumericDocValues dv, Docs docs, int offset) throws IOException; + + ToDouble SORTABLE_SHORT_TO_HALF_FLOAT = new SortableShortToHalfFloat(); + ToDouble SORTABLE_INT_TO_FLOAT = new SortableIntToFloat(); + ToDouble SORTABLE_LONG_TO_DOUBLE = new SortableLongToDouble(); + } + + private static final class SortableShortToHalfFloat implements ToDouble { + @Override + public double convert(long v) { + return HalfFloatPoint.sortableShortToHalfFloat((short) v); + } + + @Override + public void convert(long[] src, int srcOffset, double[] dst, int dstOff, int count) { + for (int i = 0; i < count; i++) { + dst[dstOff + i] = HalfFloatPoint.sortableShortToHalfFloat((short) src[srcOffset + i]); + } + } + + @Override + public BlockLoader.Block readThenConvert(BlockFactory factory, NumericDocValues dv, Docs docs, int offset) throws IOException { + try (BlockLoader.DoubleBuilder builder = factory.doublesFromDocValues(docs.count() - offset)) { + for (int i = offset; i < docs.count(); i++) { + int doc = docs.get(i); + if (dv.advanceExact(doc)) { + builder.appendDouble(HalfFloatPoint.sortableShortToHalfFloat((short) dv.longValue())); + } else { + builder.appendNull(); + } + } + return builder.build(); + } + } + } + + private static final class SortableIntToFloat implements BlockDocValuesReader.ToDouble { + @Override + public double convert(long v) { + return NumericUtils.sortableIntToFloat((int) v); + } + + @Override + public void convert(long[] src, int srcOffset, double[] dst, int dstOff, int count) { + for (int i = 0; i < count; i++) { + dst[dstOff + i] = NumericUtils.sortableIntToFloat((int) src[srcOffset + i]); + } + } + + @Override + public BlockLoader.Block readThenConvert(BlockFactory factory, NumericDocValues dv, Docs docs, int offset) throws IOException { + try (BlockLoader.DoubleBuilder builder = factory.doublesFromDocValues(docs.count() - offset)) { + for (int i = offset; i < docs.count(); i++) { + int doc = docs.get(i); + if (dv.advanceExact(doc)) { + builder.appendDouble(NumericUtils.sortableIntToFloat((int) dv.longValue())); + } else { + builder.appendNull(); + } + } + return builder.build(); + } + } + } + + private static final class SortableLongToDouble implements BlockDocValuesReader.ToDouble { + @Override + public double convert(long v) { + return NumericUtils.sortableLongToDouble(v); + } + + @Override + public void convert(long[] src, int srcOffset, double[] dst, int dstOff, int count) { + for (int i = 0; i < count; i++) { + dst[dstOff + i] = NumericUtils.sortableLongToDouble(src[srcOffset + i]); + } + } + + @Override + public BlockLoader.Block readThenConvert(BlockFactory factory, NumericDocValues dv, Docs docs, int offset) throws IOException { + try (BlockLoader.DoubleBuilder builder = factory.doublesFromDocValues(docs.count() - offset)) { + for (int i = offset; i < docs.count(); i++) { + int doc = docs.get(i); + if (dv.advanceExact(doc)) { + builder.appendDouble(NumericUtils.sortableLongToDouble(dv.longValue())); + } else { + builder.appendNull(); + } + } + return builder.build(); + } + } + } + + public record LongToScaledFloat(double scalingFactor) implements ToDouble { + @Override + public double convert(long v) { + return v / scalingFactor; + } + + @Override + public void convert(long[] src, int srcOffset, double[] dst, int dstOff, int count) { + for (int i = 0; i < count; i++) { + dst[dstOff + i] = src[srcOffset + i] / scalingFactor; + } + } + + @Override + public BlockLoader.Block readThenConvert(BlockFactory factory, NumericDocValues dv, Docs docs, int offset) throws IOException { + try (BlockLoader.DoubleBuilder builder = factory.doublesFromDocValues(docs.count() - offset)) { + for (int i = offset; i < docs.count(); i++) { + int doc = docs.get(i); + if (dv.advanceExact(doc)) { + builder.appendDouble(dv.longValue() / scalingFactor); + } else { + builder.appendNull(); + } + } + return builder.build(); + } + } } public static class DoublesBlockLoader extends DocValuesBlockLoader { @@ -387,6 +513,21 @@ public AllReader reader(LeafReaderContext context) throws IOException { } } + public interface OptionalSingletonDoubles { + /** + * Attempts to read the values of all documents in {@code docs} and convert them to doubles. + * Returns {@code null} if unable to load the values. + */ + @Nullable + BlockLoader.Block tryReadDoubles( + BlockFactory factory, + Docs docs, + int offset, + BlockDocValuesReader.ToDouble toDouble, + boolean nullsFiltered + ) throws IOException; + } + private static class SingletonDoubles extends BlockDocValuesReader { private final NumericDocValues docValues; private final ToDouble toDouble; @@ -398,17 +539,13 @@ private static class SingletonDoubles extends BlockDocValuesReader { @Override public BlockLoader.Block read(BlockFactory factory, Docs docs, int offset, boolean nullsFiltered) throws IOException { - try (BlockLoader.DoubleBuilder builder = factory.doublesFromDocValues(docs.count() - offset)) { - for (int i = offset; i < docs.count(); i++) { - int doc = docs.get(i); - if (docValues.advanceExact(doc)) { - builder.appendDouble(toDouble.convert(docValues.longValue())); - } else { - builder.appendNull(); - } + if (docValues instanceof OptionalSingletonDoubles direct) { + BlockLoader.Block block = direct.tryReadDoubles(factory, docs, offset, toDouble, nullsFiltered); + if (block != null) { + return block; } - return builder.build(); } + return toDouble.readThenConvert(factory, docValues, docs, offset); } @Override diff --git a/server/src/main/java/org/elasticsearch/index/mapper/BlockLoader.java b/server/src/main/java/org/elasticsearch/index/mapper/BlockLoader.java index f7f2720850ca4..1be7d7595f923 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/BlockLoader.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/BlockLoader.java @@ -397,6 +397,11 @@ interface BlockFactory { */ DoubleBuilder doubles(int expectedCount); + /** + * Creates a block from an array of doubles + */ + Block doubles(double[] values, int expectedCount); + /** * Build a builder to load dense vectors without any loading constraints. */ diff --git a/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java index fe3dac96541d2..5d7b4fec4a68d 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java @@ -483,7 +483,7 @@ public void writeValue(XContentBuilder b, long value) throws IOException { @Override BlockLoader blockLoaderFromDocValues(String fieldName) { - return new BlockDocValuesReader.DoublesBlockLoader(fieldName, l -> HalfFloatPoint.sortableShortToHalfFloat((short) l)); + return new BlockDocValuesReader.DoublesBlockLoader(fieldName, BlockDocValuesReader.ToDouble.SORTABLE_SHORT_TO_HALF_FLOAT); } @Override @@ -677,7 +677,7 @@ public void writeValue(XContentBuilder b, long value) throws IOException { @Override BlockLoader blockLoaderFromDocValues(String fieldName) { - return new BlockDocValuesReader.DoublesBlockLoader(fieldName, l -> NumericUtils.sortableIntToFloat((int) l)); + return new BlockDocValuesReader.DoublesBlockLoader(fieldName, BlockDocValuesReader.ToDouble.SORTABLE_INT_TO_FLOAT); } @Override @@ -837,7 +837,7 @@ public void writeValue(XContentBuilder b, long value) throws IOException { @Override BlockLoader blockLoaderFromDocValues(String fieldName) { - return new BlockDocValuesReader.DoublesBlockLoader(fieldName, NumericUtils::sortableLongToDouble); + return new BlockDocValuesReader.DoublesBlockLoader(fieldName, BlockDocValuesReader.ToDouble.SORTABLE_LONG_TO_DOUBLE); } @Override diff --git a/test/framework/src/main/java/org/elasticsearch/index/mapper/TestBlock.java b/test/framework/src/main/java/org/elasticsearch/index/mapper/TestBlock.java index 09d7a4147563b..e79c7ca1cb648 100644 --- a/test/framework/src/main/java/org/elasticsearch/index/mapper/TestBlock.java +++ b/test/framework/src/main/java/org/elasticsearch/index/mapper/TestBlock.java @@ -120,6 +120,16 @@ public DoublesBuilder appendDouble(double value) { return new DoublesBuilder(); } + @Override + public BlockLoader.Block doubles(double[] values, int expectedCount) { + try (BlockLoader.DoubleBuilder builder = doubles(expectedCount)) { + for (double value : values) { + builder.appendDouble(value); + } + return builder.build(); + } + } + @Override public BlockLoader.FloatBuilder denseVectors(int expectedCount, int dimensions) { class FloatsBuilder extends TestBlock.Builder implements BlockLoader.FloatBuilder { diff --git a/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/lucene/read/DelegatingBlockLoaderFactory.java b/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/lucene/read/DelegatingBlockLoaderFactory.java index b23114292680d..f3dca7adfa872 100644 --- a/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/lucene/read/DelegatingBlockLoaderFactory.java +++ b/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/lucene/read/DelegatingBlockLoaderFactory.java @@ -78,6 +78,11 @@ public BlockLoader.DoubleBuilder doubles(int expectedCount) { return factory.newDoubleBlockBuilder(expectedCount); } + @Override + public BlockLoader.Block doubles(double[] values, int expectedCount) { + return factory.newDoubleArrayVector(values, expectedCount).asBlock(); + } + @Override public BlockLoader.FloatBuilder denseVectors(int expectedVectorsCount, int dimensions) { return factory.newFloatBlockBuilder(expectedVectorsCount * dimensions); From 6bb2a4a86cc6cb7126c1d5c7f442859b299f03f7 Mon Sep 17 00:00:00 2001 From: Nhat Nguyen Date: Wed, 27 Aug 2025 15:50:52 -0700 Subject: [PATCH 2/7] delegate --- .../mapper/extras/ScaledFloatFieldMapper.java | 2 +- .../es819/ES819TSDBDocValuesProducer.java | 60 +++++++++++-------- .../index/mapper/BlockDocValuesReader.java | 6 +- .../index/mapper/BlockLoader.java | 10 +++- .../index/mapper/NumberFieldMapper.java | 6 +- .../es819/ES819TSDBDocValuesFormatTests.java | 32 +++++----- .../lucene/read/SingletonDoubleBuilder.java | 3 + .../lucene/read/SingletonLongBuilder.java | 3 + 8 files changed, 73 insertions(+), 49 deletions(-) diff --git a/modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/extras/ScaledFloatFieldMapper.java b/modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/extras/ScaledFloatFieldMapper.java index f6e5f17ccadf5..eb3372e9b6e85 100644 --- a/modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/extras/ScaledFloatFieldMapper.java +++ b/modules/mapper-extras/src/main/java/org/elasticsearch/index/mapper/extras/ScaledFloatFieldMapper.java @@ -377,7 +377,7 @@ public BlockLoader blockLoader(BlockLoaderContext blContext) { return BlockLoader.CONSTANT_NULLS; } if (hasDocValues() && (blContext.fieldExtractPreference() != FieldExtractPreference.STORED || isSyntheticSource)) { - return new BlockDocValuesReader.DoublesBlockLoader(name(), new BlockDocValuesReader.LongToScaledFloat(scalingFactor)); + return new BlockDocValuesReader.DoublesBlockLoader(name(), l -> l / scalingFactor); } // Multi fields don't have fallback synthetic source. if (isSyntheticSource && blContext.parentField(name()) == null) { diff --git a/server/src/main/java/org/elasticsearch/index/codec/tsdb/es819/ES819TSDBDocValuesProducer.java b/server/src/main/java/org/elasticsearch/index/codec/tsdb/es819/ES819TSDBDocValuesProducer.java index a1327baf285e2..ef2c5c872d86d 100644 --- a/server/src/main/java/org/elasticsearch/index/codec/tsdb/es819/ES819TSDBDocValuesProducer.java +++ b/server/src/main/java/org/elasticsearch/index/codec/tsdb/es819/ES819TSDBDocValuesProducer.java @@ -388,6 +388,7 @@ public BlockLoader.Block tryRead( BlockLoader.BlockFactory factory, BlockLoader.Docs docs, int offset, + boolean nullsFiltered, BlockDocValuesReader.ToDouble toDouble ) throws IOException { assert toDouble == null; @@ -468,6 +469,7 @@ public BlockLoader.Block tryRead( BlockLoader.BlockFactory factory, BlockLoader.Docs docs, int offset, + boolean nullsFiltered, BlockDocValuesReader.ToDouble toDouble ) throws IOException { return null; @@ -520,6 +522,7 @@ public BlockLoader.Block tryRead( BlockLoader.BlockFactory factory, BlockLoader.Docs docs, int offset, + boolean nullsFiltered, BlockDocValuesReader.ToDouble toDouble ) throws IOException { return null; @@ -532,7 +535,7 @@ BlockLoader.Block tryRead(BlockLoader.SingletonLongBuilder builder, BlockLoader. } } - abstract static class BaseSparseNumericValues extends NumericDocValues implements BlockDocValuesReader.OptionalSingletonDoubles { + abstract static class BaseSparseNumericValues extends NumericDocValues implements BlockLoader.OptionalColumnAtATimeReader { protected final IndexedDISI disi; BaseSparseNumericValues(IndexedDISI disi) { @@ -565,12 +568,12 @@ public final long cost() { } @Override - public BlockLoader.Block tryReadDoubles( + public BlockLoader.Block tryRead( BlockLoader.BlockFactory factory, BlockLoader.Docs docs, int offset, - BlockDocValuesReader.ToDouble toDouble, - boolean nullsFiltered + boolean nullsFiltered, + BlockDocValuesReader.ToDouble toDouble ) throws IOException { return null; } @@ -1397,16 +1400,11 @@ public BlockLoader.Block tryRead( BlockLoader.BlockFactory factory, BlockLoader.Docs docs, int offset, + boolean nullsFiltered, BlockDocValuesReader.ToDouble toDouble ) throws IOException { - if (toDouble != null) { - try (BlockLoader.SingletonDoubleBuilder builder = factory.singletonDoubles(docs.count() - offset)) { - SingletonLongToDoubleDelegate delegate = new SingletonLongToDoubleDelegate(builder, toDouble); - return tryRead(delegate, docs, offset); - } - } - try (BlockLoader.SingletonLongBuilder builder = factory.singletonLongs(docs.count() - offset)) { - return tryRead(builder, docs, offset); + try (var longs = singletonLongs(factory, toDouble, docs.count() - offset)) { + return tryRead(longs, docs, offset); } } @@ -1521,12 +1519,12 @@ public long longValue() throws IOException { } @Override - public BlockLoader.Block tryReadDoubles( + public BlockLoader.Block tryRead( BlockLoader.BlockFactory factory, BlockLoader.Docs docs, int offset, - BlockDocValuesReader.ToDouble toDouble, - boolean nullsFiltered + boolean nullsFiltered, + BlockDocValuesReader.ToDouble toDouble ) throws IOException { if (nullsFiltered == false) { return null; @@ -1557,11 +1555,11 @@ public BlockLoader.Block tryReadDoubles( final int firstIndex = disi.index(); final int lastIndex = jumpDISI.index(); final int valueCount = lastIndex - firstIndex + 1; - // TODO: encode this via docCount - if (valueCount == docs.count()) { - final double[] values = new double[valueCount]; - int i = 0; - while (i < valueCount) { + if (valueCount != docs.count()) { + return null; + } + try (var longs = singletonLongs(factory, toDouble, valueCount)) { + for (int i = 0; i < valueCount;) { final int index = firstIndex + i; final int blockIndex = index >>> ES819TSDBDocValuesFormat.NUMERIC_BLOCK_SHIFT; final int blockStartIndex = index & ES819TSDBDocValuesFormat.NUMERIC_BLOCK_MASK; @@ -1573,14 +1571,12 @@ public BlockLoader.Block tryReadDoubles( currentBlockIndex = blockIndex; decoder.decode(valuesData, currentBlock); } - // bulk convert from the final int count = Math.min(ES819TSDBDocValuesFormat.NUMERIC_BLOCK_SIZE - blockStartIndex, valueCount - i); - toDouble.convert(currentBlock, blockStartIndex, values, i, count); + longs.appendLongs(currentBlock, blockStartIndex, count); i += count; } - return factory.doubles(values, docs.count()); + return longs.build(); } - return null; } }; } @@ -1877,6 +1873,18 @@ public BlockLoader.Builder endPositionEntry() { public void close() {} } + static BlockLoader.SingletonLongBuilder singletonLongs( + BlockLoader.BlockFactory factory, + BlockDocValuesReader.ToDouble toDouble, + int valueCount + ) { + if (toDouble != null) { + return new SingletonLongToDoubleDelegate(factory.singletonDoubles(valueCount), toDouble); + } else { + return factory.singletonLongs(valueCount); + } + } + // Block builder that consumes long values and converts them to double using the provided converter function. static final class SingletonLongToDoubleDelegate implements BlockLoader.SingletonLongBuilder { private final BlockLoader.SingletonDoubleBuilder doubleBuilder; @@ -1925,7 +1933,9 @@ public BlockLoader.Builder endPositionEntry() { } @Override - public void close() {} + public void close() { + doubleBuilder.close(); + } } } diff --git a/server/src/main/java/org/elasticsearch/index/mapper/BlockDocValuesReader.java b/server/src/main/java/org/elasticsearch/index/mapper/BlockDocValuesReader.java index 06513444737f3..f2324d70ce750 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/BlockDocValuesReader.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/BlockDocValuesReader.java @@ -137,7 +137,7 @@ static class SingletonLongs extends BlockDocValuesReader implements NumericDocVa @Override public BlockLoader.Block read(BlockFactory factory, Docs docs, int offset, boolean nullsFiltered) throws IOException { if (numericDocValues instanceof BlockLoader.OptionalColumnAtATimeReader direct) { - BlockLoader.Block result = direct.tryRead(factory, docs, offset, null); + BlockLoader.Block result = direct.tryRead(factory, docs, offset, nullsFiltered, null); if (result != null) { return result; } @@ -409,7 +409,7 @@ static class SingletonDoubles extends BlockDocValuesReader implements NumericDoc @Override public BlockLoader.Block read(BlockFactory factory, Docs docs, int offset, boolean nullsFiltered) throws IOException { if (docValues instanceof BlockLoader.OptionalColumnAtATimeReader direct) { - BlockLoader.Block result = direct.tryRead(factory, docs, offset, toDouble); + BlockLoader.Block result = direct.tryRead(factory, docs, offset, nullsFiltered, toDouble); if (result != null) { return result; } @@ -736,7 +736,7 @@ public BlockLoader.Block read(BlockFactory factory, Docs docs, int offset, boole return readSingleDoc(factory, docs.get(offset)); } if (ordinals instanceof BlockLoader.OptionalColumnAtATimeReader direct) { - BlockLoader.Block block = direct.tryRead(factory, docs, offset, null); + BlockLoader.Block block = direct.tryRead(factory, docs, offset, nullsFiltered, null); if (block != null) { return block; } diff --git a/server/src/main/java/org/elasticsearch/index/mapper/BlockLoader.java b/server/src/main/java/org/elasticsearch/index/mapper/BlockLoader.java index 0d40632d8155f..5a9c48c3adcbf 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/BlockLoader.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/BlockLoader.java @@ -66,10 +66,18 @@ interface OptionalColumnAtATimeReader { * Attempts to read the values of all documents in {@code docs} * Returns {@code null} if unable to load the values. * + * @param nullsFiltered if {@code true}, then target docs are guaranteed to have a value for the field. + * see {@link ColumnAtATimeReader#read(BlockFactory, Docs, int, boolean)} * @param toDouble a function to convert long values to double, or null if no conversion is needed/supported */ @Nullable - BlockLoader.Block tryRead(BlockFactory factory, Docs docs, int offset, BlockDocValuesReader.ToDouble toDouble) throws IOException; + BlockLoader.Block tryRead( + BlockFactory factory, + Docs docs, + int offset, + boolean nullsFiltered, + BlockDocValuesReader.ToDouble toDouble + ) throws IOException; } interface RowStrideReader extends Reader { diff --git a/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java b/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java index 5d7b4fec4a68d..fe3dac96541d2 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/NumberFieldMapper.java @@ -483,7 +483,7 @@ public void writeValue(XContentBuilder b, long value) throws IOException { @Override BlockLoader blockLoaderFromDocValues(String fieldName) { - return new BlockDocValuesReader.DoublesBlockLoader(fieldName, BlockDocValuesReader.ToDouble.SORTABLE_SHORT_TO_HALF_FLOAT); + return new BlockDocValuesReader.DoublesBlockLoader(fieldName, l -> HalfFloatPoint.sortableShortToHalfFloat((short) l)); } @Override @@ -677,7 +677,7 @@ public void writeValue(XContentBuilder b, long value) throws IOException { @Override BlockLoader blockLoaderFromDocValues(String fieldName) { - return new BlockDocValuesReader.DoublesBlockLoader(fieldName, BlockDocValuesReader.ToDouble.SORTABLE_INT_TO_FLOAT); + return new BlockDocValuesReader.DoublesBlockLoader(fieldName, l -> NumericUtils.sortableIntToFloat((int) l)); } @Override @@ -837,7 +837,7 @@ public void writeValue(XContentBuilder b, long value) throws IOException { @Override BlockLoader blockLoaderFromDocValues(String fieldName) { - return new BlockDocValuesReader.DoublesBlockLoader(fieldName, BlockDocValuesReader.ToDouble.SORTABLE_LONG_TO_DOUBLE); + return new BlockDocValuesReader.DoublesBlockLoader(fieldName, NumericUtils::sortableLongToDouble); } @Override diff --git a/server/src/test/java/org/elasticsearch/index/codec/tsdb/es819/ES819TSDBDocValuesFormatTests.java b/server/src/test/java/org/elasticsearch/index/codec/tsdb/es819/ES819TSDBDocValuesFormatTests.java index 2b23d62d72803..17fb74be3c67c 100644 --- a/server/src/test/java/org/elasticsearch/index/codec/tsdb/es819/ES819TSDBDocValuesFormatTests.java +++ b/server/src/test/java/org/elasticsearch/index/codec/tsdb/es819/ES819TSDBDocValuesFormatTests.java @@ -773,7 +773,7 @@ public void testOptionalColumnAtATimeReader() throws Exception { { // bulk loading timestamp: - var block = (TestBlock) timestampDV.tryRead(factory, docs, 0, null); + var block = (TestBlock) timestampDV.tryRead(factory, docs, 0, false, null); assertNotNull(block); assertEquals(size, block.size()); for (int j = 0; j < block.size(); j++) { @@ -785,10 +785,10 @@ public void testOptionalColumnAtATimeReader() throws Exception { } { // bulk loading counter field: - var block = (TestBlock) counterDV.tryRead(factory, docs, 0, null); + var block = (TestBlock) counterDV.tryRead(factory, docs, 0, false, null); assertNotNull(block); assertEquals(size, block.size()); - var stringBlock = (TestBlock) stringCounterDV.tryRead(factory, docs, 0, null); + var stringBlock = (TestBlock) stringCounterDV.tryRead(factory, docs, 0, false, null); assertNotNull(stringBlock); assertEquals(size, stringBlock.size()); for (int j = 0; j < block.size(); j++) { @@ -805,7 +805,7 @@ public void testOptionalColumnAtATimeReader() throws Exception { } { // bulk loading gauge field: - var block = (TestBlock) gaugeDV.tryRead(factory, docs, 0, null); + var block = (TestBlock) gaugeDV.tryRead(factory, docs, 0, false, null); assertNotNull(block); assertEquals(size, block.size()); for (int j = 0; j < block.size(); j++) { @@ -843,7 +843,7 @@ public void testOptionalColumnAtATimeReader() throws Exception { { // bulk loading timestamp: - var block = (TestBlock) timestampDV.tryRead(blockFactory, docs, randomOffset, null); + var block = (TestBlock) timestampDV.tryRead(blockFactory, docs, randomOffset, false, null); assertNotNull(block); assertEquals(size, block.size()); for (int j = 0; j < block.size(); j++) { @@ -855,11 +855,11 @@ public void testOptionalColumnAtATimeReader() throws Exception { } { // bulk loading counter field: - var block = (TestBlock) counterDV.tryRead(factory, docs, randomOffset, null); + var block = (TestBlock) counterDV.tryRead(factory, docs, randomOffset, false, null); assertNotNull(block); assertEquals(size, block.size()); - var stringBlock = (TestBlock) stringCounterDV.tryRead(factory, docs, randomOffset, null); + var stringBlock = (TestBlock) stringCounterDV.tryRead(factory, docs, randomOffset, false, null); assertNotNull(stringBlock); assertEquals(size, stringBlock.size()); @@ -877,7 +877,7 @@ public void testOptionalColumnAtATimeReader() throws Exception { } { // bulk loading gauge field: - var block = (TestBlock) gaugeDV.tryRead(factory, docs, randomOffset, null); + var block = (TestBlock) gaugeDV.tryRead(factory, docs, randomOffset, false, null); assertNotNull(block); assertEquals(size, block.size()); for (int j = 0; j < block.size(); j++) { @@ -902,11 +902,11 @@ public void testOptionalColumnAtATimeReader() throws Exception { stringCounterDV = getBaseSortedDocValues(leafReader, counterFieldAsString); { // bulk loading counter field: - var block = (TestBlock) counterDV.tryRead(factory, docs, 0, null); + var block = (TestBlock) counterDV.tryRead(factory, docs, 0, false, null); assertNotNull(block); assertEquals(size, block.size()); - var stringBlock = (TestBlock) stringCounterDV.tryRead(factory, docs, 0, null); + var stringBlock = (TestBlock) stringCounterDV.tryRead(factory, docs, 0, false, null); assertNotNull(stringBlock); assertEquals(size, stringBlock.size()); @@ -1001,7 +1001,7 @@ public void testOptionalColumnAtATimeReaderWithSparseDocs() throws Exception { var docs = TestBlock.docs(docIds); { timestampDV = getBaseDenseNumericValues(leafReader, timestampField); - var block = (TestBlock) timestampDV.tryRead(factory, docs, 0, null); + var block = (TestBlock) timestampDV.tryRead(factory, docs, 0, false, null); assertNotNull(block); assertEquals(numDocsPerQValue, block.size()); for (int j = 0; j < block.size(); j++) { @@ -1012,7 +1012,7 @@ public void testOptionalColumnAtATimeReaderWithSparseDocs() throws Exception { } { counterDV = getBaseDenseNumericValues(leafReader, counterField); - var block = (TestBlock) counterDV.tryRead(factory, docs, 0, null); + var block = (TestBlock) counterDV.tryRead(factory, docs, 0, false, null); assertNotNull(block); assertEquals(numDocsPerQValue, block.size()); for (int j = 0; j < block.size(); j++) { @@ -1023,7 +1023,7 @@ public void testOptionalColumnAtATimeReaderWithSparseDocs() throws Exception { } { counterAsStringDV = getBaseSortedDocValues(leafReader, counterAsStringField); - var block = (TestBlock) counterAsStringDV.tryRead(factory, docs, 0, null); + var block = (TestBlock) counterAsStringDV.tryRead(factory, docs, 0, false, null); assertNotNull(block); assertEquals(numDocsPerQValue, block.size()); for (int j = 0; j < block.size(); j++) { @@ -1086,7 +1086,7 @@ public int get(int i) { } }; var idReader = ESTestCase.asInstanceOf(OptionalColumnAtATimeReader.class, leaf.reader().getNumericDocValues("id")); - TestBlock idBlock = (TestBlock) idReader.tryRead(factory, docs, 0, null); + TestBlock idBlock = (TestBlock) idReader.tryRead(factory, docs, 0, false, null); assertNotNull(idBlock); { @@ -1100,7 +1100,7 @@ public int get(int i) { block = (TestBlock) reader2.tryReadAHead(factory, docs, randomOffset); } else { assertNull(reader2.tryReadAHead(factory, docs, randomOffset)); - block = (TestBlock) reader2.tryRead(factory, docs, randomOffset, null); + block = (TestBlock) reader2.tryRead(factory, docs, randomOffset, false, null); } assertNotNull(block); assertThat(block.size(), equalTo(docs.count() - randomOffset)); @@ -1122,7 +1122,7 @@ public int get(int i) { block = (TestBlock) reader3.tryReadAHead(factory, docs, randomOffset); } else { assertNull(reader3.tryReadAHead(factory, docs, randomOffset)); - block = (TestBlock) reader3.tryRead(factory, docs, randomOffset, null); + block = (TestBlock) reader3.tryRead(factory, docs, randomOffset, false, null); } assertNotNull(reader3); assertNotNull(block); diff --git a/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/lucene/read/SingletonDoubleBuilder.java b/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/lucene/read/SingletonDoubleBuilder.java index 576561a11946f..e3a2ae64f5119 100644 --- a/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/lucene/read/SingletonDoubleBuilder.java +++ b/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/lucene/read/SingletonDoubleBuilder.java @@ -65,6 +65,9 @@ public long estimatedBytes() { @Override public Block build() { + if (values.length != count) { + throw new IllegalStateException("expected " + values.length + " values but got " + count); + } return blockFactory.newDoubleArrayVector(values, count).asBlock(); } diff --git a/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/lucene/read/SingletonLongBuilder.java b/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/lucene/read/SingletonLongBuilder.java index 84df5feb09ff5..de076f8b5d269 100644 --- a/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/lucene/read/SingletonLongBuilder.java +++ b/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/lucene/read/SingletonLongBuilder.java @@ -65,6 +65,9 @@ public long estimatedBytes() { @Override public Block build() { + if (values.length != count) { + throw new IllegalStateException("expected [" + values.length + "] values but got [" + count + "]"); + } return blockFactory.newLongArrayVector(values, count).asBlock(); } From 5ba87912f7118f37a7bbe16b7103606048d26ec7 Mon Sep 17 00:00:00 2001 From: Nhat Nguyen Date: Wed, 27 Aug 2025 17:12:10 -0700 Subject: [PATCH 3/7] naming --- .../codec/tsdb/es819/ES819TSDBDocValuesProducer.java | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/codec/tsdb/es819/ES819TSDBDocValuesProducer.java b/server/src/main/java/org/elasticsearch/index/codec/tsdb/es819/ES819TSDBDocValuesProducer.java index ef2c5c872d86d..c417727b94ca9 100644 --- a/server/src/main/java/org/elasticsearch/index/codec/tsdb/es819/ES819TSDBDocValuesProducer.java +++ b/server/src/main/java/org/elasticsearch/index/codec/tsdb/es819/ES819TSDBDocValuesProducer.java @@ -1493,7 +1493,7 @@ static boolean isDense(int firstDocId, int lastDocId, int length) { ); return new BaseSparseNumericValues(disi) { private final TSDBDocValuesEncoder decoder = new TSDBDocValuesEncoder(ES819TSDBDocValuesFormat.NUMERIC_BLOCK_SIZE); - private IndexedDISI jumpDISI; + private IndexedDISI lookAheadDISI; private long currentBlockIndex = -1; private final long[] currentBlock = new long[ES819TSDBDocValuesFormat.NUMERIC_BLOCK_SIZE]; @@ -1534,8 +1534,8 @@ public BlockLoader.Block tryRead( assert false : "nullsFiltered is true, but doc [" + firstDoc + "] has no value"; throw new IllegalStateException("nullsFiltered is true, but doc [" + firstDoc + "] has no value"); } - if (jumpDISI == null) { - jumpDISI = new IndexedDISI( + if (lookAheadDISI == null) { + lookAheadDISI = new IndexedDISI( data, entry.docsWithFieldOffset, entry.docsWithFieldLength, @@ -1545,7 +1545,7 @@ public BlockLoader.Block tryRead( ); } final int lastDoc = docs.get(docs.count() - 1); - if (jumpDISI.advanceExact(lastDoc) == false) { + if (lookAheadDISI.advanceExact(lastDoc) == false) { assert false : "nullsFiltered is true, but doc [" + lastDoc + "] has no value"; throw new IllegalStateException("nullsFiltered is true, but doc [" + lastDoc + "] has no value"); } @@ -1553,7 +1553,7 @@ public BlockLoader.Block tryRead( // and last doc equals the doc count, all values can be read and converted in bulk // TODO: Pass docCount attr for enrich and lookup. final int firstIndex = disi.index(); - final int lastIndex = jumpDISI.index(); + final int lastIndex = lookAheadDISI.index(); final int valueCount = lastIndex - firstIndex + 1; if (valueCount != docs.count()) { return null; From b3b9dbb1e59e515b759a335977b5294779f8b1cd Mon Sep 17 00:00:00 2001 From: Nhat Nguyen Date: Wed, 27 Aug 2025 18:37:38 -0700 Subject: [PATCH 4/7] tests --- .../es819/ES819TSDBDocValuesProducer.java | 8 +++ .../es819/ES819TSDBDocValuesFormatTests.java | 54 ++++++++++++++++--- .../index/mapper/MapperTestCase.java | 18 ++++--- 3 files changed, 66 insertions(+), 14 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/codec/tsdb/es819/ES819TSDBDocValuesProducer.java b/server/src/main/java/org/elasticsearch/index/codec/tsdb/es819/ES819TSDBDocValuesProducer.java index c417727b94ca9..841a052a70009 100644 --- a/server/src/main/java/org/elasticsearch/index/codec/tsdb/es819/ES819TSDBDocValuesProducer.java +++ b/server/src/main/java/org/elasticsearch/index/codec/tsdb/es819/ES819TSDBDocValuesProducer.java @@ -43,6 +43,7 @@ import org.apache.lucene.util.compress.LZ4; import org.apache.lucene.util.packed.DirectMonotonicReader; import org.apache.lucene.util.packed.PackedInts; +import org.elasticsearch.core.Assertions; import org.elasticsearch.core.IOUtils; import org.elasticsearch.index.codec.tsdb.TSDBDocValuesEncoder; import org.elasticsearch.index.mapper.BlockDocValuesReader; @@ -1558,6 +1559,13 @@ public BlockLoader.Block tryRead( if (valueCount != docs.count()) { return null; } + if (Assertions.ENABLED) { + for (int i = 0; i < docs.count(); i++) { + final int doc = docs.get(i + offset); + assert disi.advanceExact(doc) : "nullsFiltered is true, but doc [" + doc + "] has no value"; + assert disi.index() == firstIndex + i : "unexpected disi index " + (firstIndex + i) + "!=" + disi.index(); + } + } try (var longs = singletonLongs(factory, toDouble, valueCount)) { for (int i = 0; i < valueCount;) { final int index = firstIndex + i; diff --git a/server/src/test/java/org/elasticsearch/index/codec/tsdb/es819/ES819TSDBDocValuesFormatTests.java b/server/src/test/java/org/elasticsearch/index/codec/tsdb/es819/ES819TSDBDocValuesFormatTests.java index 17fb74be3c67c..80ea6c5187153 100644 --- a/server/src/test/java/org/elasticsearch/index/codec/tsdb/es819/ES819TSDBDocValuesFormatTests.java +++ b/server/src/test/java/org/elasticsearch/index/codec/tsdb/es819/ES819TSDBDocValuesFormatTests.java @@ -60,6 +60,7 @@ import java.util.stream.IntStream; import static org.hamcrest.Matchers.equalTo; +import static org.hamcrest.Matchers.instanceOf; public class ES819TSDBDocValuesFormatTests extends ES87TSDBDocValuesFormatTests { @@ -773,7 +774,7 @@ public void testOptionalColumnAtATimeReader() throws Exception { { // bulk loading timestamp: - var block = (TestBlock) timestampDV.tryRead(factory, docs, 0, false, null); + var block = (TestBlock) timestampDV.tryRead(factory, docs, 0, random().nextBoolean(), null); assertNotNull(block); assertEquals(size, block.size()); for (int j = 0; j < block.size(); j++) { @@ -785,10 +786,10 @@ public void testOptionalColumnAtATimeReader() throws Exception { } { // bulk loading counter field: - var block = (TestBlock) counterDV.tryRead(factory, docs, 0, false, null); + var block = (TestBlock) counterDV.tryRead(factory, docs, 0, random().nextBoolean(), null); assertNotNull(block); assertEquals(size, block.size()); - var stringBlock = (TestBlock) stringCounterDV.tryRead(factory, docs, 0, false, null); + var stringBlock = (TestBlock) stringCounterDV.tryRead(factory, docs, 0, random().nextBoolean(), null); assertNotNull(stringBlock); assertEquals(size, stringBlock.size()); for (int j = 0; j < block.size(); j++) { @@ -805,7 +806,7 @@ public void testOptionalColumnAtATimeReader() throws Exception { } { // bulk loading gauge field: - var block = (TestBlock) gaugeDV.tryRead(factory, docs, 0, false, null); + var block = (TestBlock) gaugeDV.tryRead(factory, docs, 0, random().nextBoolean(), null); assertNotNull(block); assertEquals(size, block.size()); for (int j = 0; j < block.size(); j++) { @@ -929,6 +930,7 @@ public void testOptionalColumnAtATimeReaderWithSparseDocs() throws Exception { final String counterAsStringField = "counter_as_string"; final String timestampField = "@timestamp"; String queryField = "query_field"; + String temperatureField = "temperature_field"; long currentTimestamp = 1704067200000L; long currentCounter = 10_000_000; @@ -936,6 +938,7 @@ public void testOptionalColumnAtATimeReaderWithSparseDocs() throws Exception { try (var dir = newDirectory(); var iw = new IndexWriter(dir, config)) { int numDocsPerQValue = 120; int numDocs = numDocsPerQValue * (1 + random().nextInt(40)); + Long[] temperatureValues = new Long[numDocs]; long q = 1; for (int i = 1; i <= numDocs; i++) { @@ -949,7 +952,11 @@ public void testOptionalColumnAtATimeReaderWithSparseDocs() throws Exception { if (i % 120 == 0) { q++; } - + if (random().nextBoolean()) { + long v = random().nextLong(); + temperatureValues[numDocs - i] = v; + d.add(new NumericDocValuesField(temperatureField, v)); + } iw.addDocument(d); if (i % 100 == 0) { iw.commit(); @@ -1001,7 +1008,7 @@ public void testOptionalColumnAtATimeReaderWithSparseDocs() throws Exception { var docs = TestBlock.docs(docIds); { timestampDV = getBaseDenseNumericValues(leafReader, timestampField); - var block = (TestBlock) timestampDV.tryRead(factory, docs, 0, false, null); + var block = (TestBlock) timestampDV.tryRead(factory, docs, 0, random().nextBoolean(), null); assertNotNull(block); assertEquals(numDocsPerQValue, block.size()); for (int j = 0; j < block.size(); j++) { @@ -1012,7 +1019,7 @@ public void testOptionalColumnAtATimeReaderWithSparseDocs() throws Exception { } { counterDV = getBaseDenseNumericValues(leafReader, counterField); - var block = (TestBlock) counterDV.tryRead(factory, docs, 0, false, null); + var block = (TestBlock) counterDV.tryRead(factory, docs, 0, random().nextBoolean(), null); assertNotNull(block); assertEquals(numDocsPerQValue, block.size()); for (int j = 0; j < block.size(); j++) { @@ -1023,7 +1030,7 @@ public void testOptionalColumnAtATimeReaderWithSparseDocs() throws Exception { } { counterAsStringDV = getBaseSortedDocValues(leafReader, counterAsStringField); - var block = (TestBlock) counterAsStringDV.tryRead(factory, docs, 0, false, null); + var block = (TestBlock) counterAsStringDV.tryRead(factory, docs, 0, random().nextBoolean(), null); assertNotNull(block); assertEquals(numDocsPerQValue, block.size()); for (int j = 0; j < block.size(); j++) { @@ -1032,6 +1039,37 @@ public void testOptionalColumnAtATimeReaderWithSparseDocs() throws Exception { assertEquals(expectedCounter, actualCounter); } } + { + int startIndex = ESTestCase.between(0, temperatureValues.length - 1); + int endIndex = ESTestCase.between(startIndex + 1, temperatureValues.length); + List testDocs = new ArrayList<>(); + for (int i = startIndex; i < endIndex; i++) { + if (temperatureValues[i] != null) { + testDocs.add(i); + } + } + if (testDocs.isEmpty() == false) { + NumericDocValues dv = leafReader.getNumericDocValues(temperatureField); + assertThat(dv, instanceOf(OptionalColumnAtATimeReader.class)); + OptionalColumnAtATimeReader directReader = (OptionalColumnAtATimeReader) dv; + docs = TestBlock.docs(testDocs.stream().mapToInt(n -> n).toArray()); + assertNull(directReader.tryRead(factory, docs, 0, false, null)); + TestBlock block = (TestBlock) directReader.tryRead(factory, docs, 0, true, null); + assertNotNull(block); + for (int i = 0; i < testDocs.size(); i++) { + assertThat(block.get(i), equalTo(temperatureValues[testDocs.get(i)])); + } + } + if (testDocs.size() > 2) { + // currently bulk loading is disabled with gaps + testDocs.remove(ESTestCase.between(1, testDocs.size() - 2)); + docs = TestBlock.docs(testDocs.stream().mapToInt(n -> n).toArray()); + NumericDocValues dv = leafReader.getNumericDocValues(temperatureField); + OptionalColumnAtATimeReader directReader = (OptionalColumnAtATimeReader) dv; + assertNull(directReader.tryRead(factory, docs, 0, false, null)); + assertNull(directReader.tryRead(factory, docs, 0, true, null)); + } + } } } } diff --git a/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperTestCase.java b/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperTestCase.java index bcd45b8131392..9ea67821c181b 100644 --- a/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperTestCase.java +++ b/test/framework/src/main/java/org/elasticsearch/index/mapper/MapperTestCase.java @@ -93,7 +93,6 @@ import static org.hamcrest.Matchers.equalTo; import static org.hamcrest.Matchers.hasSize; import static org.hamcrest.Matchers.instanceOf; -import static org.hamcrest.Matchers.not; import static org.hamcrest.Matchers.nullValue; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; @@ -1560,7 +1559,7 @@ private void testSingletonBulkBlockReading(Function Date: Wed, 27 Aug 2025 20:59:39 -0700 Subject: [PATCH 5/7] avoid buffer --- .../es819/ES819TSDBDocValuesProducer.java | 19 ++++++--------- .../index/mapper/BlockLoader.java | 7 ++---- .../elasticsearch/index/mapper/TestBlock.java | 24 +++++++++++-------- .../read/DelegatingBlockLoaderFactory.java | 5 ---- .../lucene/read/SingletonDoubleBuilder.java | 10 ++++++++ .../read/SingletonDoubleBuilderTests.java | 2 ++ 6 files changed, 35 insertions(+), 32 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/codec/tsdb/es819/ES819TSDBDocValuesProducer.java b/server/src/main/java/org/elasticsearch/index/codec/tsdb/es819/ES819TSDBDocValuesProducer.java index 841a052a70009..4c3477b925c5f 100644 --- a/server/src/main/java/org/elasticsearch/index/codec/tsdb/es819/ES819TSDBDocValuesProducer.java +++ b/server/src/main/java/org/elasticsearch/index/codec/tsdb/es819/ES819TSDBDocValuesProducer.java @@ -1404,8 +1404,8 @@ public BlockLoader.Block tryRead( boolean nullsFiltered, BlockDocValuesReader.ToDouble toDouble ) throws IOException { - try (var longs = singletonLongs(factory, toDouble, docs.count() - offset)) { - return tryRead(longs, docs, offset); + try (var singletonLongBuilder = singletonLongBuilder(factory, toDouble, docs.count() - offset)) { + return tryRead(singletonLongBuilder, docs, offset); } } @@ -1566,7 +1566,7 @@ public BlockLoader.Block tryRead( assert disi.index() == firstIndex + i : "unexpected disi index " + (firstIndex + i) + "!=" + disi.index(); } } - try (var longs = singletonLongs(factory, toDouble, valueCount)) { + try (var singletonLongBuilder = singletonLongBuilder(factory, toDouble, valueCount)) { for (int i = 0; i < valueCount;) { final int index = firstIndex + i; final int blockIndex = index >>> ES819TSDBDocValuesFormat.NUMERIC_BLOCK_SHIFT; @@ -1580,10 +1580,10 @@ public BlockLoader.Block tryRead( decoder.decode(valuesData, currentBlock); } final int count = Math.min(ES819TSDBDocValuesFormat.NUMERIC_BLOCK_SIZE - blockStartIndex, valueCount - i); - longs.appendLongs(currentBlock, blockStartIndex, count); + singletonLongBuilder.appendLongs(currentBlock, blockStartIndex, count); i += count; } - return longs.build(); + return singletonLongBuilder.build(); } } }; @@ -1881,7 +1881,7 @@ public BlockLoader.Builder endPositionEntry() { public void close() {} } - static BlockLoader.SingletonLongBuilder singletonLongs( + static BlockLoader.SingletonLongBuilder singletonLongBuilder( BlockLoader.BlockFactory factory, BlockDocValuesReader.ToDouble toDouble, int valueCount @@ -1897,7 +1897,6 @@ static BlockLoader.SingletonLongBuilder singletonLongs( static final class SingletonLongToDoubleDelegate implements BlockLoader.SingletonLongBuilder { private final BlockLoader.SingletonDoubleBuilder doubleBuilder; private final BlockDocValuesReader.ToDouble toDouble; - private final double[] buffer = new double[ES819TSDBDocValuesFormat.NUMERIC_BLOCK_SIZE]; // The passed builder is used to store the converted double values and produce the final block containing them. SingletonLongToDoubleDelegate(BlockLoader.SingletonDoubleBuilder doubleBuilder, BlockDocValuesReader.ToDouble toDouble) { @@ -1912,11 +1911,7 @@ public BlockLoader.SingletonLongBuilder appendLong(long value) { @Override public BlockLoader.SingletonLongBuilder appendLongs(long[] values, int from, int length) { - assert length <= buffer.length : "length " + length + " > " + buffer.length; - for (int i = 0; i < length; i++) { - buffer[i] = toDouble.convert(values[from + i]); - } - doubleBuilder.appendDoubles(buffer, 0, length); + doubleBuilder.appendLongs(toDouble, values, from, length); return this; } diff --git a/server/src/main/java/org/elasticsearch/index/mapper/BlockLoader.java b/server/src/main/java/org/elasticsearch/index/mapper/BlockLoader.java index 5a9c48c3adcbf..5c8eb75ea5048 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/BlockLoader.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/BlockLoader.java @@ -407,11 +407,6 @@ interface BlockFactory { */ DoubleBuilder doubles(int expectedCount); - /** - * Creates a block from an array of doubles - */ - Block doubles(double[] values, int expectedCount); - /** * Build a builder to load dense vectors without any loading constraints. */ @@ -575,6 +570,8 @@ interface SingletonDoubleBuilder extends Builder { SingletonDoubleBuilder appendDouble(double value); SingletonDoubleBuilder appendDoubles(double[] values, int from, int length); + + SingletonDoubleBuilder appendLongs(BlockDocValuesReader.ToDouble toDouble, long[] values, int from, int length); } interface LongBuilder extends Builder { diff --git a/test/framework/src/main/java/org/elasticsearch/index/mapper/TestBlock.java b/test/framework/src/main/java/org/elasticsearch/index/mapper/TestBlock.java index eb3de69969282..be34117c5f97d 100644 --- a/test/framework/src/main/java/org/elasticsearch/index/mapper/TestBlock.java +++ b/test/framework/src/main/java/org/elasticsearch/index/mapper/TestBlock.java @@ -120,16 +120,6 @@ public DoublesBuilder appendDouble(double value) { return new DoublesBuilder(); } - @Override - public BlockLoader.Block doubles(double[] values, int expectedCount) { - try (BlockLoader.DoubleBuilder builder = doubles(expectedCount)) { - for (double value : values) { - builder.appendDouble(value); - } - return builder.build(); - } - } - @Override public BlockLoader.FloatBuilder denseVectors(int expectedCount, int dimensions) { class FloatsBuilder extends TestBlock.Builder implements BlockLoader.FloatBuilder { @@ -286,6 +276,20 @@ public BlockLoader.SingletonDoubleBuilder appendDouble(double value) { return this; } + @Override + public BlockLoader.SingletonDoubleBuilder appendLongs( + BlockDocValuesReader.ToDouble toDouble, + long[] longValues, + int from, + int length + ) { + for (int i = 0; i < length; i++) { + values[count + i] = toDouble.convert(longValues[from + i]); + } + this.count += length; + return this; + } + @Override public BlockLoader.Builder appendNull() { throw new UnsupportedOperationException(); diff --git a/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/lucene/read/DelegatingBlockLoaderFactory.java b/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/lucene/read/DelegatingBlockLoaderFactory.java index ae74e06e1a658..724d65f207840 100644 --- a/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/lucene/read/DelegatingBlockLoaderFactory.java +++ b/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/lucene/read/DelegatingBlockLoaderFactory.java @@ -78,11 +78,6 @@ public BlockLoader.DoubleBuilder doubles(int expectedCount) { return factory.newDoubleBlockBuilder(expectedCount); } - @Override - public BlockLoader.Block doubles(double[] values, int expectedCount) { - return factory.newDoubleArrayVector(values, expectedCount).asBlock(); - } - @Override public BlockLoader.FloatBuilder denseVectors(int expectedVectorsCount, int dimensions) { return factory.newFloatBlockBuilder(expectedVectorsCount * dimensions); diff --git a/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/lucene/read/SingletonDoubleBuilder.java b/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/lucene/read/SingletonDoubleBuilder.java index e3a2ae64f5119..ae34064faf2fc 100644 --- a/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/lucene/read/SingletonDoubleBuilder.java +++ b/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/lucene/read/SingletonDoubleBuilder.java @@ -10,6 +10,7 @@ import org.elasticsearch.compute.data.Block; import org.elasticsearch.compute.data.BlockFactory; import org.elasticsearch.core.Releasable; +import org.elasticsearch.index.mapper.BlockDocValuesReader; import org.elasticsearch.index.mapper.BlockLoader; /** @@ -84,6 +85,15 @@ public BlockLoader.SingletonDoubleBuilder appendDoubles(double[] values, int fro return this; } + @Override + public BlockLoader.SingletonDoubleBuilder appendLongs(BlockDocValuesReader.ToDouble toDouble, long[] longValues, int from, int length) { + for (int i = 0; i < length; i++) { + values[count + i] = toDouble.convert(longValues[from + i]); + } + this.count += length; + return this; + } + @Override public void close() { blockFactory.adjustBreaker(-valuesSize(values.length)); diff --git a/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/lucene/read/SingletonDoubleBuilderTests.java b/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/lucene/read/SingletonDoubleBuilderTests.java index 9c6d376da3e8a..393542cd46d70 100644 --- a/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/lucene/read/SingletonDoubleBuilderTests.java +++ b/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/lucene/read/SingletonDoubleBuilderTests.java @@ -72,6 +72,8 @@ private void testRead(BlockFactory factory) throws IOException { double value = Double.longBitsToDouble(docValues.longValue()); if (randomBoolean()) { builder.appendDoubles(new double[] { value }, 0, 1); + } else if (randomBoolean()) { + builder.appendLongs(Double::longBitsToDouble, new long[] { docValues.longValue() }, 0, 1); } else { builder.appendDouble(value); } From 93942caa67d089482de7696e5e4cd516ded33ba2 Mon Sep 17 00:00:00 2001 From: Nhat Nguyen Date: Wed, 27 Aug 2025 23:12:01 -0700 Subject: [PATCH 6/7] Update docs/changelog/133365.yaml --- docs/changelog/133365.yaml | 5 +++++ 1 file changed, 5 insertions(+) create mode 100644 docs/changelog/133365.yaml diff --git a/docs/changelog/133365.yaml b/docs/changelog/133365.yaml new file mode 100644 index 0000000000000..60c193c67b34e --- /dev/null +++ b/docs/changelog/133365.yaml @@ -0,0 +1,5 @@ +pr: 133365 +summary: Skip iterating DISI when reading metric values +area: Codec +type: enhancement +issues: [] From d8e66d439eea2d1f5391cbdee3679d28a082e00f Mon Sep 17 00:00:00 2001 From: Nhat Nguyen Date: Thu, 28 Aug 2025 08:11:56 -0700 Subject: [PATCH 7/7] smaller builder --- .../elasticsearch/index/mapper/BlockLoader.java | 4 ---- .../elasticsearch/index/mapper/TestBlock.java | 13 ------------- .../lucene/read/SingletonDoubleBuilder.java | 13 ------------- .../lucene/read/SingletonDoubleBuilderTests.java | 16 ++-------------- 4 files changed, 2 insertions(+), 44 deletions(-) diff --git a/server/src/main/java/org/elasticsearch/index/mapper/BlockLoader.java b/server/src/main/java/org/elasticsearch/index/mapper/BlockLoader.java index 5c8eb75ea5048..19056bf837a2e 100644 --- a/server/src/main/java/org/elasticsearch/index/mapper/BlockLoader.java +++ b/server/src/main/java/org/elasticsearch/index/mapper/BlockLoader.java @@ -567,10 +567,6 @@ interface SingletonLongBuilder extends Builder { * Specialized builder for collecting dense arrays of double values. */ interface SingletonDoubleBuilder extends Builder { - SingletonDoubleBuilder appendDouble(double value); - - SingletonDoubleBuilder appendDoubles(double[] values, int from, int length); - SingletonDoubleBuilder appendLongs(BlockDocValuesReader.ToDouble toDouble, long[] values, int from, int length); } diff --git a/test/framework/src/main/java/org/elasticsearch/index/mapper/TestBlock.java b/test/framework/src/main/java/org/elasticsearch/index/mapper/TestBlock.java index be34117c5f97d..53af8f04fe46b 100644 --- a/test/framework/src/main/java/org/elasticsearch/index/mapper/TestBlock.java +++ b/test/framework/src/main/java/org/elasticsearch/index/mapper/TestBlock.java @@ -263,19 +263,6 @@ public BlockLoader.Block build() { return new TestBlock(Arrays.stream(values).boxed().collect(Collectors.toUnmodifiableList())); } - @Override - public BlockLoader.SingletonDoubleBuilder appendDoubles(double[] newValues, int from, int length) { - System.arraycopy(newValues, from, values, count, length); - count += length; - return this; - } - - @Override - public BlockLoader.SingletonDoubleBuilder appendDouble(double value) { - values[count++] = value; - return this; - } - @Override public BlockLoader.SingletonDoubleBuilder appendLongs( BlockDocValuesReader.ToDouble toDouble, diff --git a/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/lucene/read/SingletonDoubleBuilder.java b/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/lucene/read/SingletonDoubleBuilder.java index ae34064faf2fc..204805ae87e51 100644 --- a/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/lucene/read/SingletonDoubleBuilder.java +++ b/x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/lucene/read/SingletonDoubleBuilder.java @@ -72,19 +72,6 @@ public Block build() { return blockFactory.newDoubleArrayVector(values, count).asBlock(); } - @Override - public BlockLoader.SingletonDoubleBuilder appendDouble(double value) { - values[count++] = value; - return this; - } - - @Override - public BlockLoader.SingletonDoubleBuilder appendDoubles(double[] values, int from, int length) { - System.arraycopy(values, from, this.values, count, length); - count += length; - return this; - } - @Override public BlockLoader.SingletonDoubleBuilder appendLongs(BlockDocValuesReader.ToDouble toDouble, long[] longValues, int from, int length) { for (int i = 0; i < length; i++) { diff --git a/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/lucene/read/SingletonDoubleBuilderTests.java b/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/lucene/read/SingletonDoubleBuilderTests.java index 393542cd46d70..77fd7ae8b3391 100644 --- a/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/lucene/read/SingletonDoubleBuilderTests.java +++ b/x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/lucene/read/SingletonDoubleBuilderTests.java @@ -69,14 +69,7 @@ private void testRead(BlockFactory factory) throws IOException { try (var builder = new SingletonDoubleBuilder(ctx.reader().numDocs(), factory)) { for (int i = 0; i < ctx.reader().maxDoc(); i++) { assertThat(docValues.advanceExact(i), equalTo(true)); - double value = Double.longBitsToDouble(docValues.longValue()); - if (randomBoolean()) { - builder.appendDoubles(new double[] { value }, 0, 1); - } else if (randomBoolean()) { - builder.appendLongs(Double::longBitsToDouble, new long[] { docValues.longValue() }, 0, 1); - } else { - builder.appendDouble(value); - } + builder.appendLongs(Double::longBitsToDouble, new long[] { docValues.longValue() }, 0, 1); } try (var build = (DoubleVector) builder.build().asVector()) { for (int i = 0; i < build.getPositionCount(); i++) { @@ -115,12 +108,7 @@ public void testMoreValues() throws IOException { try (var builder = new SingletonDoubleBuilder(count - offset, blockFactory())) { for (int i = offset; i < leafReader.maxDoc(); i++) { assertThat(docValues.advanceExact(i), equalTo(true)); - double value = Double.longBitsToDouble(docValues.longValue()); - if (randomBoolean()) { - builder.appendDoubles(new double[] { value }, 0, 1); - } else { - builder.appendDouble(value); - } + builder.appendLongs(Double::longBitsToDouble, new long[] { docValues.longValue() }, 0, 1); } try (var build = (DoubleVector) builder.build().asVector()) { assertThat(build.getPositionCount(), equalTo(count - offset));