Fix Incoherency in Expression Selectors

[nishantmonu51]

Review comment - #5278 (comment)
There is an incoherency. This is a selector of ExprEval, not of ExprEval.value(). This class is used only in ExpressionSelectors.makeExprEvalSelector(). This incoherency already causes problems in AggregatorUtil: ExpressionSelectors.makeExprEvalSelector() is used there only to call getObject(), not taking advantage of ConstantColumnValueSelector optimization.

To make things more coherent, either

ExpressionSelectors.makeExprEvalSelector() should return ColumnValueSelector, not of ExprEval (with according changes)
Or Supplier should be returned from this method, enforcing all clients to go via get().asDouble() path (and ConstantColumnValueSelector should adapt accordingly).

[nishantmonu51]

+cc @gianm for thoughts.

[gianm]

The main reason I did this was because some selectors wrap expression selectors and prefer an ExprEval object so they can get object casting behavior (asDouble, asString, etc). If we have some standard functions for casting Object to various Druid types then I think the ExprEval selectors aren't needed, and we can go with the first solution: "ExpressionSelectors.makeExprEvalSelector() should return ColumnValueSelector, not of ExprEval"

[gianm]

Btw, there is no incoherency that I'm aware of in the current code. It is just confusing since there are both ExprEval selectors and normal value Object selectors, and callers have to use the right one.

[nishantmonu51]

Thanks for your thoughts @gianm. Will refactor things accordingly in a follow up PR.
the incoherancy in comment here is for the newly added isNull method, which returns false for ObjectSelectors instead of computing the object and determine its nullability for performance reasons.

[leventov]

@gianm the fact that AggregatorUtil misses the optimization at the moment is the sign of the inconsistency.

[github-actions]

This issue has been marked as stale due to 280 days of inactivity.
It will be closed in 4 weeks if no further activity occurs. If this issue is still
relevant, please simply write any comment. Even if closed, you can still revive the
issue at any time or discuss it on the dev@druid.apache.org list.
Thank you for your contributions.

[github-actions]

This issue has been closed due to lack of activity. If you think that
is incorrect, or the issue requires additional review, you can revive the issue at
any time.