apache / apache/datafusion

Add support for SessionState in supports_filters_pushdown for a Custom Data Source

Open
#11,193 15 comments 0 reactions 0 assignees View on GitHub
enhancement
Dominant language
Rust
Stars
9.3k
Forks
2.4k
Avg merge
3d 7h
Merged PRs (30d)
344

Description

### Is your feature request related to a problem or challenge?

We need the ability to get the `TaskContext.task_id` any place where a Custom Data Source is invoked. As it stands currently, the `state: &SessionState` is available in `TableProvider.scan` and `task_ctx: Arc` is available in `ExecutionPlan.execute`, but not in the `supports_filters_pushdown`. This prohibits per-query customization or tracking of external state in this method. For example if there are 3 `filters` for a custom table, and 10 are possible, we need to be able to choose the best one at runtime.

Further, the `task_id` should always be available by passing the `TaskContext` or from `SessionState` to keep things consistent.

In trying to implement this it proved infeasible because `supports_filters_pushdown` is in 2 interfaces in 2 separate crates: `TableProvider` (in `core`) and `TableSource` (in `expr`). It is not possible to add `state: &SessionState` to the `TableSource` implementation as it cannot access the `core` crate, a cyclic dependency occurs the way it is now. This was intentional to make `LogicalPlan` separable, which makes sense, but preventing this type of enhancement.

### Describe the solution you'd like

Add `&SessionState` or minimally `TaskContext` in every pertinent method for per-query specific processing in a custom data source.

A possible way to solve this is to make a new `datafusion-traits` crate, and to move `SessionState` and other common items to `datafusion-common`, such that these components are used by `core` and `expr`. It will make some components available in `expr` that are not strictly necessary, but I think that is a good trade-off. This work could be combined with other efforts to break `core` into more sub-crates, that would make DataFusion much more flexible overall.

### Describe alternatives you've considered

_No response_

### Additional context

Restructuring crates in a project of this size will be a lot of work, but I believe the benefit will be there. There are other issues that also would benefit. I would recommended a separate restructure ticket that can be reviewed before any implementation is attempted. In addition then this would need to be implemented by multiple contributors, it will inevitably cause a lot of temporary breakage and retesting will also be required.

Contributor guide

Open the contributing guide

Research direction

Start by comparing supports_filters_pushdown in the TableProvider interface in core with the TableSource interface in expr, and trace how SessionState and TaskContext are currently passed to TableProvider.scan and ExecutionPlan.execute. Before implementation, define and review a crate-restructuring plan that resolves the core/expr dependency cycle; done means custom data sources can access per-query context in every pertinent method without breaking the separated LogicalPlan design.

Written by the indexing model from the issue text.

Assessment

Tech stack
rust
Domain
backend-api-design, data-engineering
Issue type
Feature
Difficulty
5/5
Estimated time
Over a week
Activity status
Stale
Clarity
Needs clarification
Newbie friendliness
25/100

Get new issues in your inbox

A short digest of beginner-friendly GitHub issues.