-
Notifications
You must be signed in to change notification settings - Fork 1.3k
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
Try to push down full filter before break-up #5367
Conversation
As an alternative to jumping to full support of ordered indexes, here is a lighter weight PR that would still unblock me. |
I have a question / comment: #5357 (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.
LGTM.
) -> Result<Vec<TableProviderFilterPushDown>> { | ||
filters | ||
.iter() | ||
.map(|f| self.supports_filter_pushdown(f)) |
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.
👍
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.
Seems reasonable to me
These tests are showing up as broken in this branch, but they appear to be broken in main as well:
|
ed4f554
to
3e34c53
Compare
3e34c53
to
fb8814b
Compare
Benchmark runs are scheduled for baseline = 38185ca and contender = fad360d. fad360d is a master commit associated with this PR. Results will be available as each benchmark for each run completes. |
Which issue does this PR close?
Closes #5357.
Rationale for this change
Allow TableProviders with multi-column indexes to fully filter results. Presently, if a TableProvider is ordered by
[col_a, col_b]
, and it gets a querywhere col_a=1 and col_b=2
, then it receives two calls tosupports_filter_pushdown()
: one forcol_a
and one forcol_b
and it would have to returnexact
forcol_a
andUnsupported
forcol_b
because it can filter them together, and it can filtercol_a
because it's first in the sort order, but it cannot filtercol_b
on it's own.This change would allow the TableProvider to say "Yes, I can filter expressions like
col_a = x and col_b = y
.What changes are included in this PR?
Change the TableProvider API to have a single method that accepts the entire Vec of Exprs and returns a Vec of results so the TableProvider can assess them all wholistically.
Are these changes tested?
yes.
Are there any user-facing changes?
No