feat(python): Support for MergeSort #12462
Conversation
facebook-github-bot
added
the
CLA Signed
|
This pull request was exported from Phabricator. Differential Revision: D70294152 |
✅ Deploy Preview for meta-velox canceled.
|
Summary: Adding support for mege sort in PyVelox plan builder API. Differential Revision: D70294152
pedroerp
force-pushed
the
export-D70294152
branch
from
f508182 to
4acb1cf
Compare
|
This pull request was exported from Phabricator. Differential Revision: D70294152 |
| @@ -282,6 +282,22 @@ PyPlanBuilder& PyPlanBuilder::mergeJoin( | |||
| return *this; | |||
| } | |||
|
|
|||
| PyPlanBuilder& PyPlanBuilder::mergeSort( | |||
There was a problem hiding this comment.
nit: IMO this name is a bit confusing because the data is already sorted and we are just merging them. we aren't doing any sorting per-se. Something like sortedMerge would be more descriptive. I'll leave it up to you though.
There was a problem hiding this comment.
That's a good point. I'll rename it.
| sources.reserve(pySources.size()); | ||
|
|
||
| for (const auto& pySource : pySources) { | ||
| if (pySource.has_value()) { |
There was a problem hiding this comment.
In what situation would anyone pass in an empty PyPlanNode? Wondering why this is a vector of optionals.
There was a problem hiding this comment.
That is because plan_builder.get_plan_node() returns an optional. It could be done if the plan builder is empty (if no nodes were added yet).
| @@ -213,6 +213,15 @@ class PyPlanBuilder { | |||
| const std::string& filter, | |||
| core::JoinType joinType); | |||
|
|
|||
| /// Takes N sorted `source` subtrees and merges them into a sorted output. | |||
| /// Assumes that all sources are sorted in `keys`. | |||
There was a problem hiding this comment.
| /// Assumes that all sources are sorted in `keys`. | |
| /// Assumes that all sources are sorted on `keys`. |
| py::arg("sources"), | ||
| py::doc(R"( | ||
| Takes N sorted `source` subtrees and merges them into a sorted output. | ||
| Assumes that all sources are sorted in `keys`. |
There was a problem hiding this comment.
| Assumes that all sources are sorted in `keys`. | |
| Assumes that all sources are sorted on `keys`. |
pedroerp
force-pushed
the
export-D70294152
branch
from
4acb1cf to
60169c3
Compare
Summary: Adding support for mege sort in PyVelox plan builder API. Reviewed By: kostasxmeta Differential Revision: D70294152
Summary: Adding support for mege sort in PyVelox plan builder API. Reviewed By: kostasxmeta Differential Revision: D70294152
pedroerp
force-pushed
the
export-D70294152
branch
from
60169c3 to
82a390c
Compare
|
This pull request was exported from Phabricator. Differential Revision: D70294152 |
Summary: Pull Request resolved: facebookincubator#12462 Adding support for mege sort in PyVelox plan builder API. Reviewed By: kostasxmeta Differential Revision: D70294152
pedroerp
force-pushed
the
export-D70294152
branch
from
82a390c to
d6a7475
Compare
|
This pull request was exported from Phabricator. Differential Revision: D70294152 |
|
This pull request has been merged in e065010. |
Summary: Adding support for mege sort in PyVelox plan builder API.
Differential Revision: D70294152