Skip to content

Commit aee4e35

Browse files
mjmbischoffmjmbischoff
committed
Refactor MvContains null handling by reusing existing IsNullEvaluatorFactory and adding global constant boolean evaluators for improved reusability.
1 parent 2aa3eec commit aee4e35

File tree

3 files changed

+71
-75
lines changed

3 files changed

+71
-75
lines changed

‎x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/operator/EvalOperator.java‎

Lines changed: 62 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -187,4 +187,66 @@ public String toString() {
187187
}
188188
};
189189
private static final String CONSTANT_NULL_NAME = "ConstantNull";
190+
191+
public static final ExpressionEvaluator.Factory CONSTANT_TRUE_FACTORY = new ExpressionEvaluator.Factory() {
192+
@Override
193+
public ExpressionEvaluator get(DriverContext driverContext) {
194+
return new ExpressionEvaluator() {
195+
@Override
196+
public Block eval(Page page) {
197+
return driverContext.blockFactory().newConstantBooleanBlockWith(true, page.getPositionCount());
198+
}
199+
200+
@Override
201+
public void close() {}
202+
203+
@Override
204+
public String toString() {
205+
return CONSTANT_TRUE_NAME;
206+
}
207+
208+
@Override
209+
public long baseRamBytesUsed() {
210+
return 0;
211+
}
212+
};
213+
}
214+
215+
@Override
216+
public String toString() {
217+
return CONSTANT_TRUE_NAME;
218+
}
219+
};
220+
private static final String CONSTANT_TRUE_NAME = "ConstantTrue";
221+
222+
public static final ExpressionEvaluator.Factory CONSTANT_FALSE_FACTORY = new ExpressionEvaluator.Factory() {
223+
@Override
224+
public ExpressionEvaluator get(DriverContext driverContext) {
225+
return new ExpressionEvaluator() {
226+
@Override
227+
public Block eval(Page page) {
228+
return driverContext.blockFactory().newConstantBooleanBlockWith(false, page.getPositionCount());
229+
}
230+
231+
@Override
232+
public void close() {}
233+
234+
@Override
235+
public String toString() {
236+
return CONSTANT_FALSE_NAME;
237+
}
238+
239+
@Override
240+
public long baseRamBytesUsed() {
241+
return 0;
242+
}
243+
};
244+
}
245+
246+
@Override
247+
public String toString() {
248+
return CONSTANT_FALSE_NAME;
249+
}
250+
};
251+
private static final String CONSTANT_FALSE_NAME = "ConstantFalse";
190252
}

‎x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/function/scalar/multivalue/MvContains.java‎

Lines changed: 3 additions & 70 deletions
Original file line numberDiff line numberDiff line change
@@ -37,6 +37,7 @@
3737
import org.elasticsearch.xpack.esql.expression.function.FunctionAppliesToLifecycle;
3838
import org.elasticsearch.xpack.esql.expression.function.FunctionInfo;
3939
import org.elasticsearch.xpack.esql.expression.function.Param;
40+
import org.elasticsearch.xpack.esql.expression.predicate.nulls.IsNull;
4041
import org.elasticsearch.xpack.esql.planner.PlannerUtils;
4142

4243
import java.io.IOException;
@@ -181,7 +182,7 @@ public ExpressionEvaluator.Factory toEvaluator(ToEvaluator toEvaluator) {
181182
var subsetType = PlannerUtils.toElementType(right().dataType());
182183

183184
if (subsetType == ElementType.NULL) {
184-
return new ConstantBooleanTrueEvaluator();
185+
return EvalOperator.CONSTANT_TRUE_FACTORY;
185186
}
186187

187188
if (supersetType != ElementType.NULL && supersetType != subsetType) {
@@ -200,7 +201,7 @@ public ExpressionEvaluator.Factory toEvaluator(ToEvaluator toEvaluator) {
200201
case DOUBLE -> new MvContainsDoubleEvaluator.Factory(source(), toEvaluator.apply(left()), toEvaluator.apply(right()));
201202
case INT -> new MvContainsIntEvaluator.Factory(source(), toEvaluator.apply(left()), toEvaluator.apply(right()));
202203
case LONG -> new MvContainsLongEvaluator.Factory(source(), toEvaluator.apply(left()), toEvaluator.apply(right()));
203-
case NULL -> new MvContainsNullSupersetEvaluator.Factory(source(), toEvaluator.apply(right()));
204+
case NULL -> new IsNull.IsNullEvaluatorFactory(toEvaluator.apply(right()));
204205
default -> throw EsqlIllegalArgumentException.illegalDataType(dataType());
205206
};
206207
}
@@ -351,74 +352,6 @@ public String toString() {
351352
}
352353
}
353354

354-
/**
355-
* Evaluator for when the lhs is null
356-
* Like the ones below should be able to autogenerate as well when the generator is more flexible
357-
*/
358-
public static class MvContainsNullSupersetEvaluator implements EvalOperator.ExpressionEvaluator {
359-
private static final long BASE_RAM_BYTES_USED = RamUsageEstimator.shallowSizeOfInstance(MvContainsNullSupersetEvaluator.class);
360-
private final EvalOperator.ExpressionEvaluator subsetFieldEvaluator;
361-
private final DriverContext driverContext;
362-
363-
public MvContainsNullSupersetEvaluator(EvalOperator.ExpressionEvaluator subsetFieldEvaluator, DriverContext driverContext) {
364-
this.subsetFieldEvaluator = subsetFieldEvaluator;
365-
this.driverContext = driverContext;
366-
}
367-
368-
@Override
369-
public Block eval(Page page) {
370-
try (Block block = subsetFieldEvaluator.eval(page)) {
371-
return eval(page.getPositionCount(), block);
372-
}
373-
}
374-
375-
public BooleanBlock eval(int positionCount, Block subsetBlock) {
376-
try (BooleanBlock.Builder result = driverContext.blockFactory().newBooleanBlockBuilder(positionCount)) {
377-
for (int p = 0; p < positionCount; p++) {
378-
MvContains.process(result, p, subsetBlock);
379-
}
380-
return result.build();
381-
}
382-
}
383-
384-
@Override
385-
public long baseRamBytesUsed() {
386-
long baseRamBytesUsed = BASE_RAM_BYTES_USED;
387-
baseRamBytesUsed += subsetFieldEvaluator.baseRamBytesUsed();
388-
return baseRamBytesUsed;
389-
}
390-
391-
@Override
392-
public void close() {
393-
Releasables.closeExpectNoException(subsetFieldEvaluator);
394-
}
395-
396-
@Override
397-
public String toString() {
398-
return "MvContainsNullSupersetEvaluator[" + "subsetField=" + subsetFieldEvaluator + "]";
399-
}
400-
401-
public static class Factory implements EvalOperator.ExpressionEvaluator.Factory {
402-
private final Source source;
403-
private final EvalOperator.ExpressionEvaluator.Factory subsetFieldEvaluator;
404-
405-
public Factory(Source source, EvalOperator.ExpressionEvaluator.Factory subsetField) {
406-
this.source = source;
407-
this.subsetFieldEvaluator = subsetField;
408-
}
409-
410-
@Override
411-
public MvContainsNullSupersetEvaluator get(DriverContext context) {
412-
return new MvContainsNullSupersetEvaluator(subsetFieldEvaluator.get(context), context);
413-
}
414-
415-
@Override
416-
public String toString() {
417-
return "MvContainsNullSupersetEvaluator[" + "subsetField=" + subsetFieldEvaluator + "]";
418-
}
419-
}
420-
}
421-
422355
/**
423356
* Currently {@code EvaluatorImplementer} generates:
424357
* if (allBlocksAreNulls) {

‎x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/expression/predicate/nulls/IsNull.java‎

Lines changed: 6 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313
import org.elasticsearch.compute.data.Page;
1414
import org.elasticsearch.compute.operator.DriverContext;
1515
import org.elasticsearch.compute.operator.EvalOperator;
16+
import org.elasticsearch.compute.operator.EvalOperator.ExpressionEvaluator;
1617
import org.elasticsearch.core.Releasables;
1718
import org.elasticsearch.xpack.esql.capabilities.TranslationAware;
1819
import org.elasticsearch.xpack.esql.core.expression.Expression;
@@ -103,7 +104,7 @@ public Object fold(FoldContext ctx) {
103104
}
104105

105106
@Override
106-
public EvalOperator.ExpressionEvaluator.Factory toEvaluator(ToEvaluator toEvaluator) {
107+
public ExpressionEvaluator.Factory toEvaluator(ToEvaluator toEvaluator) {
107108
return new IsNullEvaluatorFactory(toEvaluator.apply(field()));
108109
}
109110

@@ -138,9 +139,9 @@ public Query asQuery(LucenePushdownPredicates pushdownPredicates, TranslatorHand
138139
return new NotQuery(source(), new ExistsQuery(source(), handler.nameOf(field())));
139140
}
140141

141-
record IsNullEvaluatorFactory(EvalOperator.ExpressionEvaluator.Factory field) implements EvalOperator.ExpressionEvaluator.Factory {
142+
public record IsNullEvaluatorFactory(ExpressionEvaluator.Factory field) implements ExpressionEvaluator.Factory {
142143
@Override
143-
public EvalOperator.ExpressionEvaluator get(DriverContext context) {
144+
public ExpressionEvaluator get(DriverContext context) {
144145
return new IsNullEvaluator(context, field.get(context));
145146
}
146147

@@ -150,9 +151,9 @@ public String toString() {
150151
}
151152
}
152153

153-
record IsNullEvaluator(DriverContext driverContext, EvalOperator.ExpressionEvaluator field)
154+
record IsNullEvaluator(DriverContext driverContext, ExpressionEvaluator field)
154155
implements
155-
EvalOperator.ExpressionEvaluator {
156+
ExpressionEvaluator {
156157
private static final long BASE_RAM_BYTES_USED = RamUsageEstimator.shallowSizeOfInstance(IsNullEvaluator.class);
157158

158159
@Override

0 commit comments

Comments
 (0)