You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "LorrensP-2158466 (via GitHub)" <gi...@apache.org> on 2024/05/18 13:05:41 UTC
[I] UserDefindedLogicalNode::from_template does not return a Result<...>. [datafusion]
LorrensP-2158466 opened a new issue, #10571:
URL: https://github.com/apache/datafusion/issues/10571
### Is your feature request related to a problem or challenge?
This is really a feature request but more of a question.
Currently `UserDefinedLogicalNode::from_template` only returns `Arc<dyn Self>` but i have noticed that `LogicalPlan::with_new_exprs` does return a Result. So when it calls `from_template` on an Extension node, it just wraps it in an `Ok()`.
In the example of `UserDefinedLogicalNodeCore` where TopK is implemented, the implementation of `from_template` first asserts the inputs and exprs.
I have also tried to implement a custom LogicalPlan node and my own implementation of `from_template` also has several error cases (not just asserting the new values) which i unfortunately have to unwrap.
Is this expected behaviour, or is it maybe nicer to return `Result`s
### Describe the solution you'd like
_No response_
### Describe alternatives you've considered
_No response_
### Additional context
_No response_
--
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] UserDefindedLogicalNode::from_template does not return a Result<...>. [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on issue #10571:
URL: https://github.com/apache/datafusion/issues/10571#issuecomment-2119371269
I agree it would be much better to change the API and return an `Result<..>`
Thank you for the report
--
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] UserDefindedLogicalNode::from_template does not return a Result<...>. [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on issue #10571:
URL: https://github.com/apache/datafusion/issues/10571#issuecomment-2120463462
> I don't see this as a really difficult API change. Is it ok if I do this? Edit: there is a PR already,did not see it, sorry.
@lewiszlw beats us to it!
(BTW if anyone wants a workaround for this in older versions that does not requier a panic, they could potentially return a node that would generate a runtime error if the arguments to `from_template` didn't error)
--
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] UserDefindedLogicalNode::from_template does not return a Result<...>. [datafusion]
Posted by "LorrensP-2158466 (via GitHub)" <gi...@apache.org>.
LorrensP-2158466 commented on issue #10571:
URL: https://github.com/apache/datafusion/issues/10571#issuecomment-2119941766
I don't see this as a really difficult API change. Is it ok if I do 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] UserDefindedLogicalNode::from_template does not return a Result<...>. [datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb closed issue #10571: UserDefindedLogicalNode::from_template does not return a Result<...>.
URL: https://github.com/apache/datafusion/issues/10571
--
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