Repository navigation
refactor: use inherent is::<T>() on dyn TableProvider/dyn ExecutionPlan for type checks - #11585
Merged
Merged
Conversation
… for type checks DataFusion 54 (apache/datafusion#21346) makes `Any` a supertrait of `TableProvider`/`ExecutionPlan` and adds inherent `is::<T>()` / `downcast_ref::<T>()` on the trait objects. Replace `downcast_ref::<T>().is_some()" / ".is_none()` chains with `is::<T>()` wherever the receiver is one of those two trait objects. No behavior change; several multi-line wrapper-allowlist conditions in the cayenne optimizer collapse to one line per type check.
Contributor
There was a problem hiding this comment.
Pull request overview
Refactors internal type checks across the query planning/optimization and distributed codec codepaths to use DataFusion 54’s inherent is::<T>() on dyn TableProvider / dyn ExecutionPlan, replacing verbose downcast_ref::<T>().is_some()/is_none() chains with clearer boolean predicates (no intended behavior change).
Changes:
- Replace
downcast_ref::<T>().is_some()withis::<T>()forExecutionPlan/TableProvidertrait objects. - Replace
downcast_ref::<T>().is_none()chains with!is::<T>(), notably simplifying Cayenne optimizer wrapper allowlist checks. - Apply the same readability refactor in related tests and distributed plan/table-provider codec logic.
Reviewed changes
Copilot reviewed 15 out of 15 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| crates/vortex/src/persistent/metrics.rs | Simplifies ExecutionPlanVisitor node-type detection using plan.is::<DataSourceExec>(). |
| crates/runtime/tests/cayenne/mod.rs | Uses is::<AcceleratedTable>() for asserting provider wrapper type in integration test. |
| crates/runtime/src/datafusion/planner/mod.rs | Updates wrapper-peeling tests to assert provider type via is::<MemTable>(). |
| crates/runtime/src/datafusion/mod.rs | Updates accelerated-provider resolution test to assert MemTable via is::<MemTable>(). |
| crates/runtime/src/datafusion/builder.rs | Simplifies Cayenne table-provider detection with provider.is::<CayenneTableProvider>(). |
| crates/runtime/src/dataconnector/iceberg_cluster.rs | Uses plan.is::<IcebergTableScan>() before wrapping for distributed planning. |
| crates/runtime/src/cluster/datafusion/codec/spice_physical_codec.rs | Uses is::<...>() for physical plan node encoding decisions where applicable. |
| crates/runtime/src/cluster/datafusion/codec/spice_logical_codec.rs | Uses is::<ListUDFTable>() when encoding UDTF-produced TableProviders. |
| crates/runtime/src/accelerated_table/refresh_task.rs | Updates test assertion to is::<MetadataEnrichedTableProvider>() on underlying provider. |
| crates/runtime-datafusion/src/extension/bytes_processed.rs | Uses is::<BytesProcessedExec>() in physical optimizer early-exit condition. |
| crates/datafusion-optimizer-rules/src/physical_plan/http_subquery_pushdown.rs | Uses is::<HttpExec>() and updates related test assertions. |
| crates/datafusion-optimizer-rules/src/physical_plan/flightsql/broadcast_join.rs | Collapses multi-is_none() wrapper walk conditions into !is::<...>() checks. |
| crates/datafusion-optimizer-rules/src/physical_plan/flightsql/aggregate_pushdown.rs | Uses is::<...>() when walking/identifying wrapper nodes and test plan types. |
| crates/cayenne/src/provider/scan.rs | Simplifies wrapper allowlist/type recognition with is::<...>(). |
| crates/cayenne/src/optimizer_rules.rs | Rewrites long wrapper-allowlist is_none() chains into readable !plan.is::<...>() predicates and updates test assertions. |
Contributor
✅ Pull with Spice PassedPassing checks:
|
Jeadie
marked this pull request as ready for review
July 6, 2026 23:44
phillipleblanc
approved these changes
Jul 7, 2026
Jeadie
enabled auto-merge
July 9, 2026 00:18
github-actions Bot
pushed a commit
that referenced
this pull request
Jul 13, 2026
…ionPlan` for type checks (#11585) * refactor: use inherent `is::<T>()` on dyn TableProvider/ExecutionPlan for type checks DataFusion 54 (apache/datafusion#21346) makes `Any` a supertrait of `TableProvider`/`ExecutionPlan` and adds inherent `is::<T>()` / `downcast_ref::<T>()` on the trait objects. Replace `downcast_ref::<T>().is_some()" / ".is_none()` chains with `is::<T>()` wherever the receiver is one of those two trait objects. No behavior change; several multi-line wrapper-allowlist conditions in the cayenne optimizer collapse to one line per type check. * fix formatting --------- Co-authored-by: Jeadie <[email protected]>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
DataFusion 54 (apache/datafusion#21346) made
Anya supertrait ofTableProvider(andExecutionPlan), adding inherentis::<T>()anddowncast_ref::<T>()methods on the trait objects.I thought we might be able to use these to improve recursive downcasting that is often buggy and brittle (e.g.
search::util::find_concrete_table_providerorsearch::util::find_index_in_table_provider), but was not able. Instead I found and replacedx.downcast_ref::<T>().is_some()/.is_none()chains withx.is::<T>()/!x.is::<T>().