ESQL: Push filters past inline stats #137572

astefan · 2025-11-04T13:08:48Z

Addresses #127497

…push_filters_past_inlinestats

elasticsearchmachine · 2025-11-12T14:50:54Z

Hi @astefan, I've created a changelog YAML for you.

elasticsearchmachine · 2025-11-12T14:50:54Z

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

…push_filters_past_inlinestats

…tefan/elasticsearch into push_filters_past_inlinestats

astefan · 2025-11-13T07:48:09Z

x-pack/plugin/esql/qa/testFixtures/src/main/resources/inlinestats.csv-spec

 c:long
 1
 ;
+


These tests are mostly mirroring the unit tests in PushDownAndCombineFiltersTests

astefan · 2025-11-13T08:07:09Z

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

        }));
    }

+    public static LogicalPlan newMainPlan(LogicalPlan optimizedPlan, InlineJoin.LogicalPlanTuple subPlans, LocalRelation resultWrapper) {


Extracted this as a method and made public to be used in the unit tests. I needed a way to use the mechanism EsqlSession uses to simulate part of the flow inline stats goes through.

astefan · 2025-11-13T08:11:57Z

...ava/org/elasticsearch/xpack/esql/optimizer/rules/logical/PushDownAndCombineFiltersTests.java

        );
        assertEquals(expectedPushedFilters, actualPushedFilters);
    }
+


These tests are convoluted (apologies for that) but they are a must. The nature of this optimization (where the results of queries do not change and the PR impact is not easily visible) requires a deep analysis of what happens so that the filters go to the right positions in the logical plan.

If you have further ideas for what to test, please let me know. I could have came up with more complex queries, but the amount of cognitive load on each test writing is high enough that I wanted to speed up the process a bit :-).

My advice: read the method javadoc with the modified logical plan and then try to match what you see there with the actual Java code, otherwise you'd spend a lot of minutes (tens of) for the test code alone.

Should we add these into a new file?
PushDownAndCombineFiltersInlineJoinTests.java or something.

bpintea

First pass. Looks good, only left cosmetics remarks so far.

bpintea · 2025-11-13T14:19:08Z

...ain/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/PushDownAndCombineFilters.java

        return new ScopedFilter(rest, leftFilters, rightFilters);
    }

+    // split the filter condition in 2 parts:


Suggested change

// split the filter condition in 2 parts:

// split the filter condition in 3 parts:

:)

bpintea · 2025-11-13T14:20:11Z

...ain/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/PushDownAndCombineFilters.java

+        List<Expression> leftFilters = new ArrayList<>(filters);
+
+        AttributeSet leftOutput = ij.left().outputSet();
+        AttributeSet rightOutput = AttributeSet.of(ij.config().rightFields());


Suggested change

AttributeSet rightOutput = AttributeSet.of(ij.config().rightFields());

AttributeSet rightJoinSet = AttributeSet.of(ij.config().rightFields());

As it's not really the output of the right (i.e. aggs + groups), but just the groups.

bpintea · 2025-11-13T14:20:24Z

...ain/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/PushDownAndCombineFilters.java

+        List<Expression> bothSides = new ArrayList<>();
+        List<Expression> leftFilters = new ArrayList<>(filters);
+
+        AttributeSet leftOutput = ij.left().outputSet();


Suggested change

AttributeSet leftOutput = ij.left().outputSet();

AttributeSet leftOutputSet = ij.left().outputSet();

Just to match the below.

bpintea · 2025-11-13T14:32:49Z

...ain/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/PushDownAndCombineFilters.java

+    // 1. filters that can be applied only to the right
+    // 2. filters that can be applied to both sides
+    // 3. filters that can be applied only to the left


I'd replace the "can be applied" with "reference" and then at the end "attributes": "1. filters that reference only to the RHS attributes" (or "..the right attributes"), since we're not applying those as such.

bpintea · 2025-11-13T14:47:43Z

...ava/org/elasticsearch/xpack/esql/optimizer/rules/logical/PushDownAndCombineFiltersTests.java

        );
        assertEquals(expectedPushedFilters, actualPushedFilters);
    }
+


Should we add these into a new file?
PushDownAndCombineFiltersInlineJoinTests.java or something.

bpintea · 2025-11-13T14:55:32Z

...ain/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/PushDownAndCombineFilters.java

                join = (Join) join.replaceLeft(left);
                // we completely applied the left filters, so we can remove them from the scoped filters
-                scoped = new ScopedFilter(scoped.commonFilters(), List.of(), scoped.rightFilters);
+                scoped = new ScopedFilter(commonFilters, List.of(), scoped.rightFilters);


Suggested change

scoped = new ScopedFilter(commonFilters, List.of(), scoped.rightFilters);

scoped = new ScopedFilter(commonFilters, List.of(), pushableToRightSide);

Here and below: it's hard to track which filters come from where. The suggestion above is one way to go.

Another would be to rewrite scoped above instead of extracting the three variables (or return them "ready to be applied" from scopeInlineStatsFilter() already.

I've tried multiple approaches here, and none of them is ideal. If I do it like you say (and I believe I did try this approach) another decision based on the join type must be taken somewhere further below. Because the logic is common in many places for inlinejoin and other types of join, I really wanted to not duplicate the logic. Initially I also used a single method for scoping, but it was too messy. Imho, the code I came up with is the least intrusive one and somewhat clean.

Likely, I didn't find the best approach; please, if you have the time, try to make it more easily understandable by refactoring the way you think best. From my point of view (and ease of grasping the logic), what is in this PR is the best version.

I have refactored this after I looked into pushing down filters on the right-hand side as well. Not possible yet #138257

bpintea · 2025-11-13T14:58:49Z

...ain/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/PushDownAndCombineFilters.java

+            var pushableToLeftSide = join instanceof InlineJoin ? scoped.commonFilters() : scoped.leftFilters();
+            var pushableToRightSide = scoped.rightFilters();
+            var commonFilters = join instanceof InlineJoin ? scoped.leftFilters() : scoped.commonFilters();


[Related to the previous comment above]

Suggested change

var pushableToLeftSide = join instanceof InlineJoin ? scoped.commonFilters() : scoped.leftFilters();

var pushableToRightSide = scoped.rightFilters();

var commonFilters = join instanceof InlineJoin ? scoped.leftFilters() : scoped.commonFilters();

scoped = join instanceof InlineJoin ? new ScopedFilter(scoped.leftFilters, scoped.commonFilters, scoped.rightFilters) : scoped;

..and then keep the rest of the code below unchanged.

Alternatively, do this shuffling in scopeInlineStatsFilter() already.

I guess the naming here is a bit tricky since, as done now, the filters pushable on the LHS are considered to be the ones taken from the RHS join keys. However, these are the same as the LHS join keys (for InlineJoin).
Basically, we want to push on the LHS of the (inline) join the filters defined on the groupings (which are the join keys). Given that, I think I'd update scopeInlineStatsFilter() code to start with the bothSides var be initialised with the filters param (and not leftFilters as is now) and then extract to the leftFilters those predicates/filters that are part of the join keys.

This would then allow initialising resulting ScopedFilter instance inline with the usage at this code location where this comment is left.

bpintea · 2025-11-13T15:07:12Z

...ava/org/elasticsearch/xpack/esql/optimizer/rules/logical/PushDownAndCombineFiltersTests.java

        assertEquals(expectedPushedFilters, actualPushedFilters);
    }
+
+    /**


Suggested change

/**

/*

I guess this might be done as a Javadoc (which, by the book, isn't) for consistency. The trouble is that this is the default rendering, which is difficult to read. I guess it can stay (for consistency), but I'll take on this crusade one day :).

bpintea · 2025-11-13T15:20:32Z

...ava/org/elasticsearch/xpack/esql/optimizer/rules/logical/PushDownAndCombineFiltersTests.java

Nice set of tests.
Could we also add some with filters on the aggs and maybe on the groups and see how that mixes?

FROM employees | INLINE STATS avgByL = AVG(salary) BY languages | INLINE STATS avgByG = AVG(salary) BY gender | WHERE languages > 2 AND gender IS NOT NULL AND avgByL > .... AND avgByG < ... | KEEP avg*, languages, gender, emp_no

FROM employees | INLINE STATS avg = AVG(salary) WHERE ... BY languages | WHERE languages > 2 | KEEP avg, languages, salary, emp_no

with possibly some combination of the filter(s) in the INLINE STATS WHERE clause (on the group or other field) and the filter(s) in the WHERE command.

Added one unit test and two csv tests as the AggregateFunction filters handling is not done on the logical plan side of things from what I could tell.

bpintea · 2025-11-13T15:22:35Z

...ain/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/PushDownAndCombineFilters.java

+            // - filters scoped to the right
+            // - filter that requires both sides to be evaluated
+            ScopedFilter scoped = join instanceof InlineJoin ij
+                ? scopeInlineStatsFilter(Predicates.splitAnd(filter.condition()), ij)


Optional: Predicates.splitAnd(filter.condition() could be taken out in a var, to make the ternary expression "lighter".

bpintea

Nice!
I've only left one more comment on making the namings easier to understand.
But I believe the logic is correct. So LGTM.

bpintea · 2025-11-13T17:25:01Z

...ain/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/PushDownAndCombineFilters.java

+                if (f.references().subsetOf(leftOutput)) {
+                    bothSides.add(f);
+                } else {
+                    rightFilters.add(f);


This case should never happen, no?
There shouldn't be an attribute that is part of the join key, but not part of the left, as otherwise the join couldn't happen. Might be easier to replace this with an assertion and have ScopedFilter instance created with a Set.of()?

Actually, that's a very good point. There is an use case I didn't cover (the original issue focused on the groupings pushing down) and that is the one where the calculated values are filtered further in the query. In this case those filters could be pushed down on the right hand side of the InlineJoin and the filtering done before the actual HashJoin and, I think, it should be more performant (hashing happening on fewer rows/values).

This is yet not possible. See #138257

bpintea · 2025-11-13T17:25:25Z

...ain/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/PushDownAndCombineFilters.java

-            // 3. filter that requires both sides to be evaluated
-            ScopedFilter scoped = scopeFilter(Predicates.splitAnd(filter.condition()), left, right);
+            // Split the filter condition in 3 parts.
+            // For InlineJoin we use a scoping that allows pushing down filters either to right side only or to both sides.


Suggested change

// For InlineJoin we use a scoping that allows pushing down filters either to right side only or to both sides.

// For InlineJoin we use a scoping that allows pushing down filters either to left side only or to both sides.

bpintea · 2025-11-13T17:51:58Z

...ain/java/org/elasticsearch/xpack/esql/optimizer/rules/logical/PushDownAndCombineFilters.java

+            var pushableToLeftSide = join instanceof InlineJoin ? scoped.commonFilters() : scoped.leftFilters();
+            var pushableToRightSide = scoped.rightFilters();
+            var commonFilters = join instanceof InlineJoin ? scoped.leftFilters() : scoped.commonFilters();


I guess the naming here is a bit tricky since, as done now, the filters pushable on the LHS are considered to be the ones taken from the RHS join keys. However, these are the same as the LHS join keys (for InlineJoin).
Basically, we want to push on the LHS of the (inline) join the filters defined on the groupings (which are the join keys). Given that, I think I'd update scopeInlineStatsFilter() code to start with the bothSides var be initialised with the filters param (and not leftFilters as is now) and then extract to the leftFilters those predicates/filters that are part of the join keys.

This would then allow initialising resulting ScopedFilter instance inline with the usage at this code location where this comment is left.

…push_filters_past_inlinestats

bpintea

Thanks Andrei, looks great now!

elasticsearchmachine · 2025-11-19T08:28:12Z

💔 Backport failed

The backport operation could not be completed due to the following error:

There are no branches to backport to. Aborting.

You can use sqren/backport to manually backport by running backport --upstream elastic/elasticsearch --pr 137572

astefan added 2 commits November 4, 2025 15:07

wip

915c9f2

Merge branch 'main' of https://github.com/elastic/elasticsearch into …

5b57d5c

…push_filters_past_inlinestats

elasticsearchmachine added the v9.3.0 label Nov 4, 2025

astefan added 2 commits November 11, 2025 17:42

More tests

530e20e

Merge branch 'main' of https://github.com/elastic/elasticsearch into …

cbd91b4

…push_filters_past_inlinestats

astefan force-pushed the push_filters_past_inlinestats branch from 13d73b6 to cbd91b4 Compare November 11, 2025 16:02

astefan added 2 commits November 12, 2025 16:48

Even more tests

182ae78

Merge branch 'main' of https://github.com/elastic/elasticsearch into …

ad13d74

…push_filters_past_inlinestats

astefan added >enhancement :Analytics/ES|QL AKA ESQL labels Nov 12, 2025

astefan requested a review from bpintea November 12, 2025 14:50

astefan marked this pull request as ready for review November 12, 2025 14:50

elasticsearchmachine added the Team:Analytics Meta label for analytical engine team (ESQL/Aggs/Geo) label Nov 12, 2025

astefan and others added 4 commits November 12, 2025 16:50

Update docs/changelog/137572.yaml

924d879

doh. Add capability to csv tests

59c47c8

Merge branch 'main' of https://github.com/elastic/elasticsearch into …

544f47e

…push_filters_past_inlinestats

Merge branch 'push_filters_past_inlinestats' of https://github.com/as…

aa394c6

…tefan/elasticsearch into push_filters_past_inlinestats

astefan commented Nov 13, 2025

View reviewed changes

Remove TODO

4ff00f8

astefan commented Nov 13, 2025

View reviewed changes

bpintea reviewed Nov 13, 2025

View reviewed changes

bpintea approved these changes Nov 13, 2025

View reviewed changes

astefan added 4 commits November 18, 2025 18:43

Simplify PushDownAndCombineFilters

f8b4952

Merge branch 'main' of https://github.com/elastic/elasticsearch into …

a28f7f3

…push_filters_past_inlinestats

More tests

266273f

Merge branch 'main' of https://github.com/elastic/elasticsearch into …

36a1ab6

…push_filters_past_inlinestats

astefan requested a review from bpintea November 18, 2025 19:23

astefan added the auto-backport Automatically create backport pull requests when merged label Nov 18, 2025

bpintea approved these changes Nov 19, 2025

View reviewed changes

astefan merged commit 45ebb58 into elastic:main Nov 19, 2025
34 checks passed

astefan deleted the push_filters_past_inlinestats branch November 19, 2025 08:27

elasticsearchmachine added the backport pending label Nov 19, 2025

astefan removed backport pending auto-backport Automatically create backport pull requests when merged labels Nov 19, 2025

astefan mentioned this pull request Nov 19, 2025

ESQL: push down filters past INLINESTATS for groupings elements only #127497

Closed

               c:long
 
               ;

	// split the filter condition in 2 parts:
	// split the filter condition in 3 parts:

	AttributeSet rightOutput = AttributeSet.of(ij.config().rightFields());
	AttributeSet rightJoinSet = AttributeSet.of(ij.config().rightFields());

	AttributeSet leftOutput = ij.left().outputSet();
	AttributeSet leftOutputSet = ij.left().outputSet();

	scoped = new ScopedFilter(commonFilters, List.of(), scoped.rightFilters);
	scoped = new ScopedFilter(commonFilters, List.of(), pushableToRightSide);

	// For InlineJoin we use a scoping that allows pushing down filters either to right side only or to both sides.
	// For InlineJoin we use a scoping that allows pushing down filters either to left side only or to both sides.

ESQL: Push filters past inline stats #137572

ESQL: Push filters past inline stats #137572

Uh oh!

Conversation

astefan commented Nov 4, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

elasticsearchmachine commented Nov 12, 2025

Uh oh!

elasticsearchmachine commented Nov 12, 2025

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

astefan Nov 13, 2025 • 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!

bpintea left a comment

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!

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!

bpintea Nov 13, 2025 • 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!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

bpintea left a comment

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!

Choose a reason for hiding this comment

Uh oh!

bpintea left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

elasticsearchmachine commented Nov 19, 2025

💔 Backport failed

Uh oh!

Reviewers

Assignees

Labels

Projects

astefan commented Nov 4, 2025 •

edited

Loading

astefan Nov 13, 2025 •

edited

Loading

bpintea Nov 13, 2025 •

edited

Loading