gianm commented on code in PR #15694:
URL: https://github.com/apache/druid/pull/15694#discussion_r1509492442
##########
processing/src/main/java/org/apache/druid/math/expr/Expr.java:
##########
@@ -763,4 +763,36 @@ private static Set<String> map(
return results;
}
}
+
+
+ /**
+ * Returns the single-threaded version of the given expression tree.
+ *
+ * Nested expressions in the subtree are also optimized.
+ * Individual {@link Expr}-s which have a singleThreaded implementation via
{@link SingleThreaded} are substituted.
+ */
+ static Expr singleThreaded(Expr expr)
+ {
+ return expr.visit(
+ node -> {
+ if (node instanceof SingleThreaded) {
+ SingleThreaded canBeSingleThreaded = (SingleThreaded) node;
+ return canBeSingleThreaded.toSingleThreaded();
+ } else {
+ return node;
+ }
+ }
+ );
+ }
+
+ /**
+ * Implementing this interface allows to provide a non-threadsafe {@link
Expr} implementation.
+ */
+ interface SingleThreaded
+ {
+ /**
+ * Non-threadsafe of this expression.
Review Comment:
"version of" (missing word)
##########
processing/src/main/java/org/apache/druid/math/expr/ConstantExpr.java:
##########
@@ -100,6 +106,67 @@ public String stringify()
{
return toString();
}
+
+ @Override
+ public final ExprEval eval(ObjectBinding bindings)
+ {
+ return realEval();
+ }
+
+ protected abstract ExprEval<T> realEval();
+
+
+ @Override
+ public Expr toSingleThreaded()
+ {
+ return new ExprEvalBasedConstantExpr<T>(realEval());
+ }
+
+ /**
+ * Constant expression based on a concreate ExprEval.
+ *
+ * Not multi-thread safe.
+ */
+ @NotThreadSafe
+ @SuppressWarnings("Immutable")
+ private static final class ExprEvalBasedConstantExpr<T> extends
ConstantExpr<T>
+ {
+ private final ExprEval<T> eval;
+
+ private ExprEvalBasedConstantExpr(ExprEval<T> eval)
+ {
+ super(eval.type(), eval.value);
+ this.eval = eval;
+ }
+
+ @Override
+ public ExprEval<T> realEval()
Review Comment:
Could stay `protected`? I got briefly confused as to why this was `public`
but I think it doesn't need to be. Similar comments for the others.
##########
processing/src/main/java/org/apache/druid/math/expr/Expr.java:
##########
@@ -763,4 +763,36 @@ private static Set<String> map(
return results;
}
}
+
+
+ /**
+ * Returns the single-threaded version of the given expression tree.
+ *
+ * Nested expressions in the subtree are also optimized.
+ * Individual {@link Expr}-s which have a singleThreaded implementation via
{@link SingleThreaded} are substituted.
+ */
+ static Expr singleThreaded(Expr expr)
+ {
+ return expr.visit(
+ node -> {
+ if (node instanceof SingleThreaded) {
+ SingleThreaded canBeSingleThreaded = (SingleThreaded) node;
+ return canBeSingleThreaded.toSingleThreaded();
+ } else {
+ return node;
+ }
+ }
+ );
+ }
+
+ /**
+ * Implementing this interface allows to provide a non-threadsafe {@link
Expr} implementation.
+ */
+ interface SingleThreaded
Review Comment:
The name here is misleading, since it's not that the Expr _is_
single-threaded, it's that it has a faster single-threaded implementation
available. Maybe a better name is: `SingleThreadSpecializable`
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
