Suggest get_expr_planners() return Vec<> rather than &[]
- Dominant language
- Rust
- Stars
- 9.3k
- Forks
- 2.4k
- Avg merge
- 3d 7h
- Merged PRs (30d)
- 344
Description
[see here](https://github.com/apache/datafusion/blob/7a1a23d5f50cd113613cf54ee5346f0e83d1edc4/datafusion/core/src/execution/session_state.rs#L1501C1-L1504C6)
Basically this implementation uses a private field access on `SessionState` to return the slice, for us normals we have to call `expr_planners() which will give us a [Vec](https://doc.rust-lang.org/nightly/alloc/vec/struct.Vec.html)<[Arc](https://doc.rust-lang.org/nightly/alloc/sync/struct.Arc.html)>
To not share a temporary value in my ContextProvider implementation of `get_expr_planners` I have to hold onto the Vec in my own state which feels clunky compared to other functions like `udfs()` where I'm just returning `self.state.scalar_functions().keys().cloned().collect()·`
I think changing the API to use Vec rather than a slice would be more symmetrical and easier to just defer up to SessionState in my impl
Contributor guide
Research direction
Start with datafusion/core/src/execution/session_state.rs at the referenced get_expr_planners implementation, then inspect the ContextProvider trait and its callers. Check how expr_planners() and related methods such as udfs() expose state. Done means the API shape is consistent with the requested Vec return and affected code and tests still compile and pass.
Written by the indexing model from the issue text.
Assessment
- Tech stack
- rust
- Domain
- api, backend-api-design
- Issue type
- Refactor
- Difficulty
- 4/5
- Estimated time
- 3-5 days
- Activity status
- Stale
- Clarity
- Mostly clear
- Newbie friendliness
- 38/100