ES|QL: Late materialization golden tests by GalLalouche · Pull Request #141082 · elastic/elasticsearch

GalLalouche · 2026-01-21T21:10:09Z

This PR adds golden tests for node-reduce late materialization of TopN, so we can remove its snapshot hiding.

To support this, I also added support for two new stages in golden tests: node_reduce, and local_node_reduce, record the pair output of PlannerUtils.reductionPlan, and the same pair under local optimization.

alex-spies

Thanks @GalLalouche !

First pass, focused on the golden testing itself. This looks alright with minor comments, only.

Will do a second pass to have a look at the actual expectations.

x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/GoldenTestCase.java

...src/test/java/org/elasticsearch/xpack/esql/plugin/LateMaterializationPlannerGoldenTests.java

alex-spies · 2026-01-29T18:10:15Z

...ializationPlannerGoldenTests/testBasicTopNLateMaterialization/physical_optimization.expected

@@ -0,0 +1,7 @@
+ProjectExec[[hire_date{f}, salary{f}, emp_no{f}]]
+\_TopNExec[[Order[hire_date{f},ASC,LAST]],20[INTEGER],null]


For follow-up: I see that we strip the name ids from the expectations.

Can we, instead, normalize the name ids but still assert them?

In many tests, the exact name id makes a major difference, and asserting them without golden tests is super painful. Tests for the unmapped fields feature could be drastically simplified with golden tests, for instance, but we must look at the name ids.

If you agree, I'd fold this test enhancement into #138888.

I suppose this would be achievable using one of our Node transformations. I agree, let's do this as a follow up.

elasticsearchmachine · 2026-01-30T12:12:39Z

Pinging @elastic/es-analytical-engine (Team:Analytics)

alex-spies

This is super cool, thanks @GalLalouche !

I have mainly minor remarks related to how easy it is to review and understand the test expectations.

There is a non-minor point, namely the reduction_local_node_reduce.expected expectations don't seem to make sense to me; I don't think the reduce plan undergoes optimization in Production code - but if it does, we have a problem :) Could you please double check this? I left a related remark below.

...ializationPlannerGoldenTests/testBasicTopNLateMaterialization/physical_optimization.expected

x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/GoldenTestCase.java

...src/test/java/org/elasticsearch/xpack/esql/plugin/LateMaterializationPlannerGoldenTests.java

...s/LateMaterializationPlannerGoldenTests/testExpressionSortTopN/reduction_local_data.expected

alex-spies · 2026-01-30T11:55:11Z

...lannerGoldenTests/testPushedDownTopN/local_reduce_physical_optimization_data_driver.expected

@@ -0,0 +1,4 @@
+ExchangeSinkExec[[emp_no{f}, height{f}],false]
+\_ProjectExec[[_doc{f}, height{f}]]
+  \_FieldExtractExec[height{f}]<[],[]>


Yeah, this isn't ideal. We could extract only in the reduce driver. An optimization for the future, maybe.

...src/test/java/org/elasticsearch/xpack/esql/plugin/LateMaterializationPlannerGoldenTests.java

alex-spies · 2026-01-30T12:27:31Z

...izationPlannerGoldenTests/testLookupJoinOnDataNode/local_reduce_planned_data_driver.expected

+\_FragmentExec[filter=null, estimatedRowSize=0, reducer=[], fragment=[<>
+Project[[_doc{f}, emp_no{f}, languages{f}, language_code{r}, language_name{f}]]
+\_TopN[[Order[emp_no{f},ASC,LAST]],20[INTEGER],false]
+  \_Join[LEFT,[language_code{r}],[language_code{f}],null]


Oooh, there's an interesting optimization opportunity here!

We could easily perform the lookup join in the reduce driver or even the coordinator, that would also be a form of late materialization - and could reduce the work on the lookup node by 1/n, where n is the number of data drivers running on the node. If the lookup shard isn't replicated, this could affect the overall latency, and if the lookup node is somehow under pressure (e.g. from concurrent query requests that involve joins), even more so.

@julian-elastic, I think @smalyshev noticed this as well (it is related to remote enriches); should we open a tracking issue for an optimization where we try to perform TopNs before joins? (That only works when the lookup isn't remote/CCS, but still).

alex-spies

This is very nice, thanks @GalLalouche !

I have mostly minor comments; feel free to address those at your own discretion and !

A non-minor comment is that we're still performing local optimization of reduce plans in cases that don't need that. Actually, I can't think of a case where we should perform or gain anything out of local optimization of reduce plans. Since that affects already live production code, though, we can clean that up in a follow-up - in which case I'd like to ask that we create a tracking issue, please! (Even though that should be a tiny fix.) See below for more details.

alex-spies · 2026-02-11T16:44:42Z

x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/GoldenTestCase.java

+        NODE_REDUCE(new DualFileOutput("local_reduce_planned_reduce_driver", "local_reduce_planned_data_driver")),
+        /**
+         * A combination of {@link Stage#NODE_REDUCE} and {@link  Stage#LOCAL_PHYSICAL_OPTIMIZATION}: first produce the node
+         * reduce and data node plans, and then perform local physical optimization on both.


I was a bit confused, because my understanding is that node reduce plans shouldn't ever be optimized (at least for now). I learned that we do run the optimizer, even though I don't think it does anything on node reduce plans right now :)

I left a suggestion (for production code changes) below. If we plan to address that in a follow-up, maybe let's leave a TODO comment.

In terms of testing, I think only 1 plan is interesting here: the optimized physical data driver plan. The node reduce plan shouldn't change in this stage from what we got at just the NODE_REDUCE step.

Created #142392.

alex-spies · 2026-02-11T16:48:53Z

x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/GoldenTestCase.java

+                                var foo = localOptimize(reductionPlan.dataNodePlan(), conf);
+                                var bar = verifyOrWrite(foo, outputPath(dualFileOutput.dataNodeOutput()));


Var naming can probably be more specific than foo and bar :D

😅

Inlined varfiable is best variable.

...l/src/main/java/org/elasticsearch/xpack/esql/plugin/NodeReduceLocalPhysicalOptimization.java

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plugin/ClusterComputeHandler.java

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plugin/ComputeService.java

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/plugin/ReductionPlan.java

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/planner/PlannerUtils.java

alex-spies · 2026-02-11T17:43:40Z

x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/planner/PlannerUtils.java

@@ -262,6 +262,8 @@ public static PhysicalPlan localPlan(
            return EstimatesRowSize.estimateRowSize(f.estimatedRowSize(), localOptimized);
        });

+        // TODO add a test assertion for the consistency checker (see


Hmm, I wonder if this is the right place for it.

Above, we only update the fragment - and run 2 optimizer passes on it, which both perform a consistency check at the end.

We're being inconsistent with the exchange that's downstream from the fragment; but that must've been the case already before we reached here - the localPlan method must've gotten an inconsistent plan to begin with!

So, maybe it's useful to add a check at the beginning of this method? (And only run it if assertions are enabled.) A check down here isn't bad, either, but may make it look like the optimizer passes above had any chance of messing up the plan, but they don't really.

Moved to the beginning.

...src/test/java/org/elasticsearch/xpack/esql/plugin/LateMaterializationPlannerGoldenTests.java

x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/optimizer/GoldenTestCase.java

GalLalouche · 2026-02-12T16:35:03Z

Thanks for the in-depth review, @alex-spies!

This PR adds golden tests for node-reduce late materialization of TopN, so we can remove its snapshot hiding. To support this, I also added support for two new stages in golden tests: node_reduce, and local_node_reduce, record the pair output of PlannerUtils.reductionPlan, and the same pair under local optimization.

#142834) Just what it says on the tin. Follow-up to #141082 and #132757.

elastic#142834) Just what it says on the tin. Follow-up to elastic#141082 and elastic#132757.

Late materialization golden tests

5036b99

GalLalouche added >test Issues or PRs that are addressing/adding tests Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) :Analytics/ES|QL AKA ESQL v9.4.0 labels Jan 21, 2026

GalLalouche added 2 commits January 21, 2026 23:48

Remove old test file

84976f5

Fix failing test

3410841

GalLalouche requested a review from alex-spies January 22, 2026 12:43

Small changes

4066eb4

alex-spies reviewed Jan 29, 2026

View reviewed changes

Merge branch 'main' into tests/late_golden

54390d1

GalLalouche marked this pull request as ready for review January 30, 2026 12:12

CR: Add minimum random transport version

5635f49

alex-spies requested changes Jan 30, 2026

View reviewed changes

GalLalouche added 3 commits January 30, 2026 16:50

CR: Add ESQL query to the output

84a15de

Remove local node reduction tests in some cases

06d4661

Merge branch 'main' into tests/late_golden

2d87993

GalLalouche mentioned this pull request Feb 2, 2026

ESQL: Incorrect ExchangeExecs in node reduce plan #141654

Closed

GalLalouche added 2 commits February 2, 2026 20:24

More CR Fixes

f82a95a

Merge branch 'main' into tests/late_golden

c4cec77

GalLalouche requested a review from alex-spies February 2, 2026 18:30

Fix bug with synthetic names

a0eace3

alex-spies approved these changes Feb 11, 2026

View reviewed changes

alex-spies mentioned this pull request Feb 12, 2026

ESQL: remove incorrect inline stats pruning #141056

Merged

Merge branch 'main' into tests/late_golden

05202af

GalLalouche mentioned this pull request Feb 12, 2026

ES|QL: Don't preform local optimization in the node-reduce driver. #142392

Open

GalLalouche added 2 commits February 12, 2026 18:18

CR fixes

1b338cb

Merge branch 'main' into tests/late_golden

a4b3fc9

GalLalouche enabled auto-merge (squash) February 12, 2026 16:48

GalLalouche disabled auto-merge February 12, 2026 16:48

GalLalouche enabled auto-merge (squash) February 12, 2026 16:48

GalLalouche changed the title ~~Late materialization golden tests~~ ES|QL: Late materialization golden tests Feb 12, 2026

GalLalouche added 2 commits February 12, 2026 20:51

Merge branch 'main' into tests/late_golden

a05b114

Merge branch 'main' into tests/late_golden

5466e2b

GalLalouche merged commit 3041047 into elastic:main Feb 13, 2026
35 checks passed

GalLalouche mentioned this pull request Feb 23, 2026

ESQL: Remove snapshot protection from node reduce late materialization #142834

Merged

GalLalouche added a commit that referenced this pull request Feb 23, 2026

ESQL: Remove snapshot protection from node reduce late materialization (

d8aae4c

#142834) Just what it says on the tin. Follow-up to #141082 and #132757.

sidosera pushed a commit to sidosera/elasticsearch that referenced this pull request Feb 24, 2026

ESQL: Remove snapshot protection from node reduce late materialization (

bb80680

elastic#142834) Just what it says on the tin. Follow-up to elastic#141082 and elastic#132757.

		@@ -0,0 +1,7 @@
		ProjectExec[[hire_date{f}, salary{f}, emp_no{f}]]
		\_TopNExec[[Order[hire_date{f},ASC,LAST]],20[INTEGER],null]

		var foo = localOptimize(reductionPlan.dataNodePlan(), conf);
		var bar = verifyOrWrite(foo, outputPath(dualFileOutput.dataNodeOutput()));

Conversation

GalLalouche commented Jan 21, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

alex-spies left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

elasticsearchmachine commented Jan 30, 2026

Uh oh!

alex-spies left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

alex-spies left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

GalLalouche commented Feb 12, 2026

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

GalLalouche commented Jan 21, 2026 •

edited

Loading

alex-spies left a comment •

edited

Loading