westonpace commented on code in PR #34651:
URL: https://github.com/apache/arrow/pull/34651#discussion_r1198949157


##########
cpp/src/arrow/engine/substrait/relation_internal.cc:
##########
@@ -367,6 +367,23 @@ ARROW_ENGINE_EXPORT Result<DeclarationInfo> 
MakeAggregateDeclaration(
 
 }  // namespace internal
 
+namespace {
+
+bool IsSortNullsFirst(const substrait::SortField::SortDirection& direction) {
+  return direction % 2 == 1;

Review Comment:
   We now throw an error on `CLUSTERED`:
   
   ```
         case substrait::SortField::SortDirection::
             SortField_SortDirection_SORT_DIRECTION_CLUSTERED:
         default:
           return Status::NotImplemented(
               "Acero does not support the specified sort direction: ", dir);
   ```



##########
cpp/src/arrow/engine/substrait/relation_internal.cc:
##########
@@ -757,6 +774,95 @@ Result<DeclarationInfo> FromProto(const substrait::Rel& 
rel, const ExtensionSet&
       return ProcessEmit(std::move(join), std::move(join_declaration),
                          std::move(join_schema));
     }
+    case substrait::Rel::RelTypeCase::kFetch: {
+      const auto& fetch = rel.fetch();
+      RETURN_NOT_OK(CheckRelCommon(fetch, conversion_options));
+
+      if (!fetch.has_input()) {
+        return Status::Invalid("substrait::FetchRel with no input relation");
+      }
+
+      ARROW_ASSIGN_OR_RAISE(auto input,
+                            FromProto(fetch.input(), ext_set, 
conversion_options));
+
+      int64_t offset = fetch.offset();
+      int64_t count = fetch.count();
+
+      acero::Declaration fetch_dec{
+          "fetch", {input.declaration}, acero::FetchNodeOptions(offset, 
count)};
+
+      DeclarationInfo fetch_declaration{std::move(fetch_dec), 
input.output_schema};
+      return ProcessEmit(fetch, std::move(fetch_declaration),
+                         fetch_declaration.output_schema);
+    }
+    case substrait::Rel::RelTypeCase::kSort: {
+      const auto& sort = rel.sort();
+      RETURN_NOT_OK(CheckRelCommon(sort, conversion_options));
+
+      if (!sort.has_input()) {
+        return Status::Invalid("substrait::SortRel with no input relation");
+      }
+
+      ARROW_ASSIGN_OR_RAISE(auto input,
+                            FromProto(sort.input(), ext_set, 
conversion_options));
+
+      if (sort.sorts_size() == 0) {
+        return Status::Invalid("substrait::SortRel with no sorts");
+      }
+
+      std::vector<compute::SortKey> sort_keys;

Review Comment:
   I've added the `reserve`



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

Reply via email to