You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "amartins23 (via GitHub)" <gi...@apache.org> on 2023/10/16 14:01:56 UTC
[I] Lower &mut SessionContext to &SessionContext in substrait consumer/producer [arrow-datafusion]
amartins23 opened a new issue, #7836:
URL: https://github.com/apache/arrow-datafusion/issues/7836
### Is your feature request related to a problem or challenge?
Currently functions to convert from substrait to DataFusion plans, or to produce a substrait plan from a DataFusion logical plan, require a mutable reference to SessionContext. However, none of the implementations seems to require that. Since state access in the session context does not require a mutable pointer, I don't see the need to take a mutable reference to the session context.
Allowing shared references to SessionContext to be used would simplify consuming code and should not be a breaking as you can pass an &mut reference to a function expecting a shared reference.
### Describe the solution you'd like
Change the various conversion functions in the datafusion-substrait crate to receive &SessionContext instead of &mut SessionContext.
### 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@arrow.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Lower &mut SessionContext to &SessionContext in substrait consumer/producer [arrow-datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on issue #7836:
URL: https://github.com/apache/arrow-datafusion/issues/7836#issuecomment-1784076894
Fixed in https://github.com/apache/arrow-datafusion/pull/7965
--
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@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Lower &mut SessionContext to &SessionContext in substrait consumer/producer [arrow-datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb closed issue #7836: Lower &mut SessionContext to &SessionContext in substrait consumer/producer
URL: https://github.com/apache/arrow-datafusion/issues/7836
--
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@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Lower &mut SessionContext to &SessionContext in substrait consumer/producer [arrow-datafusion]
Posted by "my-vegetable-has-exploded (via GitHub)" <gi...@apache.org>.
my-vegetable-has-exploded commented on issue #7836:
URL: https://github.com/apache/arrow-datafusion/issues/7836#issuecomment-1767450812
I'd like to have a try.
--
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@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Lower &mut SessionContext to &SessionContext in substrait consumer/producer [arrow-datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on issue #7836:
URL: https://github.com/apache/arrow-datafusion/issues/7836#issuecomment-1784076420
> Sorry for the delay in completing this task.
No worries -- thank you for doing so!
--
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@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Lower &mut SessionContext to &SessionContext in substrait consumer/producer [arrow-datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on issue #7836:
URL: https://github.com/apache/arrow-datafusion/issues/7836#issuecomment-1784076850
Fixed in ull/7965
--
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@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Lower &mut SessionContext to &SessionContext in substrait consumer/producer [arrow-datafusion]
Posted by "my-vegetable-has-exploded (via GitHub)" <gi...@apache.org>.
my-vegetable-has-exploded commented on issue #7836:
URL: https://github.com/apache/arrow-datafusion/issues/7836#issuecomment-1783809237
> 2\. Ideally add some doc comments / examples of how to do this conversion
Sorry for the delay in completing this task. Would an example like [this](https://medium.com/@omri-levy/one-query-plan-three-different-engines-e5dc74aeb52f) be OK?
--
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@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [I] Lower &mut SessionContext to &SessionContext in substrait consumer/producer [arrow-datafusion]
Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on issue #7836:
URL: https://github.com/apache/arrow-datafusion/issues/7836#issuecomment-1767422838
Hi @amartins23
It seems like the consumer functions such as
https://docs.rs/datafusion-substrait/latest/datafusion_substrait/logical_plan/consumer/fn.from_substrait_plan.html
https://docs.rs/datafusion-substrait/latest/datafusion_substrait/physical_plan/consumer/fn.from_substrait_rel.html
Require mutable access, but the producers do not (they already take `&SessionContext`, not `&mut SessionContext`)
https://docs.rs/datafusion-substrait/latest/datafusion_substrait/logical_plan/producer/fn.to_substrait_plan.html
This I think this ticket would require:
1. Converting from &mut SessionContext` to &SessionContext`
2. Ideally add some doc comments / examples of how to do this conversion
This seems straightforward and a good first project so marking it as such
Note there may be some subtlety I don't understand that would make this not feasible.
--
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@arrow.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org