You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "wjones127 (via GitHub)" <gi...@apache.org> on 2024/07/15 19:40:47 UTC

[I] ExprPlanner not propagated to SqlToRel [datafusion]

wjones127 opened a new issue, #11477:
URL: https://github.com/apache/datafusion/issues/11477

   ### 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.
   
   https://github.com/apache/datafusion/blob/f204869ff55bb3e39cf23fc0a34ebd5021e6773f/datafusion/sql/src/planner.rs#L206-L213
   
   ### 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());
   }
   ```


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [I] ExprPlanner not propagated to SqlToRel [datafusion]

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on issue #11477:
URL: https://github.com/apache/datafusion/issues/11477#issuecomment-2231851660

   Cross referencing -- a related discussion on https://github.com/apache/datafusion/pull/11180 https://github.com/apache/datafusion/pull/11180#discussion_r1667729599


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [I] ExprPlanner not propagated to SqlToRel [datafusion]

Posted by "jayzhan211 (via GitHub)" <gi...@apache.org>.
jayzhan211 commented on issue #11477:
URL: https://github.com/apache/datafusion/issues/11477#issuecomment-2229713743

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


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [I] ExprPlanner not propagated to SqlToRel [datafusion]

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb closed issue #11477: ExprPlanner not propagated to SqlToRel
URL: https://github.com/apache/datafusion/issues/11477


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [I] ExprPlanner not propagated to SqlToRel [datafusion]

Posted by "wjones127 (via GitHub)" <gi...@apache.org>.
wjones127 commented on issue #11477:
URL: https://github.com/apache/datafusion/issues/11477#issuecomment-2229673349

   > 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`:
   
   ```rust
   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?


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [I] ExprPlanner not propagated to SqlToRel [datafusion]

Posted by "jayzhan211 (via GitHub)" <gi...@apache.org>.
jayzhan211 commented on issue #11477:
URL: https://github.com/apache/datafusion/issues/11477#issuecomment-2229631902

   After https://github.com/apache/datafusion/pull/11403, I think you can easily get the default planners with
   
   ```rust
   SessionStateBuilder::new()
               .with_default_features()
               .build()
   ```


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [I] ExprPlanner not propagated to SqlToRel [datafusion]

Posted by "jayzhan211 (via GitHub)" <gi...@apache.org>.
jayzhan211 commented on issue #11477:
URL: https://github.com/apache/datafusion/issues/11477#issuecomment-2229629221

   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.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [I] ExprPlanner not propagated to SqlToRel [datafusion]

Posted by "wjones127 (via GitHub)" <gi...@apache.org>.
wjones127 commented on issue #11477:
URL: https://github.com/apache/datafusion/issues/11477#issuecomment-2229260251

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


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org


Re: [I] ExprPlanner not propagated to SqlToRel [datafusion]

Posted by "jayzhan211 (via GitHub)" <gi...@apache.org>.
jayzhan211 commented on issue #11477:
URL: https://github.com/apache/datafusion/issues/11477#issuecomment-2229701907

   I see. You are right, we can get the planners without additional step.


-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org


---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org