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