feat(python): PyVelox bindings for PlanBuilder and PlanNode #12101
Conversation
facebook-github-bot
added
the
CLA Signed
|
This pull request was exported from Phabricator. Differential Revision: D68002774 |
✅ Deploy Preview for meta-velox canceled.
|
…incubator#12101) Summary: Adding Python bindings to allow plans to be built using PlanBuilder. Plan builder is still a test dependency, but will be moved out of test soon. Differential Revision: D68002774
pedroerp
force-pushed
the
export-D68002774
branch
from
69feeb7 to
043a2fa
Compare
|
This pull request was exported from Phabricator. Differential Revision: D68002774 |
|
|
||
| if (!aliases.empty()) { | ||
| for (const auto& item : aliases) { | ||
| aliasMap[item.first.cast<std::string>()] = |
There was a problem hiding this comment.
dicts can hold anything, and in python many things can be cast to string, Would it make sense to add some validation on the contents of aliases ?
For e.g what if you pass in a key or value as a double ?
|
|
||
| // If there are subfields, create the appropriate structures and add to the | ||
| // scan. | ||
| if (!subfields.empty() || !rowIndexColumnName.empty()) { |
There was a problem hiding this comment.
Similar comment about adding some documentation and validation for subfields.
| /// column name to the list of subitems to project from it. | ||
| /// @param rowIndexColumnName If defined, create an output column with that | ||
| /// name producing $row_ids. This name needs to be part of the outputSchema. | ||
| /// @param connectorId ID of the connector to use for this scan. |
There was a problem hiding this comment.
nit : would be nice to have an example invocation.
|
|
||
| py::class_<velox::py::PyPlanBuilder>(m, "PlanBuilder", py::module_local()) | ||
| .def(py::init<>()) | ||
| .def("get_plan_node", &velox::py::PyPlanBuilder::getPlanNode, py::doc(R"( |
There was a problem hiding this comment.
shouldnt we return the plan_node only after its built ? can you call get_plan_node while you are building the plan node ?
There was a problem hiding this comment.
maybe we should rename this to build()?
Summary:
Adding Python bindings to allow plans to be built using PlanBuilder.
Plan builder is still a test dependency, but will be moved out of test soon.
Differential Revision: D68002774