yahoNanJing commented on PR #8669: URL: https://github.com/apache/arrow-datafusion/pull/8669#issuecomment-1872529557
> First of all, thank you again @yahoNanJing -- this is really important functionality. > > After reviewing the tests carefully, before merging, I think this PR needs > > 1. We need to resolve the `col IS NULL` question (I may just still be confused) > 2. Some additional tests to avoid regressions > > In terms of additional tests I think the current tests would not fail if certain behaviors of the code were broken. Thus I suggest we add the following cases that have special handling int he code. > > 1. A test ensuring that multiple guarantees are correctly applied. For example, a predicate like `col1 IN (10) AND col2 IN (20)` and row groups such that `col1 IN (10)` is can be true but `col2 IN (20)` does not. > 2. A test with a predicate on a column that has no statistics > 3. A test where the statistics return the incorrect data type (e.g. that the cast has to be present). Thanks @alamb for your review and suggestions. It's my bad. Maybe multiple rules are mixed together so that the `RowGroupPruningStatistics`'s `contained` implementation does not affect the final result. I will introduce independent test cases for this implementation in a few days. -- 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]
