apache / datafusion

Apache DataFusion SQL Query Engine
https://datafusion.apache.org/
Apache License 2.0
6.23k stars 1.18k forks source link

Suggest get_expr_planners() return Vec<> rather than &[] #11960

Open rtyler opened 2 months ago

rtyler commented 2 months ago

see here

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<Arc<dyn ExprPlanner>>

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

jayzhan211 commented 2 months ago

Couldn't we use to_vec() to get Vec<Arc<dyn ExprPlanner>>?

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

Do you have an example for share a temporary value?

alamb commented 2 months ago

Maybe it is easiest to explain the proposal with a PR as well