ExprPlanner not propagated to SqlToRel

[wjones127]

Describe the bug

When upgrading to DataFusion 40, I kept on encountering this error:

Internal error: Expected a simplified result, but none was found.

This happened when parsing a array literal. It seems in order to parse it, we need to register the proper ExprPlanner on the SessionState. We use SessionState::default(), which registers it for us. But then it's not being propagated to SqlToRel, so it fails. (Note the planners: vec![])

datafusion/datafusion/sql/src/planner.rs

Lines 206 to 213 in f204869

	pub fn new_with_options(context_provider: &'a S, options: ParserOptions) -> Self {
	let normalize = options.enable_ident_normalization;
	SqlToRel {
	context_provider,
	options,
	normalizer: IdentNormalizer::new(normalize),
	planners: vec![],

To Reproduce

No response

Expected behavior

I'm not sure I understand the design, but it seems odd we don't propagate these. At the very least, we should provide a warning in the docs.

Additional context

Current workaround:

for planner in context_provider.state.expr_planners() {
    sql_to_rel = sql_to_rel.with_user_defined_planner(planner.clone());
}

[wjones127]

@jayzhan211 could you confirm what the intended behavior is for this?

[jayzhan211]

We assume planner should exist, at least with datafusion's builtin one, otherwise we don't know how to handle it thus throws error for the case.

[jayzhan211]

After #11403, I think you can easily get the default planners with

SessionStateBuilder::new()
            .with_default_features()
            .build()

[wjones127]

We assume planner should exist, at least with datafusion's builtin one, otherwise we don't know how to handle it thus throws error for the case.

I'm not sure I understand your answer. When we call SessionState::default(), all the correct planners are registered. For example, it already has the ArrayFunctionPlanner it should need to parse array literals.

What I'm confused by is that I have to do this additional step to propagate it to SqlToRel:

let session_state: SessionState = ...;
let mut sql_to_rel = SqlToRel::new(...);
for planner in session_state.expr_planners() {
    sql_to_rel = sql_to_rel.with_user_defined_planner(planner.clone());
}

Why do I have to do that? Why doesn't SqlToRel copy the expression planners from the ContextProvider that was passed in?

[jayzhan211]

I see. Let me see if it is possible to avoid the additional copied

[alamb]

Cross referencing -- a related discussion on #11180 #11180 (comment)