Implement user defined planner for position #11243
Conversation
github-actions
bot
added
sql
|
🤔 We seem to be hitting the dreaded sql planner stack overflow in debug builds again https://github.com/apache/datafusion/actions/runs/9778881627/job/26996930268?pr=11243 I think best way to fix this is to make a function (rather than inlining the code into the same big match expression) |
| #[derive(Default)] | ||
| pub struct PositionPlanner; | ||
|
|
||
| impl UserDefinedSQLPlanner for PositionPlanner { |
There was a problem hiding this comment.
@jayzhan211 do you suggest we have one planner for each module (like a UnicodePlanner rather than a PositionPlanner?
I think this is similar to what you are proposing with CoreFunctionsPlanner in #11273 (comment) in #11273
We could make this change in a follow on PR too
There was a problem hiding this comment.
@jayzhan211 do you suggest we have one planner for each module (like a UnicodePlanner rather than a PositionPlanner?
Yes. And with different plan_* function
There was a problem hiding this comment.
Filed #11305 to track. Thank you
There was a problem hiding this comment.
Thank you for the contribution @xinlifoobar -- this makes sense to me
There was a problem hiding this comment.
lgtm thanks @xinlifoobar but I have same concern as @alamb on having a separate planner
|
🚀 let's consolidate the planners as a follow on PR |
* Implement user defined planner for position * Fix format * Move planner to session_state * Extract function
Which issue does this PR close?
Closes #11242
Rationale for this change
What changes are included in this PR?
Are these changes tested?
Are there any user-facing changes?