clintropolis commented on pull request #10613: URL: https://github.com/apache/druid/pull/10613#issuecomment-787199952
>At this point I wanted to ask about the testing. I haven't looked at it much, and I would deeply appreciate it if you could describe briefly what the testing strategy is for this new feature, and why you think it covers all the important cases. That'll make reviewing it a lot quicker. I guess there are 2 main areas of changes, some modifications in `druid-core` at the `Expr` layer so that logical operations can process single value string columns, and in `druid-processing` vector processors and filter matcher stuffs. The `Expr` changes are mostly covered by `VectorExprSanityTest`, which does a sort of brute-force comparision of vectorized and non-vectorized expressions and ensures that the outputs are identical, but I added a few other odds and ends to cover the changes to null constants. For the filter changes, most of the coverage is provided by the tests that extend `BaseFilterTest`, which itself has been improved to include testing matches against a virtual column (which was missing before, and serves as coverage for the bug for vectorized filter matching on virtual columns which is fixed by this PR), with new test cases added to `SelectorFilterTest` and `BoundFilterTest` to cover this and new functionality. In `ExpressionFilterTest`, every vectorizable expression is now using `assertFilterMatches ` instead of `assertFilterMatchesSkipVectorize`, but there are a couple of stragglers such as the `like` expression which are not vectorizable yet still using the latter method. A couple calcite tests which previously could not vectorize are now vectorizable as well. The only stuff without great coverage as far as I can tell from the report is the changes vector processor stuff with group by engine, but that will be covered in a follow-up PR that adds the vectorized expression support to vector group by (dictionary building vectorized group by), and the `decorate` methods in `DimensionSpec` which i think is expected since nothing implements them (or the other vectorized decorate methods for that matter) ---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: [email protected] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
