Skip to content

Commit

Permalink
make EvaluatorMapper.fold() throw VerificationException if there is e…
Browse files Browse the repository at this point in the history
…xception thrown from ExpressionEvaluator
  • Loading branch information
fang-xing-esql committed Nov 15, 2024
1 parent 00167e2 commit 6b1de66
Show file tree
Hide file tree
Showing 40 changed files with 1,390 additions and 657 deletions.
37 changes: 37 additions & 0 deletions docs/reference/esql/functions/kibana/definition/bit_length.json

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

Original file line number Diff line number Diff line change
Expand Up @@ -103,7 +103,6 @@ private TypeSpec type() {
builder.addField(DRIVER_CONTEXT, "driverContext", Modifier.PRIVATE, Modifier.FINAL);

builder.addField(WARNINGS, "warnings", Modifier.PRIVATE);
builder.addField(EXCEPTION, "exception", Modifier.PRIVATE);

builder.addMethod(ctor());
builder.addMethod(eval());
Expand All @@ -121,7 +120,6 @@ private TypeSpec type() {
builder.addMethod(toStringMethod());
builder.addMethod(close());
builder.addMethod(warnings());
builder.addMethod(exception());
builder.addMethod(registerException());
return builder.build();
}
Expand Down Expand Up @@ -327,8 +325,7 @@ static MethodSpec warnings() {
builder.addModifiers(Modifier.PRIVATE).returns(WARNINGS);
builder.beginControlFlow("if (warnings == null)");
builder.addStatement("""
this.warnings = Warnings.createWarnings(
driverContext.warningsMode(),
this.warnings = driverContext.createWarnings(
source.source().getLineNumber(),
source.source().getColumnNumber(),
source.text()
Expand All @@ -338,20 +335,10 @@ static MethodSpec warnings() {
return builder.build();
}

static MethodSpec exception() {
MethodSpec.Builder builder = MethodSpec.methodBuilder("exception").addAnnotation(Override.class);
builder.addModifiers(Modifier.PUBLIC).returns(EXCEPTION);
builder.addStatement("return exception");
return builder.build();
}

static MethodSpec registerException() {
MethodSpec.Builder builder = MethodSpec.methodBuilder("registerException");
builder.addModifiers(Modifier.PRIVATE).addParameter(EXCEPTION, "exception");
builder.addStatement("warnings().registerException(exception)");
builder.beginControlFlow("if (this.exception == null)");
builder.addStatement("this.exception = exception");
builder.endControlFlow();
builder.addStatement("driverContext.registerException(warnings(), exception)");
return builder.build();
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,7 +34,6 @@
import static org.elasticsearch.compute.gen.Types.BLOCK;
import static org.elasticsearch.compute.gen.Types.BYTES_REF;
import static org.elasticsearch.compute.gen.Types.DRIVER_CONTEXT;
import static org.elasticsearch.compute.gen.Types.EXCEPTION;
import static org.elasticsearch.compute.gen.Types.EXPRESSION_EVALUATOR;
import static org.elasticsearch.compute.gen.Types.EXPRESSION_EVALUATOR_FACTORY;
import static org.elasticsearch.compute.gen.Types.SOURCE;
Expand Down Expand Up @@ -137,7 +136,6 @@ private TypeSpec type() {
builder.superclass(ABSTRACT_NULLABLE_MULTIVALUE_FUNCTION_EVALUATOR);
builder.addField(SOURCE, "source", Modifier.PRIVATE, Modifier.FINAL);
builder.addField(WARNINGS, "warnings", Modifier.PRIVATE);
builder.addField(EXCEPTION, "exception", Modifier.PRIVATE);
}

builder.addMethod(ctor());
Expand All @@ -160,7 +158,6 @@ private TypeSpec type() {
builder.addType(factory());
if (warnExceptions.isEmpty() == false) {
builder.addMethod(EvaluatorImplementer.warnings());
builder.addMethod(EvaluatorImplementer.exception());
builder.addMethod(EvaluatorImplementer.registerException());
}
return builder.build();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,14 +16,18 @@
import org.elasticsearch.core.Releasable;
import org.elasticsearch.core.Releasables;

import java.util.ArrayList;
import java.util.Collections;
import java.util.IdentityHashMap;
import java.util.List;
import java.util.Objects;
import java.util.Set;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.concurrent.atomic.AtomicInteger;
import java.util.concurrent.atomic.AtomicReference;

import static org.elasticsearch.compute.operator.Warnings.MAX_ADDED_WARNINGS;

/**
* A driver-local context that is shared across operators.
*
Expand Down Expand Up @@ -60,6 +64,8 @@ public class DriverContext {

private final WarningsMode warningsMode;

private final List<Exception> warnings = new ArrayList<>(MAX_ADDED_WARNINGS);

public DriverContext(BigArrays bigArrays, BlockFactory blockFactory) {
this(bigArrays, blockFactory, WarningsMode.COLLECT);
}
Expand All @@ -76,7 +82,8 @@ public static DriverContext getLocalDriver() {
return new DriverContext(
BigArrays.NON_RECYCLING_INSTANCE,
// TODO maybe this should have a small fixed limit?
new BlockFactory(new NoopCircuitBreaker(CircuitBreaker.REQUEST), BigArrays.NON_RECYCLING_INSTANCE)
new BlockFactory(new NoopCircuitBreaker(CircuitBreaker.REQUEST), BigArrays.NON_RECYCLING_INSTANCE),
WarningsMode.LOCAL
);
}

Expand Down Expand Up @@ -188,7 +195,26 @@ public WarningsMode warningsMode() {
*/
public enum WarningsMode {
COLLECT,
IGNORE
IGNORE,
LOCAL
}

public Warnings createWarnings(int lineNumber, int columnNumber, String sourceText) {
return Warnings.createWarnings(warningsMode, lineNumber, columnNumber, sourceText);
}

public void registerException(Warnings warnings, Exception e) {
if (warningsMode == WarningsMode.COLLECT) {
warnings.registerException(e);
} else if (warningsMode == WarningsMode.LOCAL) {
if (warnings().size() < MAX_ADDED_WARNINGS) { // folding errors do not go to thread, throw a VerificationException in stead
warnings().add(e);
}
}
}

public List<Exception> warnings() {
return this.warnings;
}

private static class AsyncActions {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,10 +81,6 @@ default boolean eagerEvalSafeInLazy() {
* @return the returned Block has its own reference and the caller is responsible for releasing it.
*/
Block eval(Page page);

default Exception exception() {
return null;
}
}

public static final ExpressionEvaluator.Factory CONSTANT_NULL_FACTORY = new ExpressionEvaluator.Factory() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,15 +53,15 @@ public static Warnings createWarningsTreatedAsFalse(
return createWarnings(warningsMode, lineNumber, columnNumber, sourceText, "treating result as false");
}

private static Warnings createWarnings(
public static Warnings createWarnings(
DriverContext.WarningsMode warningsMode,
int lineNumber,
int columnNumber,
String sourceText,
String first
) {
switch (warningsMode) {
case COLLECT -> {
case COLLECT, LOCAL -> {
return new Warnings(lineNumber, columnNumber, sourceText, first);
}
case IGNORE -> {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -597,19 +597,6 @@ ROW long = TO_LONG(100), double = 99., int = 100
99.0 |0.0 |99.0
;


zeroBucketsRow#[skip:-8.13.99, reason:BUCKET renamed in 8.14]
ROW a = 1
| STATS max = max(a) BY b = BUCKET(a, 0, 0, 0)
;
warningRegex:evaluation of \[BUCKET\(a, 0, 0, 0\)\] failed, treating result as null. Only first 20 failures recorded
warningRegex:java.lang.ArithmeticException: / by zero

max:integer | b:double
1 | null
;


zeroBuckets#[skip:-8.13.99, reason:BUCKET renamed in 8.14]
FROM employees
| STATS max = max(salary) BY b = BUCKET(salary, 0, 0, 0)
Expand All @@ -621,7 +608,6 @@ max:integer | b:double
74999 | null
;


zeroBucketsDouble#[skip:-8.13.99, reason:BUCKET renamed in 8.14]
FROM employees
| STATS max = max(salary) BY b = BUCKET(salary, 0.)
Expand All @@ -633,18 +619,6 @@ max:integer | b:double
74999 | null
;

minusOneBucketsRow#[skip:-8.13.99, reason:BUCKET renamed in 8.14]
ROW a = 1
| STATS max = max(a) BY b = BUCKET(a, -1, 0, 0)
;
warningRegex:evaluation of \[BUCKET\(a, -1, 0, 0\)\] failed, treating result as null. Only first 20 failures recorded
warningRegex:java.lang.ArithmeticException: / by zero

max:integer | b:double
1 | null
;


minusOneBuckets#[skip:-8.13.99, reason:BUCKET renamed in 8.14]
FROM employees
| STATS max = max(salary) BY b = BUCKET(salary, -1, 0, 0)
Expand All @@ -656,19 +630,6 @@ max:integer | b:double
74999 | null
;


tooManyBucketsRow#[skip:-8.13.99, reason:BUCKET renamed in 8.14]
ROW a = 1
| STATS max = max(a) BY b = BUCKET(a, 100000000000, 0, 0)
;
warningRegex:evaluation of \[BUCKET\(a, 100000000000, 0, 0\)\] failed, treating result as null. Only first 20 failures recorded
warningRegex:java.lang.ArithmeticException: / by zero

max:integer | b:double
1 | null
;


tooManyBuckets#[skip:-8.13.99, reason:BUCKET renamed in 8.14]
FROM employees
| STATS max = max(salary) BY b = BUCKET(salary, 100000000000, 0, 0)
Expand All @@ -680,7 +641,6 @@ max:integer | b:double
74999 | null
;


foldableBuckets
required_capability: casting_operator
FROM employees
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -204,64 +204,6 @@ error_rate:double | hour:date
// end::docsCaseHourlyErrorRate-result[]
;


nullOnMultivaluesMathOperation
required_capability: disable_nullable_opts

ROW a = 5, b = [ 1, 2 ]| EVAL sum = a + b| LIMIT 1 | WHERE sum IS NULL;
warning:Line 1:37: evaluation of [a + b] failed, treating result as null. Only first 20 failures recorded.
warning:Line 1:37: java.lang.IllegalArgumentException: single-value function encountered multi-value

a:integer | b:integer | sum:integer
5 | [1, 2] | null
;


notNullOnMultivaluesMathOperation
required_capability: disable_nullable_opts

ROW a = 5, b = [ 1, 2 ]| EVAL sum = a + b| LIMIT 1 | WHERE sum IS NOT NULL;
warning:Line 1:37: evaluation of [a + b] failed, treating result as null. Only first 20 failures recorded.
warning:Line 1:37: java.lang.IllegalArgumentException: single-value function encountered multi-value

a:integer | b:integer | sum:integer
;


nullOnMultivaluesComparisonOperation
required_capability: disable_nullable_opts

ROW a = 5, b = [ 1, 2 ]| EVAL same = a == b| LIMIT 1 | WHERE same IS NULL;
warning:Line 1:38: evaluation of [a == b] failed, treating result as null. Only first 20 failures recorded.
warning:Line 1:38: java.lang.IllegalArgumentException: single-value function encountered multi-value


a:integer | b:integer | same:boolean
5 | [1, 2] | null
;


notNullOnMultivaluesComparisonOperation
required_capability: disable_nullable_opts

ROW a = 5, b = [ 1, 2 ]| EVAL same = a == b| LIMIT 1 | WHERE same IS NOT NULL;
warning:Line 1:38: evaluation of [a == b] failed, treating result as null. Only first 20 failures recorded.
warning:Line 1:38: java.lang.IllegalArgumentException: single-value function encountered multi-value

a:integer | b:integer | same:boolean
;


notNullOnMultivaluesComparisonOperationWithPartialMatch
required_capability: disable_nullable_opts

ROW a = 5, b = [ 5, 2 ]| EVAL same = a == b| LIMIT 1 | WHERE same IS NOT NULL;
warning:Line 1:38: evaluation of [a == b] failed, treating result as null. Only first 20 failures recorded.
warning:Line 1:38: java.lang.IllegalArgumentException: single-value function encountered multi-value

a:integer | b:integer | same:boolean
;

caseOnTheValue_NotOnTheField
required_capability: fixed_wrong_is_not_null_check_on_case

Expand Down
Loading

0 comments on commit 6b1de66

Please sign in to comment.