apache / datafusion

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

Move `sql_compound_identifier_to_expr` to `ExprPlanner` #11473

Closed alamb closed 1 month ago

alamb commented 1 month ago

Is your feature request related to a problem or challenge?

Part of https://github.com/apache/datafusion/issues/11207

As discussed in https://github.com/apache/datafusion/issues/11207 The idea is to move hardcoded the sql_compound_identifier_to_expr operator to function in the parser step, which is currently handled in the optimizer step. We need to implement user-defined parser mechanisms to enable more flexible SQL planning.

Describe the solution you'd like

No response

Describe alternatives you've considered

I think you can just follow the model of https://github.com/apache/datafusion/pull/11398 and add a function to ExprPlanner and then update the sql planner to call it instead of get_functon_meta

Additional context

No response

dharanad commented 1 month ago

seems like a duplicate of https://github.com/apache/datafusion/issues/11244 i will close and we can keep this open

dharanad commented 1 month ago

take

alamb commented 1 month ago

Sorry @dharanad -- I didn't see https://github.com/apache/datafusion/issues/11244 before

dharanad commented 1 month ago

Sorry @dharanad -- I didn't see #11244 before

No worries at all.

dharanad commented 1 month ago

QQ: @alamb In sql_compound_identifier_to_expr the only this which is using get_function_meta is get_field ScalarUDF. I think ideally we should move get_field to ExprPlanner. Rest of code as dependecy on SqlToRel struct

alamb commented 1 month ago

I think ideally we should move get_field to ExprPlanner. Rest of code as dependecy on SqlToRel struct

Makes sense to me 👍

alamb commented 1 month ago

After https://github.com/apache/datafusion/pull/11487 I wonder if there is anythng left for this item?

jayzhan211 commented 1 month ago

I think we are done with this item