allow Expr to provide optimized BitmapColumnIndex implementations to optimize ExpressionFilter #18094
Conversation
…optimize ExpressionFilter changes: * added `asBitmapColumnIndex` method to `Expr`, pushing down the default implementation of `ExpressionFilter.getBitmapColumnIndex` into this new method * added `asColumnIndexSupplier` and `asBitmapColumnIndex` to `Function` interface, to allow `Expr` that use `Function` to further specialize participation in filtering * `ExpressionFilter` now completely delegates to `Expr` for its `getBitmapColumnIndex` method * Moved predicate factory out of `ExpressionFilter` to standalone `ExpressionDruidPredicateFactory` since it is now used by the default implementation of `Expr.asBitmapColumnIndex` * added implementation of `asBitmapColumnIndex` to `BinEqExpression` so that the equals expression can use `ValueIndex` (except for string columns matching numeric values since that requires predicate indexes) * added implementation of `asBitmapColumnIndex` to `MvOverlapConstantArray` function to allow it use to `ValueSetIndex` with a string match value type
![github-advanced-security[bot]](https://avatars.githubusercontent.com/in/57789?s=60&v=4){kind=small}
| */ | ||
| @Nullable | ||
| default ColumnIndexSupplier asColumnIndexSupplier( | ||
| ColumnIndexSelector selector, |
Check notice
Code scanning / CodeQL
Useless parameter Note
| @Nullable | ||
| default ColumnIndexSupplier asColumnIndexSupplier( | ||
| ColumnIndexSelector selector, | ||
| @Nullable ColumnType outputType, |
Check notice
Code scanning / CodeQL
Useless parameter Note
| default ColumnIndexSupplier asColumnIndexSupplier( | ||
| ColumnIndexSelector selector, | ||
| @Nullable ColumnType outputType, | ||
| List<Expr> args |
Check notice
Code scanning / CodeQL
Useless parameter Note
| } | ||
| } | ||
| return null; | ||
| return expr.get().asBitmapColumnIndex(selector); |
|
|
||
| @Nullable | ||
| @Override | ||
| public BitmapColumnIndex asBitmapColumnIndex(ColumnIndexSelector selector, List<Expr> args) |
There was a problem hiding this comment.
What testing strategy do you have in mind for stuff like this? I suppose we could add items to ExpressionFilterTest. But ideally it would be possible to test them in FunctionTest. What do you think about extending that to have an assertBooleanExpr that calls asBitmapColumnIndex with an index selector that has indexes for the bindings?
With that, I'd also suggest updating assertExpr to check if the expr has a boolean return type, and throw if it does (so they all get moved to assertBooleanExpr).
Description
changes:
asBitmapColumnIndexmethod toExpr, pushing down the default implementation ofExpressionFilter.getBitmapColumnIndexinto this new methodasColumnIndexSupplierandasBitmapColumnIndextoFunctioninterface, to allowExprthat useFunctionto further specialize participation in filteringExpressionFilternow completely delegates toExprfor itsgetBitmapColumnIndexmethodExpressionFilterto standaloneExpressionDruidPredicateFactorysince it is now used by the default implementation ofExpr.asBitmapColumnIndexasBitmapColumnIndextoBinEqExpressionso that the equals expression can useValueIndex(except for string columns matching numeric values since that requires predicate indexes)asBitmapColumnIndextoMvOverlapConstantArrayfunction to allow it use toValueSetIndexwith a string match value typeThis PR has: