Skip to content

Commit 3673249

Browse files
committed
ESQL: Check for errors while loading blocks
Runs a sanity check after loading a block of values. Previously we were doing a quick check if assertions were enabled. Now we do two quick checks all the time. Better - we attach information about how a block was loaded when there's a problem. Relates to #128959
1 parent f18f4ee commit 3673249

File tree

2 files changed

+15
-8
lines changed

2 files changed

+15
-8
lines changed

test/framework/src/main/java/org/elasticsearch/index/mapper/MapperServiceTestCase.java

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -103,7 +103,7 @@
103103
import static org.mockito.Mockito.mock;
104104

105105
/**
106-
* Test case that lets you easilly build {@link MapperService} based on some
106+
* Test case that lets you easily build {@link MapperService} based on some
107107
* mapping. Useful when you don't need to spin up an entire index but do
108108
* need most of the trapping of the mapping.
109109
*/

x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/lucene/ValuesSourceReaderOperator.java

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -27,7 +27,6 @@
2727
import org.elasticsearch.compute.operator.AbstractPageMappingOperator;
2828
import org.elasticsearch.compute.operator.DriverContext;
2929
import org.elasticsearch.compute.operator.Operator;
30-
import org.elasticsearch.core.Assertions;
3130
import org.elasticsearch.core.Releasable;
3231
import org.elasticsearch.core.Releasables;
3332
import org.elasticsearch.index.fieldvisitor.StoredFieldLoader;
@@ -161,12 +160,6 @@ public int get(int i) {
161160
many.run();
162161
}
163162
}
164-
if (Assertions.ENABLED) {
165-
for (int f = 0; f < fields.length; f++) {
166-
assert blocks[f].elementType() == ElementType.NULL || blocks[f].elementType() == fields[f].info.type
167-
: blocks[f].elementType() + " NOT IN (NULL, " + fields[f].info.type + ")";
168-
}
169-
}
170163
success = true;
171164
for (Block b : blocks) {
172165
valuesLoaded += b.getTotalValueCount();
@@ -233,6 +226,7 @@ private void loadFromSingleLeaf(Block[] blocks, int shard, int segment, BlockLoa
233226
BlockLoader.ColumnAtATimeReader columnAtATime = field.columnAtATime(ctx);
234227
if (columnAtATime != null) {
235228
blocks[f] = (Block) columnAtATime.read(loaderBlockFactory, docs);
229+
sanityCheckBlock(columnAtATime, docs.count(), blocks[f], f);
236230
} else {
237231
rowStrideReaders.add(
238232
new RowStrideReaderWork(
@@ -282,6 +276,7 @@ private void loadFromSingleLeaf(Block[] blocks, int shard, int segment, BlockLoa
282276
}
283277
for (RowStrideReaderWork work : rowStrideReaders) {
284278
blocks[work.offset] = work.build();
279+
sanityCheckBlock(work.reader, docs.count(), blocks[work.offset], work.offset);
285280
}
286281
} finally {
287282
Releasables.close(rowStrideReaders);
@@ -385,6 +380,7 @@ void run() throws IOException {
385380
try (Block targetBlock = fieldTypeBuilders[f].build()) {
386381
target[f] = targetBlock.filter(backwards);
387382
}
383+
sanityCheckBlock(rowStride[f], docs.getPositionCount(), target[f], f);
388384
}
389385
}
390386

@@ -561,6 +557,17 @@ protected Status status(long processNanos, int pagesProcessed, long rowsReceived
561557
return new Status(new TreeMap<>(readersBuilt), processNanos, pagesProcessed, rowsReceived, rowsEmitted, valuesLoaded);
562558
}
563559

560+
private void sanityCheckBlock(Object loader, int expectedPositions, Block block, int f) {
561+
if (block.getPositionCount() != expectedPositions) {
562+
throw new IllegalStateException(loader + ": " + block + " didn't have [" + expectedPositions + "] positions");
563+
}
564+
if (block.elementType() != ElementType.NULL && block.elementType() != fields[f].info.type) {
565+
throw new IllegalStateException(
566+
loader + ": " + block + "'s element_type [" + block.elementType() + "] NOT IN (NULL, " + fields[f].info.type + ")"
567+
);
568+
}
569+
}
570+
564571
public static class Status extends AbstractPageMappingOperator.Status {
565572
public static final NamedWriteableRegistry.Entry ENTRY = new NamedWriteableRegistry.Entry(
566573
Operator.Status.class,

0 commit comments

Comments
 (0)