You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@arrow.apache.org by "alamb (via GitHub)" <gi...@apache.org> on 2023/06/06 20:48:53 UTC

[GitHub] [arrow-datafusion] alamb opened a new pull request, #6570: Move PhysicalPlanner to `physical_planer` module

alamb opened a new pull request, #6570:
URL: https://github.com/apache/arrow-datafusion/pull/6570

   # Which issue does this PR close?
   
   Part of https://github.com/apache/arrow-datafusion/issues/1754 (see https://github.com/apache/arrow-datafusion/issues/1754#issuecomment-1579410979)
   
   # Rationale for this change
   
   I am trying to extract `physical_plan` into its own crate to improve modularity and compile times
   
   I am also trying to make sure the organization of the DataFusion crate is easier to understand for beginners. 
   
   # What changes are included in this PR?
   Move `PhysicalPlanner` and related structures to `physical_planner` crate
   
   # Are these changes tested?
   
   Existing tests
   
   # Are there any user-facing changes?
   Yes, documentation is improved (the planner is no longer buried in a sub module). Also, if users used the PhysicalPlanner directly they will need to update their `use` path
   
   <!--
   If there are any breaking changes to public APIs, please add the `api change` label.
   -->


-- 
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

[GitHub] [arrow-datafusion] alamb commented on pull request #6570: Move `PhysicalPlanner` to `physical_planer` module

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #6570:
URL: https://github.com/apache/arrow-datafusion/pull/6570#issuecomment-1593711517

   > So physical_planner will be into the same crate as physical_plan? Or a different crate?
   
   I think initially it will be in the `datafusion` (datafusion/core) crate (where it is now). 
   
   We could potentially move it to its own crate in the future too (though given it is not that much code, I don't know how valuable that would be)
   
   The issue is that the planner instantiates a bunch of things defined in `datasource`;
   
   The key issue in extracting `physical_plan` from `datafusion/core`  is that `datasource` is ALSO in `datafusion/core` and I can't easily take it out (b/c `TableProvider` depends directly on `SessionState`). 
   
   Since the physical planner depends on `datasource` it has to stay in `datafusion/core` too, at least initailly. 
   
   Once I have `physical_plan` and `datasource` detangled (so `datasource` depends on `physical_plan` but not the other way round) I can finally pull out `physical_plan` 😅  -- at least that is the hope
   
   


-- 
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

[GitHub] [arrow-datafusion] alamb merged pull request #6570: Move `PhysicalPlanner` to `physical_planer` module

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb merged PR #6570:
URL: https://github.com/apache/arrow-datafusion/pull/6570


-- 
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

[GitHub] [arrow-datafusion] alamb commented on pull request #6570: Move `PhysicalPlanner` to `physical_planer` module

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on PR #6570:
URL: https://github.com/apache/arrow-datafusion/pull/6570#issuecomment-1592006277

   > How to improve the compile times ? increase the degree of parallelism 🤔
   
   Yes that is basically the plan


-- 
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

[GitHub] [arrow-datafusion] viirya commented on pull request #6570: Move `PhysicalPlanner` to `physical_planer` module

Posted by "viirya (via GitHub)" <gi...@apache.org>.
viirya commented on PR #6570:
URL: https://github.com/apache/arrow-datafusion/pull/6570#issuecomment-1593696854

   > I am trying to extract physical_plan into its own crate to improve modularity and compile times
   
   So `physical_planner` will be into the same crate as `physical_plan`?


-- 
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