feat: allow pushdown of dynamic filters having partition cols #18172
Conversation
feniljain
force-pushed
the
feat-dynamic-filter-join-partition-col
branch
from
2e64a1e to
1a02eae
Compare
feniljain
changed the title
feniljain
force-pushed
the
feat-dynamic-filter-join-partition-col
branch
4 times, most recently
from
115d3c1 to
2d8fb9c
Compare
feniljain
force-pushed
the
feat-dynamic-filter-join-partition-col
branch
2 times, most recently
from
e58d9ec to
2c1ee6e
Compare
|
@feniljain mind if I push 7818a42c7 to your branch / this PR? |
|
Sure thing! No worries, thanks for the patch :) |
adriangb
force-pushed
the
feat-dynamic-filter-join-partition-col
branch
from
8290691 to
05284cf
Compare
github-actions
bot
added
the
documentation
adriangb
force-pushed
the
feat-dynamic-filter-join-partition-col
branch
from
22079c6 to
503a5d8
Compare
|
@feniljain thanks for your patience. I had to resolve some conflicts but I think this is looking good now. I'll give it one last review tomorrow and then we can probably merge it. |
| _execution plan_ of the query. With this release, `DESCRIBE query` now outputs | ||
| the computed _schema_ of the query, consistent with the behavior of `DESCRIBE table_name`. | ||
|
|
||
| ### Introduction of `TableSchema` and changes to `FileSource::with_schema()` method |
There was a problem hiding this comment.
Note that TableSchema was not added in this PR but this PR does use it in a way that introduces a notable breaking change, so I think it's an appropriate time to add it to the upgrade guide.
friendlymatthew
left a comment
friendlymatthew
left a comment
There was a problem hiding this comment.
This makes sense to me. Thanks @feniljain
adriangb
force-pushed
the
feat-dynamic-filter-join-partition-col
branch
from
503a5d8 to
b6375bb
Compare
|
@alamb, @xudong963 or @Jefffrey could one of you approve or review this PR if you have time? It's good to me but since I pushed some commits I cannot approve it. I'd like to merge it before it gets more conflicts (I've had to resolve already). |
Added documentation explaining the introduction of the TableSchema struct and the breaking change to FileSource::with_schema() method signature (changed from SchemaRef to TableSchema). Includes: - Overview of TableSchema purpose and structure - Who is affected by the change - Migration guide for custom FileSource implementations - Code examples showing how to update implementations - Examples of using TableSchema directly 🤖 Generated with [Claude Code](https://claude.com/claude-code) Co-Authored-By: Claude <[email protected]>
adriangb
force-pushed
the
feat-dynamic-filter-join-partition-col
branch
from
b6375bb to
220060d
Compare
I can try take a look at this if I'm able, though I am not too familiar with dynamic filters in DataFusion |
|
@alamb, @xudong963 or @Jefffrey could one of you approve or review this PR if you have time? It's good to me but since I pushed some commits I cannot approve it. I'd like to merge it before it gets more conflicts (I've had to resolve already). FWIW I don't think there is any reason you can't approve it after pushing commits |
Hmm it didn't seem to let me hit the queue button but I guess as a matter of procedure probably good to get another approval anyway :) |
|
Huh got it to go now! |
…#18172) ## Which issue does this PR close? - Closes apache#18171 ## Rationale for this change Included in the issue ## Are these changes tested? While I have tested this on local with a local TPCDS-like dataset, I would appreciate if someone provides me a good way to add tests for the same 😅 --------- Co-authored-by: Adrian Garcia Badaracco <[email protected]> Co-authored-by: Claude <[email protected]>
…#18172) ## Which issue does this PR close? - Closes apache#18171 ## Rationale for this change Included in the issue ## Are these changes tested? While I have tested this on local with a local TPCDS-like dataset, I would appreciate if someone provides me a good way to add tests for the same 😅 --------- Co-authored-by: Adrian Garcia Badaracco <[email protected]> Co-authored-by: Claude <[email protected]>
5 participants
Which issue does this PR close?
Rationale for this change
Included in the issue
Are these changes tested?
While I have tested this on local with a local TPCDS-like dataset, I would appreciate if someone provides me a good way to add tests for the same 😅