-
Notifications
You must be signed in to change notification settings - Fork 25.5k
ESQL: Speed up CASE for some parameters #112295
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 12 commits
2ab7fc0
394b9d7
3979049
5fd2f66
0e1c8ac
19a8247
f3abaef
a4af922
3117287
deb5845
3c45129
9b1f6c4
b37d645
e6c0863
85c757e
15ea99c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,5 @@ | ||
pr: 112295 | ||
summary: "ESQL: Speed up CASE for some parameters" | ||
area: ES|QL | ||
type: enhancement | ||
issues: [] |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -63,6 +63,17 @@ public interface ExpressionEvaluator extends Releasable { | |
/** A Factory for creating ExpressionEvaluators. */ | ||
interface Factory { | ||
ExpressionEvaluator get(DriverContext context); | ||
|
||
/** | ||
* {@code true} if it is safe and fast to evaluate this expression eagerly | ||
* in {@link ExpressionEvaluator}s that need to be lazy, like {@code CASE}. | ||
* This defaults to {@code false}, but expressions | ||
* that evaluate quickly and can not produce warnings may override this to | ||
* {@code true} to get a significant speed-up in {@code CASE}-like operations. | ||
*/ | ||
default boolean eagerEvalSafeInLazy() { | ||
return false; | ||
} | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I suppose this is something that we could derive from the Expression - but it felt simpler to put it here. And it's just a boolean at this point - though maybe it should be a cost estimate at some point. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In case of constants, the unvisited branches can be removed and the case simplified. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That works if the expression is constant, but not if the values are constant. We still have to evaluate in that case. And when we do we can do it the fast way with something like this. |
||
} | ||
|
||
/** | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -15,6 +15,7 @@ | |
import org.elasticsearch.compute.data.BooleanBlock; | ||
import org.elasticsearch.compute.data.ElementType; | ||
import org.elasticsearch.compute.data.Page; | ||
import org.elasticsearch.compute.data.ToMask; | ||
import org.elasticsearch.compute.operator.DriverContext; | ||
import org.elasticsearch.compute.operator.EvalOperator; | ||
import org.elasticsearch.compute.operator.EvalOperator.ExpressionEvaluator; | ||
|
@@ -311,25 +312,16 @@ private Expression finishPartialFold(List<Expression> newChildren) { | |
|
||
@Override | ||
public ExpressionEvaluator.Factory toEvaluator(Function<Expression, ExpressionEvaluator.Factory> toEvaluator) { | ||
ElementType resultType = PlannerUtils.toElementType(dataType()); | ||
List<ConditionEvaluatorSupplier> conditionsFactories = conditions.stream().map(c -> c.toEvaluator(toEvaluator)).toList(); | ||
ExpressionEvaluator.Factory elseValueFactory = toEvaluator.apply(elseValue); | ||
return new ExpressionEvaluator.Factory() { | ||
@Override | ||
public ExpressionEvaluator get(DriverContext context) { | ||
return new CaseEvaluator( | ||
context.blockFactory(), | ||
resultType, | ||
conditionsFactories.stream().map(x -> x.apply(context)).toList(), | ||
elseValueFactory.get(context) | ||
); | ||
} | ||
ElementType resultType = PlannerUtils.toElementType(dataType()); | ||
|
||
@Override | ||
public String toString() { | ||
return "CaseEvaluator[conditions=" + conditionsFactories + ", elseVal=" + elseValueFactory + ']'; | ||
} | ||
}; | ||
if (conditionsFactories.size() == 1 | ||
&& conditionsFactories.get(0).value.eagerEvalSafeInLazy() | ||
&& elseValueFactory.eagerEvalSafeInLazy()) { | ||
return new CaseEagerEvaluatorFactory(resultType, conditionsFactories.get(0), elseValueFactory); | ||
} | ||
return new CaseLazyEvaluatorFactory(resultType, conditionsFactories, elseValueFactory); | ||
} | ||
|
||
record ConditionEvaluatorSupplier(Source conditionSource, ExpressionEvaluator.Factory condition, ExpressionEvaluator.Factory value) | ||
|
@@ -375,9 +367,42 @@ public void close() { | |
public String toString() { | ||
return "ConditionEvaluator[condition=" + condition + ", value=" + value + ']'; | ||
} | ||
|
||
public void registerMultivalue() { | ||
conditionWarnings.registerException(new IllegalArgumentException("CASE expects a single-valued boolean")); | ||
} | ||
} | ||
|
||
private record CaseEvaluator( | ||
private record CaseLazyEvaluatorFactory( | ||
ElementType resultType, | ||
List<ConditionEvaluatorSupplier> conditionsFactories, | ||
ExpressionEvaluator.Factory elseValueFactory | ||
) implements ExpressionEvaluator.Factory { | ||
@Override | ||
public ExpressionEvaluator get(DriverContext context) { | ||
List<ConditionEvaluator> conditions = new ArrayList<>(conditionsFactories.size()); | ||
ExpressionEvaluator elseValue = null; | ||
try { | ||
for (ConditionEvaluatorSupplier cond : conditionsFactories) { | ||
conditions.add(cond.apply(context)); | ||
} | ||
elseValue = elseValueFactory.get(context); | ||
ExpressionEvaluator result = new CaseLazyEvaluator(context.blockFactory(), resultType, conditions, elseValue); | ||
conditions = null; | ||
elseValue = null; | ||
return result; | ||
} finally { | ||
Releasables.close(conditions == null ? () -> {} : Releasables.wrap(conditions), elseValue); | ||
} | ||
} | ||
|
||
@Override | ||
public String toString() { | ||
return "CaseLazyEvaluator[conditions=" + conditionsFactories + ", elseVal=" + elseValueFactory + ']'; | ||
} | ||
} | ||
|
||
private record CaseLazyEvaluator( | ||
BlockFactory blockFactory, | ||
ElementType resultType, | ||
List<ConditionEvaluator> conditions, | ||
|
@@ -409,9 +434,7 @@ public Block eval(Page page) { | |
continue; | ||
} | ||
if (b.getValueCount(0) > 1) { | ||
condition.conditionWarnings.registerException( | ||
new IllegalArgumentException("CASE expects a single-valued boolean") | ||
); | ||
condition.registerMultivalue(); | ||
continue; | ||
} | ||
if (false == b.getBoolean(b.getFirstValueIndex(0))) { | ||
|
@@ -439,7 +462,73 @@ public void close() { | |
|
||
@Override | ||
public String toString() { | ||
return "CaseEvaluator[conditions=" + conditions + ", elseVal=" + elseVal + ']'; | ||
return "CaseLazyEvaluator[conditions=" + conditions + ", elseVal=" + elseVal + ']'; | ||
} | ||
} | ||
|
||
private record CaseEagerEvaluatorFactory( | ||
ElementType resultType, | ||
ConditionEvaluatorSupplier conditionFactory, | ||
ExpressionEvaluator.Factory elseValueFactory | ||
) implements ExpressionEvaluator.Factory { | ||
@Override | ||
public ExpressionEvaluator get(DriverContext context) { | ||
ConditionEvaluator conditionEvaluator = conditionFactory.apply(context); | ||
ExpressionEvaluator elseValue = null; | ||
try { | ||
elseValue = elseValueFactory.get(context); | ||
ExpressionEvaluator result = new CaseEagerEvaluator(resultType, context.blockFactory(), conditionEvaluator, elseValue); | ||
conditionEvaluator = null; | ||
elseValue = null; | ||
return result; | ||
} finally { | ||
Releasables.close(conditionEvaluator, elseValue); | ||
} | ||
} | ||
|
||
@Override | ||
public String toString() { | ||
return "CaseEagerEvaluator[conditions=[" + conditionFactory + "], elseVal=" + elseValueFactory + ']'; | ||
} | ||
} | ||
|
||
private record CaseEagerEvaluator( | ||
ElementType resultType, | ||
BlockFactory blockFactory, | ||
ConditionEvaluator condition, | ||
EvalOperator.ExpressionEvaluator elseVal | ||
) implements EvalOperator.ExpressionEvaluator { | ||
@Override | ||
public Block eval(Page page) { | ||
try ( | ||
BooleanBlock lhsOrRhsBlock = (BooleanBlock) condition.condition.eval(page); | ||
ToMask lhsOrRhs = lhsOrRhsBlock.toMask(); | ||
Block lhs = condition.value.eval(page); | ||
Block rhs = elseVal.eval(page); | ||
Block.Builder builder = resultType.newBlockBuilder(lhs.getTotalValueCount(), blockFactory) | ||
) { | ||
if (lhsOrRhs.hadMultivaluedFields()) { | ||
condition.registerMultivalue(); | ||
} | ||
for (int p = 0; p < lhs.getPositionCount(); p++) { | ||
|
||
if (lhsOrRhs.mask().getBoolean(p)) { | ||
builder.copyFrom(lhs, p, p + 1); | ||
} else { | ||
builder.copyFrom(rhs, p, p + 1); | ||
} | ||
} | ||
return builder.build(); | ||
} | ||
} | ||
|
||
@Override | ||
public void close() { | ||
Releasables.closeExpectNoException(condition, elseVal); | ||
} | ||
|
||
@Override | ||
public String toString() { | ||
return "CaseEagerEvaluator[conditions=[" + condition + "], elseVal=" + elseVal + ']'; | ||
} | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
another alternative is to be extract this as a marking interface that gets implemented by certain factories.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think that's a lot less readable.