ESQL: Move interface ExpressionEvaluator#143742
Conversation
The `ExpressionEvaluator` interface lived inside of `EvalOperator` and that bothered me. It's used in `FilterOperator` and `StringExtractOperator` and, well, lots of places. This moves it to a top level class. All things used in 123093432143214 places should be.
|
Pinging @elastic/es-analytical-engine (Team:Analytics) |
There was a problem hiding this comment.
Pull request overview
Moves the ExpressionEvaluator interface out of EvalOperator into a top-level org.elasticsearch.compute.expression package, updating generated evaluators, operators, tests, and benchmarks to reference the new type.
Changes:
- Introduce
org.elasticsearch.compute.expression.ExpressionEvaluator(top-level) andConstantExpressionsto host constant evaluator factories. - Update compute operators, aggregations, generated ESQL evaluators, tests, and benchmarks to use the new
ExpressionEvaluatorimport/type. - Export the new expression package via
module-info.javaand update codegen type references.
Reviewed changes
Copilot reviewed 41 out of 728 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| x-pack/plugin/esql/src/main/generated-src/org/elasticsearch/xpack/esql/expression/predicate/operator/comparison/InNanosMillisEvaluator.java | Switches evaluator interface reference to top-level ExpressionEvaluator. |
| x-pack/plugin/esql/src/main/generated-src/org/elasticsearch/xpack/esql/expression/predicate/operator/comparison/InMillisNanosEvaluator.java | Switches evaluator interface reference to top-level ExpressionEvaluator. |
| x-pack/plugin/esql/src/main/generated-src/org/elasticsearch/xpack/esql/expression/predicate/operator/comparison/InLongEvaluator.java | Switches evaluator interface reference to top-level ExpressionEvaluator. |
| x-pack/plugin/esql/src/main/generated-src/org/elasticsearch/xpack/esql/expression/predicate/operator/comparison/InIntEvaluator.java | Switches evaluator interface reference to top-level ExpressionEvaluator. |
| x-pack/plugin/esql/src/main/generated-src/org/elasticsearch/xpack/esql/expression/predicate/operator/comparison/InDoubleEvaluator.java | Switches evaluator interface reference to top-level ExpressionEvaluator. |
| x-pack/plugin/esql/src/main/generated-src/org/elasticsearch/xpack/esql/expression/predicate/operator/comparison/InBytesRefEvaluator.java | Switches evaluator interface reference to top-level ExpressionEvaluator. |
| x-pack/plugin/esql/src/main/generated-src/org/elasticsearch/xpack/esql/expression/predicate/operator/comparison/InBooleanEvaluator.java | Switches evaluator interface reference to top-level ExpressionEvaluator. |
| x-pack/plugin/esql/src/main/generated-src/org/elasticsearch/xpack/esql/expression/function/scalar/nulls/CoalesceTDigestEvaluator.java | Updates evaluator references/imports to top-level ExpressionEvaluator. |
| x-pack/plugin/esql/src/main/generated-src/org/elasticsearch/xpack/esql/expression/function/scalar/nulls/CoalesceLongEvaluator.java | Updates evaluator references/imports to top-level ExpressionEvaluator. |
| x-pack/plugin/esql/src/main/generated-src/org/elasticsearch/xpack/esql/expression/function/scalar/nulls/CoalesceIntEvaluator.java | Updates evaluator references/imports to top-level ExpressionEvaluator. |
| x-pack/plugin/esql/src/main/generated-src/org/elasticsearch/xpack/esql/expression/function/scalar/nulls/CoalesceFloatEvaluator.java | Updates evaluator references/imports to top-level ExpressionEvaluator. |
| x-pack/plugin/esql/src/main/generated-src/org/elasticsearch/xpack/esql/expression/function/scalar/nulls/CoalesceExponentialHistogramEvaluator.java | Updates evaluator references/imports to top-level ExpressionEvaluator. |
| x-pack/plugin/esql/src/main/generated-src/org/elasticsearch/xpack/esql/expression/function/scalar/nulls/CoalesceDoubleEvaluator.java | Updates evaluator references/imports to top-level ExpressionEvaluator. |
| x-pack/plugin/esql/src/main/generated-src/org/elasticsearch/xpack/esql/expression/function/scalar/nulls/CoalesceBytesRefEvaluator.java | Updates evaluator references/imports to top-level ExpressionEvaluator. |
| x-pack/plugin/esql/src/main/generated-src/org/elasticsearch/xpack/esql/expression/function/scalar/nulls/CoalesceBooleanEvaluator.java | Updates evaluator references/imports to top-level ExpressionEvaluator. |
| x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/operator/exchange/BidirectionalBatchExchangeTests.java | Updates anonymous evaluator implementations to top-level ExpressionEvaluator. |
| x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/operator/exchange/BatchDriverTests.java | Updates helper evaluators/operators to top-level ExpressionEvaluator. |
| x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/operator/StringExtractOperatorTests.java | Updates test factories/constructors to use top-level ExpressionEvaluator. |
| x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/operator/FilterOperatorTests.java | Updates evaluator record/factory to top-level ExpressionEvaluator. |
| x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/operator/EvalOperatorTests.java | Updates evaluator records/factory to top-level ExpressionEvaluator. |
| x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/operator/DriverTests.java | Updates anonymous evaluator in EvalOperator usage to top-level ExpressionEvaluator. |
| x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/operator/ColumnExtractOperatorTests.java | Updates operator factory evaluator to top-level ExpressionEvaluator. |
| x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/operator/AddGarbageRowsSourceOperator.java | Updates filter factory to return top-level ExpressionEvaluator.Factory. |
| x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/aggregation/FilteredGroupingAggregatorFunctionTests.java | Updates filter evaluator factory and evaluator type to top-level ExpressionEvaluator. |
| x-pack/plugin/esql/compute/src/test/java/org/elasticsearch/compute/ConstantBooleanExpressionEvaluator.java | Updates constant evaluator test utility to implement top-level ExpressionEvaluator. |
| x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/operator/mvdedupe/MultivalueDedupe.java | Updates import to top-level ExpressionEvaluator. |
| x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/operator/StringExtractOperator.java | Updates operator field/ctor type to top-level ExpressionEvaluator. |
| x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/operator/FilterOperator.java | Updates operator field/ctor type to top-level ExpressionEvaluator. |
| x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/operator/EvalOperator.java | Removes nested ExpressionEvaluator and constant factories after moving them. |
| x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/operator/ColumnExtractOperator.java | Updates operator field type to top-level ExpressionEvaluator. |
| x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/lucene/query/LuceneQueryExpressionEvaluator.java | Updates evaluator and factory to implement top-level ExpressionEvaluator. |
| x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/expression/package-info.java | Adds package documentation for new compute.expression package. |
| x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/expression/ExpressionEvaluator.java | Adds new top-level ExpressionEvaluator interface and moved Javadoc. |
| x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/expression/ConstantExpressions.java | Adds new home for constant evaluator factories previously on EvalOperator. |
| x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/aggregation/FilteredGroupingAggregatorFunction.java | Updates filter evaluator type to top-level ExpressionEvaluator. |
| x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/aggregation/FilteredAggregatorFunctionSupplier.java | Updates filter evaluator factory/type to top-level ExpressionEvaluator. |
| x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/aggregation/FilteredAggregatorFunction.java | Updates filter evaluator type to top-level ExpressionEvaluator. |
| x-pack/plugin/esql/compute/src/main/java/module-info.java | Exports newly added org.elasticsearch.compute.expression package. |
| x-pack/plugin/esql/compute/gen/src/main/java/org/elasticsearch/compute/gen/Types.java | Updates codegen type references for ExpressionEvaluator and its Factory. |
| benchmarks/src/main/java/org/elasticsearch/benchmark/compute/operator/EvalBenchmark.java | Updates benchmark evaluator type to top-level ExpressionEvaluator. |
| benchmarks/src/main/java/org/elasticsearch/benchmark/compute/operator/AggregatorBenchmark.java | Updates filtered benchmark to construct top-level ExpressionEvaluator. |
Comments suppressed due to low confidence (1)
x-pack/plugin/esql/compute/src/main/java/org/elasticsearch/compute/expression/ExpressionEvaluator.java:1
- Correct typo/grammar: 'much allocate' should be 'must allocate'.
/*
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review. Take the survey.
| import org.apache.lucene.util.Accountable; | ||
| import org.elasticsearch.common.breaker.CircuitBreaker; | ||
| import org.elasticsearch.common.util.BigArrays; | ||
| import org.elasticsearch.compute.data.Block; | ||
| import org.elasticsearch.compute.data.Page; | ||
| import org.elasticsearch.compute.operator.BreakingBytesRefBuilder; | ||
| import org.elasticsearch.compute.operator.Driver; | ||
| import org.elasticsearch.compute.operator.DriverContext; | ||
| import org.elasticsearch.compute.operator.EvalOperator; | ||
| import org.elasticsearch.core.Releasable; |
There was a problem hiding this comment.
This file imports several types that are only referenced from Javadoc (e.g., Accountable, CircuitBreaker, BigArrays, BreakingBytesRefBuilder, Driver, EvalOperator). Unused imports are a Java compile error. Remove the unused imports and (if desired) use fully-qualified names in Javadoc {@link ...} tags instead.
There was a problem hiding this comment.
This makes that javadoc a ton easier to use. This is intentional.
...gin/esql/compute/src/main/java/org/elasticsearch/compute/expression/ExpressionEvaluator.java
Outdated
Show resolved
Hide resolved
…pute/expression/ExpressionEvaluator.java Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
The `ExpressionEvaluator` interface lived inside of `EvalOperator` and that bothered me. It's used in `FilterOperator` and `StringExtractOperator` and, well, lots of places. This moves it to a top level class. All things used in 123093432143214 places should be. Relates to elastic#100634
The
ExpressionEvaluatorinterface lived inside ofEvalOperatorand that bothered me. It's used inFilterOperatorandStringExtractOperatorand, well, lots of places. This moves it to a top level class. All things used in 123093432143214 places should be.This change is entirely mechanical and compile time. The runnable bits should be the same.