clintropolis commented on PR #15694:
URL: https://github.com/apache/druid/pull/15694#issuecomment-1905477915
>@clintropolis I don't fully understand how the suggested Supplier based
solution should work in this case - could you please expand on that idea a bit
more?
Oh, by suppliers i was talking about `GenericIndexed`, where
`singleThreaded()` is used as a supplier of `Indexed` for everything that
actually uses `GenericIndexed` instead of using the thread-safe version
directly, which was my point here, that `eval` on `Expr` seems pretty useless
since everything should always call the single threaded method.
So basically, I was wondering why have `eval` on `Expr` at all. I have a
feeling this would be a bit too disruptive to actually do, but I really wish it
wasn't because I think this would be my preference...
The basic idea would be to make non-vectorized processing have the same
pattern as vectorized processing. The `eval` method would be removed completely
from `Expr`, and instead, `Expr` would have a new method, `asProcessor`, e.g.
something like
```java
<T> ExprProcessor<T> asProcessor(InputBindingInspector inspector);
```
where
```java
public interface ExprProcessor<TOutput>
{
ExprEval<TOutput> eval(Expr.ObjectBinding bindings);
ExpressionType getOutputType();
}
```
How it would solve this and the original problem is that `Expr` remain
immutable/thread-safe, `asProcessor` is the conceptual equivalent of
`asSingleThreaded` to use whenever we actually need to eval the expression (and
so for a constant processor we would cache the `ExprEval` as part of the
processor instead of the `Expr`), and also gives us the opportunity to do all
sorts of other specialization in the cases where the input types are known up
front (today all `eval` implementations must be able to handle any input type
because of the few cases where types are unknown).
I guess we could leave the `eval` method as deprecated and a default
implementation for a `ExprProcessor` to make it a bit less disruptive, since
this really would be a lot nicer imo, and less confusing than the
`makeSingleThreaded` which returns another `Expr`.
--
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]
