Repository navigation
feat(federation): let a SqlTable's federated scan take a sort - #91
Merged
Merged
Conversation
`SqlTable::supports_sort_pushdown` returns `allow_physical_sort_pushdown`, the switch its own scan already honors, so a sort above a federated `SqlTable` scan reaches the remote engine instead of sorting every row locally. A table with the switch off (ScyllaDB) keeps the sort local. Pins datafusion-federation at spiceai/datafusion-federation#93, which adds `SQLExecutor::supports_sort_pushdown`.
There was a problem hiding this comment.
🟡 Changes recommended
The existing switch does not guarantee the exact collation semantics required by federated sort pushdown.
1 open finding
What changed in this PR
Enables federated SqlTable scans to push sorts into remote SQL.
Changes:
- Connects federated sort support to the existing physical-sort switch.
- Adds switch-behavior coverage.
- Updates the federation dependency pin.
| File | Description |
|---|---|
core/src/sql/sql_provider_datafusion/federation.rs |
Enables and tests federated sort pushdown. |
Cargo.toml |
Updates the federation revision. |
Cargo.lock |
Locks the updated revision. |
🧠 Review effort: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
This was referenced Oct 12, 2026
spiceai/datafusion-federation#93 was squash-merged as 0ef7d34, whose tree is identical to e93dfc3, the head this branch pinned.
krinart
added a commit
to spiceai/spiceai
that referenced
this pull request
Oct 13, 2026
spiceai/datafusion-table-providers#91 was squash-merged as 201f519, whose tree is identical to ad4e086, the branch head pinned before. Both forks are now on landed revisions, so the TEMPORARY marker is dropped.
6 tasks done
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.
SqlTableopts in to the federated sort pushdown that spiceai/datafusion-federation#93 adds.SQLExecutor::supports_sort_pushdownreturnsallow_physical_sort_pushdown, the switchSqlExec::try_pushdown_sortalready honors. A sort above a federatedSqlTablescan then reaches the remote engine instead of being applied locally to every row. A table with the switch off, asScyllaDBsets it, keeps the sort local, as its own scan already does.The
datafusion-federationpin moves to0ef7d34, thespiceai-55squash merge of spiceai/datafusion-federation#93, which adds the trait method. dtp's other table types (MySQLTable,DuckDBTable,SQLiteTable,AdbcDBTable,ClickHouseTable) keep the default,false, so their behavior is unchanged.Spice moves both pins together in spiceai/spiceai#15078, because it takes
datafusion-federationfrom the same git URL.Evidence (commit 44f0766, 2026-10-12;
ad4e086only repoints the pin frome93dfc3to0ef7d34, whose trees are identical)Spice-side, with this commit and spiceai/datafusion-federation#93 patched in by path:
crates/data_components/src/federation.rs::tests::sort_pushdown::a_sort_above_a_refused_projection_reaches_the_sourcefails with the opt-in removed and fails with the switch ignored. It passes with this change. ItsPostgreSQLend-to-end numbers are in spiceai/datafusion-federation#93.