-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Alamb/tests for page reading #9281
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
sdf-jkl
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks really good
Thank you -- I am getting there. I need to add a few more combinations and then I expect to start hitting some of the bugs |
|
Update: the tests are coming along, but they are not yet at the state where they reproduce the issue yet (I believe doing so requires more than one arrow predicate) i need to catch up on other reviews, etc today now, but I will get back to these tests tomorrow and hopefully finish them up (and then get back to the actual PR !) |
Which issue does this PR close?
Rationale for this change
As we go about implementing the changes in
I am really struggling to be convinced that the logic for multi-column page skipping is accurate. Thus I am working on adding additional test coverage
What changes are included in this PR?
Add additional test coverage for multi-column row filter predicates
TODO more tests
Are these changes tested?
Yes, with CI
Are there any user-facing changes?
No