westonpace commented on code in PR #14118:
URL: https://github.com/apache/arrow/pull/14118#discussion_r1048192757
##########
cpp/src/arrow/engine/substrait/relation_internal.cc:
##########
@@ -698,6 +725,16 @@ Result<std::unique_ptr<substrait::ReadRel>>
NamedTableRelationConverter(
return std::move(read_rel);
}
+std::unique_ptr<substrait::RelCommon> GetRelCommonEmit(std::vector<int>&
emit_fields) {
Review Comment:
```suggestion
std::unique_ptr<substrait::RelCommon> MakeRelCommonEmit(std::vector<int>&
emit_fields) {
```
Minor nit on the naming
##########
cpp/src/arrow/engine/substrait/relation_internal.cc:
##########
@@ -771,6 +808,42 @@ Result<std::unique_ptr<substrait::FilterRel>>
FilterRelationConverter(
return std::move(filter_rel);
}
+Result<std::unique_ptr<substrait::ProjectRel>> ProjectRelationConverter(
+ const std::shared_ptr<Schema>& schema, const compute::Declaration&
declaration,
+ ExtensionSet* ext_set, const ConversionOptions& conversion_options) {
+ auto project_rel = std::make_unique<substrait::ProjectRel>();
+ const auto& project_node_options =
+ checked_cast<const compute::ProjectNodeOptions&>(*declaration.options);
+ const auto& expressions = project_node_options.expressions;
+ if (declaration.inputs.size() == 0) {
+ return Status::Invalid("Project node doesn't have an input.");
+ }
+
+ // handling input
+ auto declr_input = declaration.inputs[0];
+ ARROW_ASSIGN_OR_RAISE(
+ auto input_rel,
+ ToProto(std::get<compute::Declaration>(declr_input), ext_set,
conversion_options));
+
+ for (const auto& expr : expressions) {
+ compute::Expression bound_expression;
+ if (!expr.IsBound()) {
+ ARROW_ASSIGN_OR_RAISE(bound_expression, expr.Bind(*schema));
+ }
+ ARROW_ASSIGN_OR_RAISE(auto subs_expr,
+ ToProto(bound_expression, ext_set,
conversion_options));
+ project_rel->mutable_expressions()->AddAllocated(subs_expr.release());
+ }
+ project_rel->set_allocated_input(input_rel.release());
+
+ // set an emit to only output the result of the expressions passed in
+ std::vector<int> emit_fields(expressions.size());
+ std::iota(emit_fields.begin(), emit_fields.end(), schema->num_fields() - 1);
Review Comment:
```suggestion
std::iota(emit_fields.begin(), emit_fields.end(), schema->num_fields() -
expressions.size());
```
I think you get away with this in your test at the moment because you only
have 1 expression. However, the output is:
IN_FIELD[0]
IN_FIELD[1]
...
IN_FIELD[X]
EXPR[0]
EXPR[1]
...
EXPR[N]
And `schema->num_fields() == X + N` so we want to start at index `X`
(`EXPR[0]`)
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
