Skip to content
Merged
Show file tree
Hide file tree
Changes from 7 commits
Commits
Show all changes
19 commits
Select commit Hold shift + click to select a range
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 6 additions & 0 deletions docs/changelog/135011.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,6 @@
pr: 135011
summary: Handle right hand side of Inline Stats coming optimized with `LocalRelation`
shortcut
area: ES|QL
type: bug
issues: []
Original file line number Diff line number Diff line change
Expand Up @@ -1808,9 +1808,9 @@ from employees
10003 |Parto |null |0 |null |M |1 |4
;

prunedInlinestatsFollowedByInlinestats_GroupByOneFieldOnSecondInlinestats-Ignore
// values doesn't end up as null
prunedInlinestatsFollowedByInlinestats_GroupByOneFieldOnSecondInlinestats
required_capability: inline_stats
required_capability: inline_stats_fix_pruning_null_filter
from employees
| eval my_length = length(concat(first_name, null))
| inline stats count = count(my_length) where false,
Expand Down Expand Up @@ -1988,9 +1988,9 @@ null |0 |[Gino, Heping]
null |[0, 0, 0, 0] |[Berni, Chirstian, Amabile, Berni, Bojan, Chirstian, Anneke, Chirstian]|[Head Human Resources, Reporting Analyst, Support Engineer, Tech Lead]|10004
;

prunedInlinestatsFollowedByinlinestats-Ignore
// values doesn't end up as null
prunedInlinestatsFollowedByInlinestats
required_capability: inline_stats
required_capability: inline_stats_fix_pruning_null_filter
from employees
| eval my_length = length(concat(first_name, null))
| inline stats count = count(my_length) where false,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -1528,6 +1528,12 @@ public enum Cap {
*/
TS_COMMAND_V0(),

/**
* INLINE STATS fix incorrect prunning of null filtering
* https://github.com/elastic/elasticsearch/pull/135011
*/
INLINE_STATS_FIX_PRUNING_NULL_FILTER(INLINESTATS_V11.enabled),

;

private final boolean enabled;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,8 +24,8 @@
import org.elasticsearch.xpack.esql.plan.logical.LogicalPlan;
import org.elasticsearch.xpack.esql.plan.logical.Project;
import org.elasticsearch.xpack.esql.plan.logical.Sample;
import org.elasticsearch.xpack.esql.plan.logical.UnaryPlan;
import org.elasticsearch.xpack.esql.plan.logical.join.InlineJoin;
import org.elasticsearch.xpack.esql.plan.logical.local.EmptyLocalSupplier;
import org.elasticsearch.xpack.esql.plan.logical.local.LocalRelation;
import org.elasticsearch.xpack.esql.plan.logical.local.LocalSupplier;
import org.elasticsearch.xpack.esql.planner.PlannerUtils;
Expand All @@ -34,6 +34,8 @@
import java.util.ArrayList;
import java.util.List;

import static org.elasticsearch.xpack.esql.optimizer.rules.logical.PruneEmptyPlans.skipPlan;

/**
* Remove unused columns created in the plan, in fields inside eval or aggregations inside stats.
*/
Expand Down Expand Up @@ -193,9 +195,9 @@ private static LogicalPlan pruneColumnsInEsRelation(EsRelation esr, AttributeSet
return p;
}

private static LogicalPlan emptyLocalRelation(LogicalPlan plan) {
private static LogicalPlan emptyLocalRelation(UnaryPlan plan) {
// create an empty local relation with no attributes
return new LocalRelation(plan.source(), plan.output(), EmptyLocalSupplier.EMPTY);
return skipPlan(plan);
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Unrelated to the meat of the PR

}

private static boolean isLocalEmptyRelation(LogicalPlan plan) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -147,10 +147,20 @@ public static LogicalPlanTuple firstSubPlan(LogicalPlan optimizedPlan) {
// Collect the first inlinejoin (bottom up in the tree)
optimizedPlan.forEachUp(InlineJoin.class, ij -> {
// extract the right side of the plan and replace its source
if (subPlan.get() == null && ij.right().anyMatch(p -> p instanceof StubRelation)) {
var p = replaceStub(ij.left(), ij.right());
p.setOptimized();
subPlan.set(new LogicalPlanTuple(p, ij.right()));
if (subPlan.get() == null) {
if (ij.right().anyMatch(p -> p instanceof StubRelation)) {
var p = replaceStub(ij.left(), ij.right());
p.setOptimized();
subPlan.set(new LogicalPlanTuple(p, ij.right()));
} else if (ij.right() instanceof LocalRelation == false && ij.right().anyMatch(p -> p instanceof LocalRelation)) {
// In case the plan was optimized further and the StubRelation was replaced with a LocalRelation
// there is no need to replace the source of the right-hand side since it's not needed anymore
// The source itself is the plan that was optimized to a LocalRelation.
// This plan still needs to be executed though so that in the end is a single-node sub-tree with LocalRelation with data
var p = ij.right();
p.setOptimized();
subPlan.set(new LogicalPlanTuple(p, ij.right()));
}
}
});
return subPlan.get();
Expand Down