Skip to content

Commit 97fd2ac

Browse files
idegtiarenkoidegtiarenko
authored
Revert "Reuse prod code and reduce EsqlSession public surface" (#132843)
1 parent 0058284 commit 97fd2ac

File tree

3 files changed

+33
-109
lines changed

3 files changed

+33
-109
lines changed

‎muted-tests.yml

Lines changed: 0 additions & 75 deletions
Original file line numberDiff line numberDiff line change
@@ -510,93 +510,18 @@ tests:
510510
- class: org.elasticsearch.gradle.internal.transport.TransportVersionManagementPluginFuncTest
511511
method: cannot change committed ids to a branch
512512
issue: https://github.com/elastic/elasticsearch/issues/132790
513-
- class: org.elasticsearch.xpack.esql.CsvTests
514-
method: test {csv-spec:spatial.ConvertFromStringParseError}
515-
issue: https://github.com/elastic/elasticsearch/issues/132805
516-
- class: org.elasticsearch.xpack.esql.CsvTests
517-
method: test {csv-spec:floats.ScalbWithHugeScaleFactor}
518-
issue: https://github.com/elastic/elasticsearch/issues/132806
519513
- class: org.elasticsearch.reservedstate.service.FileSettingsServiceIT
520514
method: testSettingsAppliedOnStart
521515
issue: https://github.com/elastic/elasticsearch/issues/131210
522516
- class: org.elasticsearch.index.mapper.vectors.SparseVectorFieldMapperTests
523517
method: testPruningScenarios
524518
issue: https://github.com/elastic/elasticsearch/issues/132810
525-
- class: org.elasticsearch.xpack.esql.CsvTests
526-
method: test {csv-spec:spatial_shapes.ConvertFromStringParseError}
527-
issue: https://github.com/elastic/elasticsearch/issues/132812
528-
- class: org.elasticsearch.xpack.esql.CsvTests
529-
method: test {csv-spec:string.MvStringNotEqualsLong}
530-
issue: https://github.com/elastic/elasticsearch/issues/132813
531-
- class: org.elasticsearch.xpack.esql.CsvTests
532-
method: test {csv-spec:spatial.ConvertCartesianFromStringParseError}
533-
issue: https://github.com/elastic/elasticsearch/issues/132814
534-
- class: org.elasticsearch.xpack.esql.CsvTests
535-
method: test {csv-spec:spatial_shapes.ConvertCartesianShapeFromStringParseError}
536-
issue: https://github.com/elastic/elasticsearch/issues/132815
537-
- class: org.elasticsearch.xpack.esql.CsvTests
538-
method: test {csv-spec:string.RepeatNegative}
539-
issue: https://github.com/elastic/elasticsearch/issues/132818
540-
- class: org.elasticsearch.xpack.esql.CsvTests
541-
method: test {csv-spec:stats.WeightedAvgWeightMvWarning}
542-
issue: https://github.com/elastic/elasticsearch/issues/132821
543-
- class: org.elasticsearch.xpack.esql.CsvTests
544-
method: test {csv-spec:ip.PushDownIPWithIn}
545-
issue: https://github.com/elastic/elasticsearch/issues/132822
546-
- class: org.elasticsearch.xpack.esql.CsvTests
547-
method: test {csv-spec:floats.ScalbWithHugeconstantFirstArgument}
548-
issue: https://github.com/elastic/elasticsearch/issues/132823
549-
- class: org.elasticsearch.xpack.esql.CsvTests
550-
method: test {csv-spec:boolean.MvSliceWarnings}
551-
issue: https://github.com/elastic/elasticsearch/issues/132824
552-
- class: org.elasticsearch.xpack.esql.CsvTests
553-
method: test {csv-spec:change_point.Values null column}
554-
issue: https://github.com/elastic/elasticsearch/issues/132825
555-
- class: org.elasticsearch.xpack.esql.CsvTests
556-
method: test {csv-spec:string.MvStringEqualsLongString}
557-
issue: https://github.com/elastic/elasticsearch/issues/132826
558-
- class: org.elasticsearch.xpack.esql.CsvTests
559-
method: test {csv-spec:stats.WeightedAvgFieldMvWarning}
560-
issue: https://github.com/elastic/elasticsearch/issues/132827
561519
- class: org.elasticsearch.test.rest.yaml.CcsCommonYamlTestSuiteIT
562520
method: test {p0=search/160_exists_query/Test exists query on mapped date field with no doc values}
563521
issue: https://github.com/elastic/elasticsearch/issues/132828
564522
- class: org.elasticsearch.test.rest.yaml.RcsCcsCommonYamlTestSuiteIT
565523
method: test {p0=search/160_exists_query/Test exists query on keyword field in empty index}
566524
issue: https://github.com/elastic/elasticsearch/issues/132829
567-
- class: org.elasticsearch.xpack.esql.CsvTests
568-
method: test {csv-spec:ip.CidrMatchFieldArg}
569-
issue: https://github.com/elastic/elasticsearch/issues/132834
570-
- class: org.elasticsearch.xpack.esql.CsvTests
571-
method: test {csv-spec:change_point.Null values}
572-
issue: https://github.com/elastic/elasticsearch/issues/132835
573-
- class: org.elasticsearch.xpack.esql.CsvTests
574-
method: test {csv-spec:ip.Conditional}
575-
issue: https://github.com/elastic/elasticsearch/issues/132836
576-
- class: org.elasticsearch.xpack.esql.CsvTests
577-
method: test {csv-spec:ip.CdirMatchOrsIPs}
578-
issue: https://github.com/elastic/elasticsearch/issues/132837
579-
- class: org.elasticsearch.xpack.esql.CsvTests
580-
method: test {csv-spec:string.SpaceNegative}
581-
issue: https://github.com/elastic/elasticsearch/issues/132838
582-
- class: org.elasticsearch.xpack.esql.CsvTests
583-
method: test {csv-spec:string.MvStringNotEqualsFound}
584-
issue: https://github.com/elastic/elasticsearch/issues/132839
585-
- class: org.elasticsearch.xpack.esql.CsvTests
586-
method: test {csv-spec:string.ConcatOfText}
587-
issue: https://github.com/elastic/elasticsearch/issues/132840
588-
- class: org.elasticsearch.xpack.esql.CsvTests
589-
method: test {csv-spec:floats.EqualToOrEqualToMultivalue}
590-
issue: https://github.com/elastic/elasticsearch/issues/132841
591-
- class: org.elasticsearch.xpack.esql.CsvTests
592-
method: test {csv-spec:mv_percentile.FromIndexPercentile}
593-
issue: https://github.com/elastic/elasticsearch/issues/132846
594-
- class: org.elasticsearch.xpack.esql.CsvTests
595-
method: test {csv-spec:ip.CidrMatchFunctionArg}
596-
issue: https://github.com/elastic/elasticsearch/issues/132850
597-
- class: org.elasticsearch.xpack.esql.CsvTests
598-
method: test {csv-spec:conditional.CaseOnMv}
599-
issue: https://github.com/elastic/elasticsearch/issues/132851
600525
- class: org.elasticsearch.xpack.esql.CsvTests
601526
method: test {csv-spec:floats.EqualToMultivalue}
602527
issue: https://github.com/elastic/elasticsearch/issues/132852

‎x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/session/EsqlSession.java

Lines changed: 12 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -180,31 +180,19 @@ public void execute(EsqlQueryRequest request, EsqlExecutionInfo executionInfo, P
180180
analyzedPlan(parsed, executionInfo, request.filter(), new EsqlCCSUtils.CssPartialErrorsActionListener(executionInfo, listener) {
181181
@Override
182182
public void onResponse(LogicalPlan analyzedPlan) {
183-
optimizeAndExecute(request, executionInfo, planRunner, analyzedPlan, listener);
183+
SubscribableListener.<LogicalPlan>newForked(l -> preOptimizedPlan(analyzedPlan, l))
184+
.<LogicalPlan>andThen((l, p) -> preMapper.preMapper(optimizedPlan(p), l))
185+
.<Result>andThen((l, p) -> executeOptimizedPlan(request, executionInfo, planRunner, p, l))
186+
.addListener(listener);
184187
}
185188
});
186189
}
187190

188-
// visible for testing in CsvTests
189-
public void optimizeAndExecute(
190-
EsqlQueryRequest request,
191-
EsqlExecutionInfo executionInfo,
192-
PlanRunner planRunner,
193-
LogicalPlan analyzedPlan,
194-
ActionListener<Result> listener
195-
) {
196-
SubscribableListener.<LogicalPlan>newForked(l -> logicalPlanPreOptimizer.preOptimize(analyzedPlan, l))
197-
.andThenApply(this::optimizedPlan)
198-
.<LogicalPlan>andThen((l, p) -> preMapper.preMapper(p, l))
199-
.<Result>andThen((l, p) -> executeOptimizedPlan(request, executionInfo, planRunner, p, l))
200-
.addListener(listener);
201-
}
202-
203191
/**
204192
* Execute an analyzed plan. Most code should prefer calling {@link #execute} but
205193
* this is public for testing.
206194
*/
207-
private void executeOptimizedPlan(
195+
public void executeOptimizedPlan(
208196
EsqlQueryRequest request,
209197
EsqlExecutionInfo executionInfo,
210198
PlanRunner planRunner,
@@ -804,7 +792,7 @@ private PhysicalPlan logicalPlanToPhysicalPlan(LogicalPlan optimizedPlan, EsqlQu
804792
return EstimatesRowSize.estimateRowSize(0, physicalPlan);
805793
}
806794

807-
private LogicalPlan optimizedPlan(LogicalPlan logicalPlan) {
795+
public LogicalPlan optimizedPlan(LogicalPlan logicalPlan) {
808796
if (logicalPlan.preOptimized() == false) {
809797
throw new IllegalStateException("Expected pre-optimized plan");
810798
}
@@ -813,7 +801,11 @@ private LogicalPlan optimizedPlan(LogicalPlan logicalPlan) {
813801
return plan;
814802
}
815803

816-
private PhysicalPlan physicalPlan(LogicalPlan optimizedPlan) {
804+
public void preOptimizedPlan(LogicalPlan logicalPlan, ActionListener<LogicalPlan> listener) {
805+
logicalPlanPreOptimizer.preOptimize(logicalPlan, listener);
806+
}
807+
808+
public PhysicalPlan physicalPlan(LogicalPlan optimizedPlan) {
817809
if (optimizedPlan.optimized() == false) {
818810
throw new IllegalStateException("Expected optimized plan");
819811
}
@@ -823,7 +815,7 @@ private PhysicalPlan physicalPlan(LogicalPlan optimizedPlan) {
823815
return plan;
824816
}
825817

826-
private PhysicalPlan optimizedPhysicalPlan(LogicalPlan optimizedPlan) {
818+
public PhysicalPlan optimizedPhysicalPlan(LogicalPlan optimizedPlan) {
827819
var plan = physicalPlanOptimizer.optimize(physicalPlan(optimizedPlan));
828820
LOGGER.debug("Optimized physical plan:\n{}", plan);
829821
return plan;

‎x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/CsvTests.java

Lines changed: 21 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -596,21 +596,28 @@ private ActualResults executePlan(BigArrays bigArrays) throws Exception {
596596
TestPhysicalOperationProviders physicalOperationProviders = testOperationProviders(foldCtx, testDatasets);
597597

598598
PlainActionFuture<ActualResults> listener = new PlainActionFuture<>();
599-
session.optimizeAndExecute(
600-
new EsqlQueryRequest(),
601-
new EsqlExecutionInfo(randomBoolean()),
602-
planRunner(bigArrays, foldCtx, physicalOperationProviders),
603-
analyzed,
604-
listener.map(
605-
result -> new ActualResults(
606-
result.schema().stream().map(Attribute::name).toList(),
607-
result.schema().stream().map(a -> Type.asType(a.dataType().nameUpper())).toList(),
608-
result.schema().stream().map(Attribute::dataType).toList(),
609-
result.pages(),
610-
threadPool.getThreadContext().getResponseHeaders()
599+
600+
session.preOptimizedPlan(analyzed, listener.delegateFailureAndWrap((l, preOptimized) -> {
601+
session.executeOptimizedPlan(
602+
new EsqlQueryRequest(),
603+
new EsqlExecutionInfo(randomBoolean()),
604+
planRunner(bigArrays, foldCtx, physicalOperationProviders),
605+
session.optimizedPlan(preOptimized),
606+
listener.delegateFailureAndWrap(
607+
// Wrap so we can capture the warnings in the calling thread
608+
(next, result) -> next.onResponse(
609+
new ActualResults(
610+
result.schema().stream().map(Attribute::name).toList(),
611+
result.schema().stream().map(a -> Type.asType(a.dataType().nameUpper())).toList(),
612+
result.schema().stream().map(Attribute::dataType).toList(),
613+
result.pages(),
614+
threadPool.getThreadContext().getResponseHeaders()
615+
)
616+
)
611617
)
612-
)
613-
);
618+
);
619+
}));
620+
614621
return listener.get();
615622
}
616623

0 commit comments

Comments
 (0)