You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by GitBox <gi...@apache.org> on 2021/01/27 18:50:18 UTC
[GitHub] [arrow] jorgecarleitao opened a new pull request #9342: ARROW-11405: [Rust] Support multiple custom logical nodes
jorgecarleitao opened a new pull request #9342:
URL: https://github.com/apache/arrow/pull/9342
# Rational
Currently, users of the `DefaultPhysicalPlanner` can add one custom logical planner. This implies that if they use two custom nodes, they will be unable to use two custom logical planners, one per node, to plan a logical plan with both nodes.
# This PR
This PR is built on top of #9333 and allows users to use more than one custom planner. The idea here is that by doing so, we enable all kinds of customization on which people can share custom nodes and respective custom planner (e.g. on a separate crate), and consumers can simply use them together with other custom nodes.
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] alamb commented on a change in pull request #9342: ARROW-11405: [DataFusion] Support multiple custom logical nodes
Posted by GitBox <gi...@apache.org>.
alamb commented on a change in pull request #9342:
URL: https://github.com/apache/arrow/pull/9342#discussion_r565655349
##########
File path: rust/datafusion/src/physical_plan/planner.rs
##########
@@ -50,29 +50,30 @@ use arrow::compute::SortOptions;
use arrow::datatypes::{Schema, SchemaRef};
use expressions::col;
-/// This trait permits the `DefaultPhysicalPlanner` to create plans for
-/// user defined `ExtensionPlanNode`s
+/// This trait exposes the ability to plan an [`ExecutionPlan`] out of a [`LogicalPlan`].
pub trait ExtensionPlanner {
- /// Create a physical plan for an extension node
+ /// Create a physical plan for a [`UserDefinedLogicalNode`].
+ /// This errors when the planner knows how to plan the concrete implementation of `node`
+ /// but errors while doing so, and `None` when the planner does not know how to plan the `node`
+ /// and wants to delegate the planning to another [`ExtensionPlanner`].
Review comment:
This is a nice interface
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] alamb commented on pull request #9342: ARROW-11405: [DataFusion] Support multiple custom logical nodes
Posted by GitBox <gi...@apache.org>.
alamb commented on pull request #9342:
URL: https://github.com/apache/arrow/pull/9342#issuecomment-768598082
This PR and https://github.com/apache/arrow/pull/9333 will require changes to IOx but I think those changes will be for the better
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] github-actions[bot] commented on pull request #9342: ARROW-11405: [DataFusion] Support multiple custom logical nodes
Posted by GitBox <gi...@apache.org>.
github-actions[bot] commented on pull request #9342:
URL: https://github.com/apache/arrow/pull/9342#issuecomment-768547794
https://issues.apache.org/jira/browse/ARROW-11405
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] jorgecarleitao closed pull request #9342: ARROW-11405: [DataFusion] Support multiple custom logical nodes
Posted by GitBox <gi...@apache.org>.
jorgecarleitao closed pull request #9342:
URL: https://github.com/apache/arrow/pull/9342
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] jorgecarleitao commented on pull request #9342: ARROW-11405: [Rust] Support multiple custom logical nodes
Posted by GitBox <gi...@apache.org>.
jorgecarleitao commented on pull request #9342:
URL: https://github.com/apache/arrow/pull/9342#issuecomment-768497211
@alamb , we discussed this some months ago, when we introduced the extension node. Here is the PR :)
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [arrow] andygrove commented on pull request #9342: ARROW-11405: [DataFusion] Support multiple custom logical nodes
Posted by GitBox <gi...@apache.org>.
andygrove commented on pull request #9342:
URL: https://github.com/apache/arrow/pull/9342#issuecomment-769961034
I will review this PR over the weekend
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
users@infra.apache.org