Light-City commented on code in PR #38584:
URL: https://github.com/apache/arrow/pull/38584#discussion_r1382706515


##########
cpp/src/arrow/compute/api_vector.h:
##########
@@ -177,21 +174,17 @@ class ARROW_EXPORT RankOptions : public FunctionOptions {
   };
 
   explicit RankOptions(std::vector<SortKey> sort_keys = {},
-                       NullPlacement null_placement = NullPlacement::AtEnd,
                        Tiebreaker tiebreaker = RankOptions::First);
   /// Convenience constructor for array inputs
   explicit RankOptions(SortOrder order,
-                       NullPlacement null_placement = NullPlacement::AtEnd,

Review Comment:
   The logic is to retain this signature and then pass it into SortKey, right?



##########
cpp/src/arrow/engine/substrait/serde_test.cc:
##########
@@ -5378,8 +5378,8 @@ TEST(Substrait, SortAndFetch) {
 }
 
 TEST(Substrait, MixedSort) {
-  // Substrait allows two sort keys with differing direction but Acero
-  // does not.  We should detect this and reject it.
+  // Substrait allows two sort keys with differing direction and 
+  // acero is now supported.

Review Comment:
   yes



-- 
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-unsubscr...@arrow.apache.org

For queries about this service, please contact Infrastructure at:
us...@infra.apache.org

Reply via email to