From 2882cb4fac72b2d1217e09ad51eba62043e54b2f Mon Sep 17 00:00:00 2001
From: Alessandro Molina <amol@turbogears.org>
Date: Wed, 10 Nov 2021 13:04:25 +0100
Subject: [PATCH 01/24] ARROW-14656: [Python] sort_by helper for StructArray

---
 python/pyarrow/array.pxi           | 19 +++++++++++++++++++
 python/pyarrow/tests/test_array.py | 14 ++++++++++++++
 2 files changed, 33 insertions(+)

diff --git a/python/pyarrow/array.pxi b/python/pyarrow/array.pxi
index de0d3a74dfb..85dde338419 100644
--- a/python/pyarrow/array.pxi
+++ b/python/pyarrow/array.pxi
@@ -2370,6 +2370,25 @@ cdef class StructArray(Array):
         result.validate()
         return result
 
+    def sort_by(self, fieldname, order="ascending"):
+        """
+        Sort the StructArray by one of its fields.
+
+        Parameters
+        ----------
+        fieldname : str
+            The name of the field use to sort the StructArray.
+        order : "ascending" or "descending"
+            The order of the sorting.
+
+        Returns
+        -------
+        result : StructArray
+        """
+        field = self.field(fieldname)
+        indices = _pc().array_sort_indices(field, order=order)
+        return self.take(indices)
+
 
 cdef class ExtensionArray(Array):
     """
diff --git a/python/pyarrow/tests/test_array.py b/python/pyarrow/tests/test_array.py
index 6b87a0ba9b0..c4b5c8fb389 100644
--- a/python/pyarrow/tests/test_array.py
+++ b/python/pyarrow/tests/test_array.py
@@ -724,6 +724,20 @@ def test_struct_array_from_chunked():
         pa.StructArray.from_arrays([chunked_arr], ["foo"])
 
 
+def test_struct_array_sort():
+    arr = pa.StructArray.from_arrays([
+        pa.array([5, 7, 35], type=pa.int64()),
+        pa.array(["foo", "bar", "foobar"])
+    ], names=["a", "b"])
+
+    sorted_arr = arr.sort_by("a", "descending")
+    assert sorted_arr.to_pylist() == [
+        {"a": 35, "b": "foobar"},
+        {"a": 7, "b": "bar"},
+        {"a": 5, "b": "foo"},
+    ]
+
+
 def test_dictionary_from_numpy():
     indices = np.repeat([0, 1, 2], 2)
     dictionary = np.array(['foo', 'bar', 'baz'], dtype=object)

From 291c078c01652914dbd6fd91386d91a0ffece392 Mon Sep 17 00:00:00 2001
From: Alessandro Molina <amol@turbogears.org>
Date: Fri, 12 Nov 2021 17:18:11 +0100
Subject: [PATCH 02/24] Incomplete steps forward

---
 .../compute/kernels/vector_array_sort.cc      |  8 +++++
 python/pyarrow/array.pxi                      | 33 +++++++++++++++----
 python/pyarrow/table.pxi                      | 16 +++++++++
 python/pyarrow/tests/test_array.py            | 19 ++++++++++-
 4 files changed, 69 insertions(+), 7 deletions(-)

diff --git a/cpp/src/arrow/compute/kernels/vector_array_sort.cc b/cpp/src/arrow/compute/kernels/vector_array_sort.cc
index 6d809102769..8ee2a86ce7e 100644
--- a/cpp/src/arrow/compute/kernels/vector_array_sort.cc
+++ b/cpp/src/arrow/compute/kernels/vector_array_sort.cc
@@ -502,6 +502,13 @@ const ArraySortOptions* GetDefaultArraySortOptions() {
   return &kDefaultArraySortOptions;
 }
 
+template <template <typename...> class ExecTemplate>
+void AddArraySortingStructKernels(VectorKernel base, VectorFunction* func) {
+  base.signature = KernelSignature::Make({InputType::Array(Type::STRUCT)}, uint64());
+  base.exec = ExecTemplate<UInt64Type, StructType>::Exec;
+  DCHECK_OK(func->AddKernel(base));
+}
+
 const FunctionDoc array_sort_indices_doc(
     "Return the indices that would sort an array",
     ("This function computes an array of indices that define a stable sort\n"
@@ -549,6 +556,7 @@ void RegisterVectorArraySort(FunctionRegistry* registry) {
       GetDefaultArraySortOptions());
   base.init = ArraySortIndicesState::Init;
   AddArraySortingKernels<ArraySortIndices>(base, array_sort_indices.get());
+  AddArraySortingStructKernels<ArraySortIndices>(base, array_sort_indices.get());
   DCHECK_OK(registry->AddFunction(std::move(array_sort_indices)));
 
   // partition_nth_indices has a parameter so needs its init function
diff --git a/python/pyarrow/array.pxi b/python/pyarrow/array.pxi
index 85dde338419..a512c87225a 100644
--- a/python/pyarrow/array.pxi
+++ b/python/pyarrow/array.pxi
@@ -1181,6 +1181,22 @@ cdef class Array(_PandasConvertible):
         """
         return _pc().index(self, value, start, end, memory_pool=memory_pool)
 
+    def sort(self, order="ascending"):
+        """
+        Sort the Array
+
+        Parameters
+        ----------
+        order : "ascending" or "descending"
+            The order of the sorting.
+
+        Returns
+        -------
+        result : Array
+        """
+        indices = _pc().array_sort_indices(self, order=order)
+        return self.take(indices)
+
     def _to_pandas(self, options, **kwargs):
         return _array_like_to_pandas(self, options)
 
@@ -2370,23 +2386,28 @@ cdef class StructArray(Array):
         result.validate()
         return result
 
-    def sort_by(self, fieldname, order="ascending"):
+    def sort(self, order="ascending", fieldname=None):
         """
-        Sort the StructArray by one of its fields.
+        Sort the StructArray
 
         Parameters
         ----------
-        fieldname : str
-            The name of the field use to sort the StructArray.
         order : "ascending" or "descending"
             The order of the sorting.
+        fieldname : str or None, default None
+            If to sort the array by one of its fields
+            or by the whole array.
 
         Returns
         -------
         result : StructArray
         """
-        field = self.field(fieldname)
-        indices = _pc().array_sort_indices(field, order=order)
+        if fieldname is not None:
+            tosort = self.field(fieldname)
+        else:
+            # Requires a dedicated Kernel
+            raise NotImplementedError
+        indices = _pc().array_sort_indices(tosort, order=order)
         return self.take(indices)
 
 
diff --git a/python/pyarrow/table.pxi b/python/pyarrow/table.pxi
index 5f42d71c7e3..832d4b83ef0 100644
--- a/python/pyarrow/table.pxi
+++ b/python/pyarrow/table.pxi
@@ -431,6 +431,22 @@ cdef class ChunkedArray(_PandasConvertible):
         """
         return _pc().drop_null(self)
 
+    def sort(self, order="ascending"):
+        """
+        Sort the ChunkedArray
+
+        Parameters
+        ----------
+        order : "ascending" or "descending"
+            The order of the sorting.
+
+        Returns
+        -------
+        result : ChunkedArray
+        """
+        indices = _pc().array_sort_indices(self, order=order)
+        return self.take(indices)
+
     def unify_dictionaries(self, MemoryPool memory_pool=None):
         """
         Unify dictionaries across all chunks.
diff --git a/python/pyarrow/tests/test_array.py b/python/pyarrow/tests/test_array.py
index c4b5c8fb389..5b264e146ae 100644
--- a/python/pyarrow/tests/test_array.py
+++ b/python/pyarrow/tests/test_array.py
@@ -724,13 +724,30 @@ def test_struct_array_from_chunked():
         pa.StructArray.from_arrays([chunked_arr], ["foo"])
 
 
+def test_array_sort():
+    arr = pa.array([5, 7, 35], type=pa.int64())
+    sorted_arr = arr.sort("descending")
+    assert sorted_arr.to_pylist() == [35, 7, 5]
+
+    arr = pa.chunked_array([[1, 2, 3], [4, 5, 6]])
+    sorted_arr = arr.sort("descending")
+    assert sorted_arr.to_pylist() == [6, 5, 4, 3, 2, 1]
+
+
 def test_struct_array_sort():
     arr = pa.StructArray.from_arrays([
         pa.array([5, 7, 35], type=pa.int64()),
         pa.array(["foo", "bar", "foobar"])
     ], names=["a", "b"])
 
-    sorted_arr = arr.sort_by("a", "descending")
+    sorted_arr = arr.sort("descending", fieldname="a")
+    assert sorted_arr.to_pylist() == [
+        {"a": 35, "b": "foobar"},
+        {"a": 7, "b": "bar"},
+        {"a": 5, "b": "foo"},
+    ]
+
+    sorted_arr = arr.sort("descending")
     assert sorted_arr.to_pylist() == [
         {"a": 35, "b": "foobar"},
         {"a": 7, "b": "bar"},

From bc493ef3ee7a65b0c7395fa00a27e02c4abd4b40 Mon Sep 17 00:00:00 2001
From: Alessandro Molina <amol@turbogears.org>
Date: Mon, 22 Nov 2021 13:27:23 +0100
Subject: [PATCH 03/24] use sort_indices for ChunkedArray

---
 python/pyarrow/table.pxi | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/python/pyarrow/table.pxi b/python/pyarrow/table.pxi
index 832d4b83ef0..740975a4ec7 100644
--- a/python/pyarrow/table.pxi
+++ b/python/pyarrow/table.pxi
@@ -444,7 +444,7 @@ cdef class ChunkedArray(_PandasConvertible):
         -------
         result : ChunkedArray
         """
-        indices = _pc().array_sort_indices(self, order=order)
+        indices = _pc().sort_indices(self, sort_keys=[("", order)])
         return self.take(indices)
 
     def unify_dictionaries(self, MemoryPool memory_pool=None):

From 5c6280bf8df6dddab48099bd1e3829854aa92a9f Mon Sep 17 00:00:00 2001
From: Alessandro Molina <amol@turbogears.org>
Date: Mon, 22 Nov 2021 18:35:33 +0100
Subject: [PATCH 04/24] Proof of concept for StructArray

---
 .../compute/kernels/vector_array_sort.cc      | 119 +++++++++++++++++-
 python/pyarrow/array.pxi                      |   7 +-
 python/pyarrow/tests/test_array.py            |  13 +-
 3 files changed, 131 insertions(+), 8 deletions(-)

diff --git a/cpp/src/arrow/compute/kernels/vector_array_sort.cc b/cpp/src/arrow/compute/kernels/vector_array_sort.cc
index 8ee2a86ce7e..183fa5bf77c 100644
--- a/cpp/src/arrow/compute/kernels/vector_array_sort.cc
+++ b/cpp/src/arrow/compute/kernels/vector_array_sort.cc
@@ -172,6 +172,116 @@ class ArrayCompareSorter {
   }
 };
 
+template <typename ArrowType>
+class StructArrayCompareSorter {
+  using ArrayType = typename TypeTraits<ArrowType>::ArrayType;
+
+ public:
+  // `offset` is used when this is called on a chunk of a chunked array
+  NullPartitionResult operator()(uint64_t* indices_begin, uint64_t* indices_end,
+                                 const Array& array, int64_t offset,
+                                 const ArraySortOptions& options) {
+    const auto& values = checked_cast<const ArrayType&>(array);
+    ArrayVector fields = values.fields();
+
+    for(auto field = fields.begin(); field != fields.end(); field++) {
+      std::shared_ptr<DataType> physical_type = GetPhysicalType((*field)->type());
+      if(MakeFieldComparator(*physical_type) != Status::OK())
+        // TODO: Improve error handling
+        return NullPartitionResult();
+      field_comparators_.push_back(current_field_comparator_);
+    }
+
+    const auto p = PartitionNulls<ArrayType, StablePartitioner>(
+        indices_begin, indices_end, values, offset, options.null_placement);
+    if (options.order == SortOrder::Ascending) {
+      std::stable_sort(
+          p.non_nulls_begin, p.non_nulls_end,
+          [&offset, &fields, this](uint64_t left, uint64_t right) {
+            for(ArrayVector::size_type fieldidx = 0; fieldidx < fields.size(); ++fieldidx) {
+              auto field = fields[fieldidx];
+              auto field_comparator = field_comparators_[fieldidx];
+              int result = field_comparator->Compare(*field, offset, left, right);
+              if(result == -1)
+                return true;
+              else if (result == 1)
+                return false;
+            }
+            return false;
+          });
+    }
+    else {
+      std::stable_sort(
+          p.non_nulls_begin, p.non_nulls_end,
+          [&offset, &fields, this](uint64_t left, uint64_t right) {        
+            for(ArrayVector::size_type fieldidx = 0; fieldidx < fields.size(); ++fieldidx) {
+              auto field = fields[fieldidx];
+              auto field_comparator = field_comparators_[fieldidx];
+              int result = field_comparator->Compare(*field, offset, right, left);
+              if(result == -1)
+                return true;
+              else if (result == 1)
+                return false;
+            }
+            return false;
+          });
+    }
+    return p;
+  }
+
+  Status MakeFieldComparator(DataType const &physical_type) {
+    RETURN_NOT_OK(VisitTypeInline(physical_type, this));
+    DCHECK_NE(current_field_comparator_, nullptr);
+    return Status::OK();
+  }
+
+#define VISIT(TYPE) \
+  Status Visit(const TYPE& type) { return VisitGeneric(type); }
+
+    VISIT_SORTABLE_PHYSICAL_TYPES(VISIT)
+
+#undef VISIT
+
+    Status Visit(const DataType& type) {
+      return Status::TypeError("Unsupported type for StructArray sorting: ",
+                               type.ToString());
+    }
+
+    template <typename Type>
+    Status VisitGeneric(const Type&) {
+      current_field_comparator_ = std::shared_ptr<FieldValueComparator>(new ConcreteFieldValueComparator<Type>());
+      return Status::OK();
+    }
+  
+  struct FieldValueComparator {
+    virtual int Compare(Array const &array, uint64_t offset, 
+                        uint64_t leftidx, uint64_t rightidx) = 0;
+    virtual ~FieldValueComparator() = default;
+  };
+
+  template <typename Type>
+  struct ConcreteFieldValueComparator : FieldValueComparator {
+    using FieldArrayType = typename TypeTraits<Type>::ArrayType;
+    using GetView = GetViewType<Type>;
+
+    virtual int Compare(Array const &array, uint64_t offset, 
+                        uint64_t leftidx, uint64_t rightidx) {
+      const FieldArrayType& values = checked_cast<const FieldArrayType&>(array);
+      auto left_value = GetView::LogicalValue(values.GetView(leftidx - offset));
+      auto right_value = GetView::LogicalValue(values.GetView(rightidx - offset));
+      if (left_value == right_value)
+        return 0;
+      else if (left_value < right_value)
+        return -1;
+      else
+        return 1;
+    }
+  };
+
+  std::shared_ptr<FieldValueComparator> current_field_comparator_;
+  std::vector<std::shared_ptr<FieldValueComparator>> field_comparators_;
+};
+
 template <typename ArrowType>
 class ArrayCountSorter {
   using ArrayType = typename TypeTraits<ArrowType>::ArrayType;
@@ -410,6 +520,11 @@ struct ArraySorter<
   ArrayCompareSorter<Type> impl;
 };
 
+template <typename Type>
+struct ArraySorter<Type, enable_if_t<is_struct_type<Type>::value>> {
+  StructArrayCompareSorter<Type> impl;
+};
+
 struct ArraySorterFactory {
   ArraySortFunc sorter;
 
@@ -503,7 +618,7 @@ const ArraySortOptions* GetDefaultArraySortOptions() {
 }
 
 template <template <typename...> class ExecTemplate>
-void AddArraySortingStructKernels(VectorKernel base, VectorFunction* func) {
+void AddArraySortingNestedKernels(VectorKernel base, VectorFunction* func) {
   base.signature = KernelSignature::Make({InputType::Array(Type::STRUCT)}, uint64());
   base.exec = ExecTemplate<UInt64Type, StructType>::Exec;
   DCHECK_OK(func->AddKernel(base));
@@ -556,7 +671,7 @@ void RegisterVectorArraySort(FunctionRegistry* registry) {
       GetDefaultArraySortOptions());
   base.init = ArraySortIndicesState::Init;
   AddArraySortingKernels<ArraySortIndices>(base, array_sort_indices.get());
-  AddArraySortingStructKernels<ArraySortIndices>(base, array_sort_indices.get());
+  AddArraySortingNestedKernels<ArraySortIndices>(base, array_sort_indices.get());
   DCHECK_OK(registry->AddFunction(std::move(array_sort_indices)));
 
   // partition_nth_indices has a parameter so needs its init function
diff --git a/python/pyarrow/array.pxi b/python/pyarrow/array.pxi
index a512c87225a..3ed9ffb2fef 100644
--- a/python/pyarrow/array.pxi
+++ b/python/pyarrow/array.pxi
@@ -1194,7 +1194,7 @@ cdef class Array(_PandasConvertible):
         -------
         result : Array
         """
-        indices = _pc().array_sort_indices(self, order=order)
+        indices = _pc().sort_indices(self, sort_keys=[("", order)])
         return self.take(indices)
 
     def _to_pandas(self, options, **kwargs):
@@ -2405,9 +2405,8 @@ cdef class StructArray(Array):
         if fieldname is not None:
             tosort = self.field(fieldname)
         else:
-            # Requires a dedicated Kernel
-            raise NotImplementedError
-        indices = _pc().array_sort_indices(tosort, order=order)
+            tosort = self
+        indices = _pc().sort_indices(tosort, sort_keys=[("", order)])
         return self.take(indices)
 
 
diff --git a/python/pyarrow/tests/test_array.py b/python/pyarrow/tests/test_array.py
index 5b264e146ae..42fac5b24cf 100644
--- a/python/pyarrow/tests/test_array.py
+++ b/python/pyarrow/tests/test_array.py
@@ -736,13 +736,14 @@ def test_array_sort():
 
 def test_struct_array_sort():
     arr = pa.StructArray.from_arrays([
-        pa.array([5, 7, 35], type=pa.int64()),
-        pa.array(["foo", "bar", "foobar"])
+        pa.array([5, 7, 7, 35], type=pa.int64()),
+        pa.array(["foo", "car", "bar", "foobar"])
     ], names=["a", "b"])
 
     sorted_arr = arr.sort("descending", fieldname="a")
     assert sorted_arr.to_pylist() == [
         {"a": 35, "b": "foobar"},
+        {"a": 7, "b": "car"},
         {"a": 7, "b": "bar"},
         {"a": 5, "b": "foo"},
     ]
@@ -750,10 +751,18 @@ def test_struct_array_sort():
     sorted_arr = arr.sort("descending")
     assert sorted_arr.to_pylist() == [
         {"a": 35, "b": "foobar"},
+        {"a": 7, "b": "car"},
         {"a": 7, "b": "bar"},
         {"a": 5, "b": "foo"},
     ]
 
+    sorted_arr = arr.sort("ascending")
+    assert sorted_arr.to_pylist() == [
+        {"a": 5, "b": "foo"},
+        {"a": 7, "b": "bar"},
+        {"a": 7, "b": "car"},
+        {"a": 35, "b": "foobar"},
+    ]
 
 def test_dictionary_from_numpy():
     indices = np.repeat([0, 1, 2], 2)

From f5148d0759f3c43ec397c516879a53853b67b716 Mon Sep 17 00:00:00 2001
From: Alessandro Molina <amol@turbogears.org>
Date: Tue, 30 Nov 2021 11:12:06 +0100
Subject: [PATCH 05/24] in progress

---
 cpp/src/arrow/compute/kernels/vector_sort.cc  |  1 +
 .../compute/kernels/vector_sort_internal.h    | 80 +++++++++++++++++++
 .../arrow/compute/kernels/vector_sort_test.cc | 27 +++++++
 python/pyarrow/tests/test_array.py            | 31 +++++++
 4 files changed, 139 insertions(+)

diff --git a/cpp/src/arrow/compute/kernels/vector_sort.cc b/cpp/src/arrow/compute/kernels/vector_sort.cc
index 0a0399b9993..39d765e42a8 100644
--- a/cpp/src/arrow/compute/kernels/vector_sort.cc
+++ b/cpp/src/arrow/compute/kernels/vector_sort.cc
@@ -180,6 +180,7 @@ class ChunkedArraySorter : public TypeVisitor {
   Status Visit(const TYPE& type) override { return SortInternal<TYPE>(); }
 
   VISIT_SORTABLE_PHYSICAL_TYPES(VISIT)
+  VISIT(StructType)
 
 #undef VISIT
 
diff --git a/cpp/src/arrow/compute/kernels/vector_sort_internal.h b/cpp/src/arrow/compute/kernels/vector_sort_internal.h
index 9b0295deda3..f330444d25f 100644
--- a/cpp/src/arrow/compute/kernels/vector_sort_internal.h
+++ b/cpp/src/arrow/compute/kernels/vector_sort_internal.h
@@ -452,6 +452,86 @@ using ArraySortFunc = std::function<NullPartitionResult(
 
 Result<ArraySortFunc> GetArraySorter(const DataType& type);
 
+class NestedValuesComparator {
+  public:
+    NestedValuesComparator(Array const &array) {
+      for(auto field = fields.begin(); field != fields.end(); field++) {
+        std::shared_ptr<DataType> physical_type = GetPhysicalType((*field)->type());
+        NestedComparatorFactory comparator_factory = NestedComparatorFactory();
+        if(NestedComparatorFactory().MakeFieldComparator(*physical_type) != Status::OK())
+          // TODO: Improve error handling
+          return NullPartitionResult();
+        field_comparators_.push_back(current_field_comparator_);
+      }
+    }
+
+
+  private:
+    std::vector<NestedValueComparator> comparators_;
+
+  /** 
+   * Internal use factory whose purpose is to detect the right type
+   * of comparator that should be built to be able to compare two
+   * nested values in the same field or column
+   */
+  struct NestedComparatorFactory {
+    Result<std::shared_ptr<FieldValueComparator>> MakeFieldComparator(DataType const &physical_type) {
+        RETURN_NOT_OK(VisitTypeInline(physical_type, this));
+        DCHECK_NE(current_field_comparator_, nullptr);
+        return std::move(current_field_comparator_);
+    }
+
+  #define VISIT(TYPE) \
+    Status Visit(const TYPE& type) { return VisitGeneric(type); }
+
+      VISIT_SORTABLE_PHYSICAL_TYPES(VISIT)
+  #undef VISIT
+
+    Status Visit(const DataType& type) {
+      return Status::TypeError("Unsupported type for NestedComparatorFactory: ",
+                              type.ToString());
+    }
+
+    template <typename Type>
+    Status VisitGeneric(const Type&) {
+      current_field_comparator_ = std::shared_ptr<FieldValueComparator>(
+        new ConcreteFieldValueComparator<Type>()
+      );
+      return Status::OK();
+    }
+
+    std::shared_ptr<FieldValueComparator> current_field_comparator_;
+  }
+
+  
+  struct FieldValueComparator {
+    virtual int Compare(Array const &array, uint64_t offset, 
+                        uint64_t leftidx, uint64_t rightidx) = 0;
+    virtual ~FieldValueComparator() = default;
+  };
+
+  template <typename Type>
+  struct ConcreteFieldValueComparator : FieldValueComparator {
+    using FieldArrayType = typename TypeTraits<Type>::ArrayType;
+    using GetView = GetViewType<Type>;
+
+    virtual int Compare(Array const &array, uint64_t offset, 
+                        uint64_t leftidx, uint64_t rightidx) {
+      const FieldArrayType& values = checked_cast<const FieldArrayType&>(array);
+      auto left_value = GetView::LogicalValue(values.GetView(leftidx - offset));
+      auto right_value = GetView::LogicalValue(values.GetView(rightidx - offset));
+      if (left_value == right_value)
+        return 0;
+      else if (left_value < right_value)
+        return -1;
+      else
+        return 1;
+    }
+  };
+
+  std::vector<std::shared_ptr<FieldValueComparator>> field_comparators_;
+};
+
 }  // namespace internal
 }  // namespace compute
 }  // namespace arrow
diff --git a/cpp/src/arrow/compute/kernels/vector_sort_test.cc b/cpp/src/arrow/compute/kernels/vector_sort_test.cc
index 6e4f6160a29..995681c71f5 100644
--- a/cpp/src/arrow/compute/kernels/vector_sort_test.cc
+++ b/cpp/src/arrow/compute/kernels/vector_sort_test.cc
@@ -1929,5 +1929,32 @@ INSTANTIATE_TEST_SUITE_P(AllNull, TestTableSortIndicesRandom,
                          testing::Combine(first_sort_keys, num_sort_keys,
                                           testing::Values(1.0)));
 
+// Test basic cases for table.
+class TestNestedValuesComparator : public ::testing::Test {};
+
+TEST_F(TestNestedValuesComparator, Table) {
+  auto schema = ::arrow::schema({
+      {field("a", uint8())},
+      {field("b", uint32())},
+  });
+  std::shared_ptr<Table> table;
+
+  table = TableFromJSON(schema, {R"([{"a": null, "b": 5},
+                                     {"a": 1,    "b": 3},
+                                     {"a": 3,    "b": null},
+                                     {"a": null, "b": null},
+                                     {"a": 2,    "b": 5},
+                                     {"a": 1,    "b": 5},
+                                     {"a": 3,    "b": 5}
+                                    ])"});
+}
+
+TEST_F(TestNestedValuesComparator, StructArray) {
+  auto expected = ArrayFromJSON(
+    struct_({field("a", int32()), field("b", utf8())}),
+    R"([{"a": 4, "b": null}, {"a": null, "b": "foo"}])"
+  );
+}
+
 }  // namespace compute
 }  // namespace arrow
diff --git a/python/pyarrow/tests/test_array.py b/python/pyarrow/tests/test_array.py
index 42fac5b24cf..a13b29bb48f 100644
--- a/python/pyarrow/tests/test_array.py
+++ b/python/pyarrow/tests/test_array.py
@@ -764,6 +764,37 @@ def test_struct_array_sort():
         {"a": 35, "b": "foobar"},
     ]
 
+
+def test_struct_chunked_array_sort():
+    arr1 = pa.StructArray.from_arrays([
+        pa.array([5, 7], type=pa.int64()),
+        pa.array(["foo", "car"])
+    ], names=["a", "b"])
+
+    arr2 = pa.StructArray.from_arrays([
+        pa.array([7, 35], type=pa.int64()),
+        pa.array(["bar", "foobar"])
+    ], names=["a", "b"])
+
+    chunked_arr = pa.chunked_array([arr1, arr2])
+
+    sorted_arr = chunked_arr.sort("descending")
+    assert sorted_arr.to_pylist() == [
+        {"a": 35, "b": "foobar"},
+        {"a": 7, "b": "car"},
+        {"a": 7, "b": "bar"},
+        {"a": 5, "b": "foo"},
+    ]
+
+    sorted_arr = chunked_arr.sort("ascending")
+    assert sorted_arr.to_pylist() == [
+        {"a": 5, "b": "foo"},
+        {"a": 7, "b": "bar"},
+        {"a": 7, "b": "car"},
+        {"a": 35, "b": "foobar"},
+    ]
+
+
 def test_dictionary_from_numpy():
     indices = np.repeat([0, 1, 2], 2)
     dictionary = np.array(['foo', 'bar', 'baz'], dtype=object)

From c4305a145885a5a5145ecbf79558fdd7f7aef153 Mon Sep 17 00:00:00 2001
From: Alessandro Molina <amol@turbogears.org>
Date: Thu, 2 Dec 2021 11:59:02 +0100
Subject: [PATCH 06/24] Continue with refactoring

---
 .../compute/kernels/vector_sort_internal.h    | 131 +++++++++---------
 1 file changed, 69 insertions(+), 62 deletions(-)

diff --git a/cpp/src/arrow/compute/kernels/vector_sort_internal.h b/cpp/src/arrow/compute/kernels/vector_sort_internal.h
index f330444d25f..0d1ecf38919 100644
--- a/cpp/src/arrow/compute/kernels/vector_sort_internal.h
+++ b/cpp/src/arrow/compute/kernels/vector_sort_internal.h
@@ -454,82 +454,89 @@ Result<ArraySortFunc> GetArraySorter(const DataType& type);
 
 class NestedValuesComparator {
   public:
-    NestedValuesComparator(Array const &array) {
+    int Compare(const StructArray &array, uint64_t field_index, uint64_t offset, 
+                uint64_t leftrowidx, uint64_t rightrowidx) {
+      std::shared_ptr<Array> field = array.field(field_index);
+      return comparators_[field_index]->Compare(*field, offset, 
+                                                leftrowidx, rightrowidx);
+    }
+
+    Status Prepare(StructArray const &array) {
+      auto fields = array.fields();
       for(auto field = fields.begin(); field != fields.end(); field++) {
         std::shared_ptr<DataType> physical_type = GetPhysicalType((*field)->type());
         NestedComparatorFactory comparator_factory = NestedComparatorFactory();
-        if(NestedComparatorFactory().MakeFieldComparator(*physical_type) != Status::OK())
-          // TODO: Improve error handling
-          return NullPartitionResult();
-        field_comparators_.push_back(current_field_comparator_);
+        std::shared_ptr<NestedValueComparator> current_field_comparator;
+        ARROW_ASSIGN_OR_RAISE(
+          current_field_comparator,
+          NestedComparatorFactory().MakeFieldComparator(*physical_type)
+        );
+        comparators_.push_back(current_field_comparator);
       }
+      return Status::OK();
     }
 
-
   private:
-    std::vector<NestedValueComparator> comparators_;
-
-  /** 
-   * Internal use factory whose purpose is to detect the right type
-   * of comparator that should be built to be able to compare two
-   * nested values in the same field or column
-   */
-  struct NestedComparatorFactory {
-    Result<std::shared_ptr<FieldValueComparator>> MakeFieldComparator(DataType const &physical_type) {
-        RETURN_NOT_OK(VisitTypeInline(physical_type, this));
-        DCHECK_NE(current_field_comparator_, nullptr);
-        return std::move(current_field_comparator_);
-    }
+    struct NestedValueComparator {
+      virtual int Compare(Array const &array, uint64_t offset, 
+                          uint64_t leftidx, uint64_t rightidx) = 0;
+      virtual ~NestedValueComparator() = default;
+    };
 
-  #define VISIT(TYPE) \
-    Status Visit(const TYPE& type) { return VisitGeneric(type); }
+    template <typename Type>
+    struct ConcreteNestedValueComparator : NestedValueComparator {
+      using FieldArrayType = typename TypeTraits<Type>::ArrayType;
+      using GetView = GetViewType<Type>;
+
+      virtual int Compare(Array const &array, uint64_t offset, 
+                          uint64_t leftidx, uint64_t rightidx) {
+        const FieldArrayType& values = checked_cast<const FieldArrayType&>(array);
+        auto left_value = GetView::LogicalValue(values.GetView(leftidx - offset));
+        auto right_value = GetView::LogicalValue(values.GetView(rightidx - offset));
+        if (left_value == right_value)
+          return 0;
+        else if (left_value < right_value)
+          return -1;
+        else
+          return 1;
+      }
+    };
+
+    /** 
+     * Internal use factory whose purpose is to detect the right type
+     * of comparator that should be built to be able to compare two
+     * nested values in the same field or column
+     */
+    struct NestedComparatorFactory {
+      Result<std::shared_ptr<NestedValueComparator>> MakeFieldComparator(DataType const &physical_type) {
+          RETURN_NOT_OK(VisitTypeInline(physical_type, this));
+          DCHECK_NE(current_field_comparator_, nullptr);
+          return std::move(current_field_comparator_);
+      }
 
-      VISIT_SORTABLE_PHYSICAL_TYPES(VISIT)
-  #undef VISIT
+    #define VISIT(TYPE) \
+      Status Visit(const TYPE& type) { return VisitGeneric(type); }
 
-    Status Visit(const DataType& type) {
-      return Status::TypeError("Unsupported type for NestedComparatorFactory: ",
-                              type.ToString());
-    }
+        VISIT_SORTABLE_PHYSICAL_TYPES(VISIT)
+    #undef VISIT
 
-    template <typename Type>
-    Status VisitGeneric(const Type&) {
-      current_field_comparator_ = std::shared_ptr<FieldValueComparator>(
-        new ConcreteFieldValueComparator<Type>()
-      );
-      return Status::OK();
-    }
+      Status Visit(const DataType& type) {
+        return Status::TypeError("Unsupported type for NestedComparatorFactory: ",
+                                type.ToString());
+      }
 
-    std::shared_ptr<FieldValueComparator> current_field_comparator_;
-  }
+      template <typename Type>
+      Status VisitGeneric(const Type&) {
+        current_field_comparator_ = std::shared_ptr<NestedValueComparator>(
+          new ConcreteNestedValueComparator<Type>()
+        );
+        return Status::OK();
+      }
 
-  
-  struct FieldValueComparator {
-    virtual int Compare(Array const &array, uint64_t offset, 
-                        uint64_t leftidx, uint64_t rightidx) = 0;
-    virtual ~FieldValueComparator() = default;
-  };
-
-  template <typename Type>
-  struct ConcreteFieldValueComparator : FieldValueComparator {
-    using FieldArrayType = typename TypeTraits<Type>::ArrayType;
-    using GetView = GetViewType<Type>;
-
-    virtual int Compare(Array const &array, uint64_t offset, 
-                        uint64_t leftidx, uint64_t rightidx) {
-      const FieldArrayType& values = checked_cast<const FieldArrayType&>(array);
-      auto left_value = GetView::LogicalValue(values.GetView(leftidx - offset));
-      auto right_value = GetView::LogicalValue(values.GetView(rightidx - offset));
-      if (left_value == right_value)
-        return 0;
-      else if (left_value < right_value)
-        return -1;
-      else
-        return 1;
-    }
-  };
+      std::shared_ptr<NestedValueComparator> current_field_comparator_;
+    };
 
-  std::vector<std::shared_ptr<FieldValueComparator>> field_comparators_;
+  std::vector<std::shared_ptr<NestedValueComparator>> comparators_;
 };
 
 }  // namespace internal

From d638ee6718a258ffac4ac5a384fa6df667f4c122 Mon Sep 17 00:00:00 2001
From: Alessandro Molina <amol@turbogears.org>
Date: Thu, 2 Dec 2021 12:00:09 +0100
Subject: [PATCH 07/24] remove for the moment

---
 cpp/src/arrow/compute/kernels/vector_sort.cc | 1 -
 1 file changed, 1 deletion(-)

diff --git a/cpp/src/arrow/compute/kernels/vector_sort.cc b/cpp/src/arrow/compute/kernels/vector_sort.cc
index 39d765e42a8..0a0399b9993 100644
--- a/cpp/src/arrow/compute/kernels/vector_sort.cc
+++ b/cpp/src/arrow/compute/kernels/vector_sort.cc
@@ -180,7 +180,6 @@ class ChunkedArraySorter : public TypeVisitor {
   Status Visit(const TYPE& type) override { return SortInternal<TYPE>(); }
 
   VISIT_SORTABLE_PHYSICAL_TYPES(VISIT)
-  VISIT(StructType)
 
 #undef VISIT
 

From 68e54b86f07ea8477145a2b6ca0448ed0705e8e8 Mon Sep 17 00:00:00 2001
From: Alessandro Molina <amol@turbogears.org>
Date: Thu, 2 Dec 2021 16:05:02 +0100
Subject: [PATCH 08/24] Add unit test

---
 cpp/src/arrow/compute/kernels/vector_sort_test.cc | 14 +++++++++++---
 1 file changed, 11 insertions(+), 3 deletions(-)

diff --git a/cpp/src/arrow/compute/kernels/vector_sort_test.cc b/cpp/src/arrow/compute/kernels/vector_sort_test.cc
index 995681c71f5..a00a048be70 100644
--- a/cpp/src/arrow/compute/kernels/vector_sort_test.cc
+++ b/cpp/src/arrow/compute/kernels/vector_sort_test.cc
@@ -38,6 +38,8 @@
 #include "arrow/testing/util.h"
 #include "arrow/type_traits.h"
 #include "arrow/util/logging.h"
+#include "arrow/compute/kernels/vector_sort_internal.h"
+
 
 namespace arrow {
 
@@ -1950,10 +1952,16 @@ TEST_F(TestNestedValuesComparator, Table) {
 }
 
 TEST_F(TestNestedValuesComparator, StructArray) {
-  auto expected = ArrayFromJSON(
-    struct_({field("a", int32()), field("b", utf8())}),
-    R"([{"a": 4, "b": null}, {"a": null, "b": "foo"}])"
+  std::shared_ptr<StructArray> expected = std::dynamic_pointer_cast<StructArray>(
+    ArrayFromJSON(
+      struct_({field("a", int32()), field("b", utf8())}),
+      R"([{"a": 4, "b": "bar"}, {"a": 3, "b": "foo"}])"
+    )
   );
+
+  auto c = internal::NestedValuesComparator();
+  ASSERT_OK(c.Prepare(*expected));
+  ASSERT_EQ(c.Compare(*expected, 0, 0, 0, 1), 1);
 }
 
 }  // namespace compute

From f8d6478221aed062797fa6e4d6aaa53de494f26b Mon Sep 17 00:00:00 2001
From: Jedi18 <akhiljnair.188@gmail.com>
Date: Wed, 8 Dec 2021 12:39:07 +0530
Subject: [PATCH 09/24] Change StructArrayCompareSorter to use
 NestedValuesComparator, add support for Table and RecordBatch to
 NestedValuesComparator

---
 .../compute/kernels/vector_array_sort.cc      | 96 +++----------------
 .../compute/kernels/vector_sort_internal.h    | 90 ++++++++++++++++-
 .../arrow/compute/kernels/vector_sort_test.cc | 52 +++++++++-
 python/pyarrow/tests/test_array.py            | 10 +-
 4 files changed, 155 insertions(+), 93 deletions(-)

diff --git a/cpp/src/arrow/compute/kernels/vector_array_sort.cc b/cpp/src/arrow/compute/kernels/vector_array_sort.cc
index 183fa5bf77c..58fa12569a8 100644
--- a/cpp/src/arrow/compute/kernels/vector_array_sort.cc
+++ b/cpp/src/arrow/compute/kernels/vector_array_sort.cc
@@ -182,42 +182,23 @@ class StructArrayCompareSorter {
                                  const Array& array, int64_t offset,
                                  const ArraySortOptions& options) {
     const auto& values = checked_cast<const ArrayType&>(array);
-    ArrayVector fields = values.fields();
-
-    for(auto field = fields.begin(); field != fields.end(); field++) {
-      std::shared_ptr<DataType> physical_type = GetPhysicalType((*field)->type());
-      if(MakeFieldComparator(*physical_type) != Status::OK())
-        // TODO: Improve error handling
-        return NullPartitionResult();
-      field_comparators_.push_back(current_field_comparator_);
+    nested_value_comparator_ = std::make_shared<NestedValuesComparator>();
+
+    if(nested_value_comparator_->Prepare(values) != Status::OK()) {
+      // TODO: Improve error handling
+      return NullPartitionResult();
     }
 
     const auto p = PartitionNulls<ArrayType, StablePartitioner>(
         indices_begin, indices_end, values, offset, options.null_placement);
-    if (options.order == SortOrder::Ascending) {
-      std::stable_sort(
-          p.non_nulls_begin, p.non_nulls_end,
-          [&offset, &fields, this](uint64_t left, uint64_t right) {
-            for(ArrayVector::size_type fieldidx = 0; fieldidx < fields.size(); ++fieldidx) {
-              auto field = fields[fieldidx];
-              auto field_comparator = field_comparators_[fieldidx];
-              int result = field_comparator->Compare(*field, offset, left, right);
-              if(result == -1)
-                return true;
-              else if (result == 1)
-                return false;
-            }
-            return false;
-          });
-    }
-    else {
-      std::stable_sort(
+    
+    bool asc_order = options.order == SortOrder::Ascending;
+    std::stable_sort(
           p.non_nulls_begin, p.non_nulls_end,
-          [&offset, &fields, this](uint64_t left, uint64_t right) {        
-            for(ArrayVector::size_type fieldidx = 0; fieldidx < fields.size(); ++fieldidx) {
-              auto field = fields[fieldidx];
-              auto field_comparator = field_comparators_[fieldidx];
-              int result = field_comparator->Compare(*field, offset, right, left);
+          [&offset, &array, &values, asc_order, this](uint64_t left, uint64_t right) {
+            // is better to do values.fields.size() or values.schema().num_fields() ?
+            for(ArrayVector::size_type fieldidx = 0; fieldidx < values.fields().size(); ++fieldidx) {
+              int result = nested_value_comparator_->Compare(values, fieldidx, offset, asc_order ? left : right, asc_order ? right : left);
               if(result == -1)
                 return true;
               else if (result == 1)
@@ -225,61 +206,10 @@ class StructArrayCompareSorter {
             }
             return false;
           });
-    }
     return p;
   }
 
-  Status MakeFieldComparator(DataType const &physical_type) {
-    RETURN_NOT_OK(VisitTypeInline(physical_type, this));
-    DCHECK_NE(current_field_comparator_, nullptr);
-    return Status::OK();
-  }
-
-#define VISIT(TYPE) \
-  Status Visit(const TYPE& type) { return VisitGeneric(type); }
-
-    VISIT_SORTABLE_PHYSICAL_TYPES(VISIT)
-
-#undef VISIT
-
-    Status Visit(const DataType& type) {
-      return Status::TypeError("Unsupported type for StructArray sorting: ",
-                               type.ToString());
-    }
-
-    template <typename Type>
-    Status VisitGeneric(const Type&) {
-      current_field_comparator_ = std::shared_ptr<FieldValueComparator>(new ConcreteFieldValueComparator<Type>());
-      return Status::OK();
-    }
-  
-  struct FieldValueComparator {
-    virtual int Compare(Array const &array, uint64_t offset, 
-                        uint64_t leftidx, uint64_t rightidx) = 0;
-    virtual ~FieldValueComparator() = default;
-  };
-
-  template <typename Type>
-  struct ConcreteFieldValueComparator : FieldValueComparator {
-    using FieldArrayType = typename TypeTraits<Type>::ArrayType;
-    using GetView = GetViewType<Type>;
-
-    virtual int Compare(Array const &array, uint64_t offset, 
-                        uint64_t leftidx, uint64_t rightidx) {
-      const FieldArrayType& values = checked_cast<const FieldArrayType&>(array);
-      auto left_value = GetView::LogicalValue(values.GetView(leftidx - offset));
-      auto right_value = GetView::LogicalValue(values.GetView(rightidx - offset));
-      if (left_value == right_value)
-        return 0;
-      else if (left_value < right_value)
-        return -1;
-      else
-        return 1;
-    }
-  };
-
-  std::shared_ptr<FieldValueComparator> current_field_comparator_;
-  std::vector<std::shared_ptr<FieldValueComparator>> field_comparators_;
+  std::shared_ptr<NestedValuesComparator> nested_value_comparator_;
 };
 
 template <typename ArrowType>
diff --git a/cpp/src/arrow/compute/kernels/vector_sort_internal.h b/cpp/src/arrow/compute/kernels/vector_sort_internal.h
index 0d1ecf38919..58372c9b2da 100644
--- a/cpp/src/arrow/compute/kernels/vector_sort_internal.h
+++ b/cpp/src/arrow/compute/kernels/vector_sort_internal.h
@@ -25,6 +25,7 @@
 #include "arrow/array.h"
 #include "arrow/compute/api_vector.h"
 #include "arrow/compute/kernels/chunked_internal.h"
+#include "arrow/table.h"
 #include "arrow/type.h"
 #include "arrow/type_traits.h"
 
@@ -454,13 +455,15 @@ Result<ArraySortFunc> GetArraySorter(const DataType& type);
 
 class NestedValuesComparator {
   public:
-    int Compare(const StructArray &array, uint64_t field_index, uint64_t offset, 
+    // StructArray Compare overload
+    int Compare(StructArray const &array, uint64_t field_index, uint64_t offset, 
                 uint64_t leftrowidx, uint64_t rightrowidx) {
       std::shared_ptr<Array> field = array.field(field_index);
       return comparators_[field_index]->Compare(*field, offset, 
                                                 leftrowidx, rightrowidx);
     }
 
+    // StructArray Prepare overload
     Status Prepare(StructArray const &array) {
       auto fields = array.fields();
       for(auto field = fields.begin(); field != fields.end(); field++) {
@@ -476,10 +479,62 @@ class NestedValuesComparator {
       return Status::OK();
     }
 
+    // Table Compare overload
+    int Compare(Table const &table, uint64_t field_index, uint64_t offset, 
+                uint64_t leftrowidx, uint64_t rightrowidx) {
+      std::shared_ptr<ChunkedArray> field = table.column(field_index);
+      return comparators_[field_index]->Compare(*field, offset, 
+                                                leftrowidx, rightrowidx);
+    }
+
+    // Table Prepare overload
+    Status Prepare(Table const &array) {
+      auto fields = array.columns();
+      for(auto field = fields.cbegin(); field != fields.cend(); field++) {
+        std::shared_ptr<DataType> physical_type = GetPhysicalType((*field)->type());
+        NestedComparatorFactory comparator_factory = NestedComparatorFactory();
+        std::shared_ptr<NestedValueComparator> current_field_comparator;
+        ARROW_ASSIGN_OR_RAISE(
+          current_field_comparator,
+          NestedComparatorFactory().MakeFieldComparator(*physical_type)
+        );
+        comparators_.push_back(current_field_comparator);
+      }
+      return Status::OK();
+    }
+
+    // RecordBatch Compare overload
+    int Compare(RecordBatch const &batch, uint64_t field_index, uint64_t offset, 
+                uint64_t leftrowidx, uint64_t rightrowidx) {
+      std::shared_ptr<Array> field = batch.column(field_index);
+      return comparators_[field_index]->Compare(*field, offset, 
+                                                leftrowidx, rightrowidx);
+    }
+
+    // RecordBatch Prepare overload
+    Status Prepare(RecordBatch const &batch) {
+      auto fields = batch.columns();
+      for(auto field = fields.begin(); field != fields.end(); field++) {
+        std::shared_ptr<DataType> physical_type = GetPhysicalType((*field)->type());
+        NestedComparatorFactory comparator_factory = NestedComparatorFactory();
+        std::shared_ptr<NestedValueComparator> current_field_comparator;
+        ARROW_ASSIGN_OR_RAISE(
+          current_field_comparator,
+          NestedComparatorFactory().MakeFieldComparator(*physical_type)
+        );
+        comparators_.push_back(current_field_comparator);
+      }
+      return Status::OK();
+    }
+
   private:
     struct NestedValueComparator {
       virtual int Compare(Array const &array, uint64_t offset, 
                           uint64_t leftidx, uint64_t rightidx) = 0;
+
+      virtual int Compare(ChunkedArray const &array, uint64_t offset, 
+                          uint64_t leftidx, uint64_t rightidx) = 0;
+
       virtual ~NestedValueComparator() = default;
     };
 
@@ -500,6 +555,39 @@ class NestedValuesComparator {
         else
           return 1;
       }
+
+      virtual int Compare(ChunkedArray const &chunked_array, uint64_t offset, 
+                          uint64_t leftidx, uint64_t rightidx) {
+        decltype(GetView::LogicalValue(checked_cast<const FieldArrayType&>(chunked_array.chunk(0)).GetView(0))) l_value, r_value;
+
+        int n_chunks = chunked_array.num_chunks();
+        for(int i = 0; i < n_chunks; i++) {
+          auto chunk = chunked_array.chunk(i);
+          if(leftidx < chunk->length()) {
+            const FieldArrayType& values = checked_cast<const FieldArrayType&>(*chunk);
+            l_value = GetView::LogicalValue(values.GetView(leftidx));
+            break;
+          }
+          leftidx -= chunk->length();
+        }
+
+        for(int i = 0; i < n_chunks; i++) {
+          auto chunk = chunked_array.chunk(i);
+          if(rightidx < chunk->length()) {
+            const FieldArrayType& values = checked_cast<const FieldArrayType&>(*chunk);
+            r_value = GetView::LogicalValue(values.GetView(rightidx));
+            break;
+          }
+          rightidx -= chunk->length();
+        }
+
+        if (l_value == r_value)
+          return 0;
+        else if (l_value < r_value)
+          return -1;
+        else
+          return 1;
+      }
     };
 
     /** 
diff --git a/cpp/src/arrow/compute/kernels/vector_sort_test.cc b/cpp/src/arrow/compute/kernels/vector_sort_test.cc
index a00a048be70..9f2bace7fcc 100644
--- a/cpp/src/arrow/compute/kernels/vector_sort_test.cc
+++ b/cpp/src/arrow/compute/kernels/vector_sort_test.cc
@@ -1946,22 +1946,66 @@ TEST_F(TestNestedValuesComparator, Table) {
                                      {"a": 3,    "b": null},
                                      {"a": null, "b": null},
                                      {"a": 2,    "b": 5},
-                                     {"a": 1,    "b": 5},
-                                     {"a": 3,    "b": 5}
+                                     {"a": 5,    "b": 2},
+                                     {"a": 3,    "b": 2}
                                     ])"});
+
+  auto c = internal::NestedValuesComparator();
+  ASSERT_OK(c.Prepare(*table));
+
+  ASSERT_EQ(c.Compare(*table, 0, 0, 0, 1), -1);
+  ASSERT_EQ(c.Compare(*table, 0, 0, 2, 3), 1);
+  ASSERT_EQ(c.Compare(*table, 1, 0, 2, 3), 0);
+  ASSERT_EQ(c.Compare(*table, 0, 0, 1, 2), -1);
+  ASSERT_EQ(c.Compare(*table, 0, 0, 0, 2), -1);
+  ASSERT_EQ(c.Compare(*table, 0, 0, 5, 6), 1);
+  ASSERT_EQ(c.Compare(*table, 1, 0, 5, 6), 0);
+  ASSERT_EQ(c.Compare(*table, 0, 0, 5, 1), 1);
+  ASSERT_EQ(c.Compare(*table, 1, 0, 5, 4), -1);
 }
 
 TEST_F(TestNestedValuesComparator, StructArray) {
   std::shared_ptr<StructArray> expected = std::dynamic_pointer_cast<StructArray>(
     ArrayFromJSON(
-      struct_({field("a", int32()), field("b", utf8())}),
-      R"([{"a": 4, "b": "bar"}, {"a": 3, "b": "foo"}])"
+      struct_({field("a", int32()), field("b", utf8()), field("c", int32())}),
+      R"([{"a": 4, "b": "bar", "c": 4}, {"a": 3, "b": "foo", "c": 4}])"
     )
   );
 
   auto c = internal::NestedValuesComparator();
   ASSERT_OK(c.Prepare(*expected));
   ASSERT_EQ(c.Compare(*expected, 0, 0, 0, 1), 1);
+  ASSERT_EQ(c.Compare(*expected, 1, 0, 0, 1), -1);
+  ASSERT_EQ(c.Compare(*expected, 2, 0, 0, 1), 0);
+}
+
+TEST_F(TestNestedValuesComparator, RecordBatch) {
+  auto schema = ::arrow::schema({
+      {field("a", uint8())},
+      {field("b", uint32())},
+  });
+
+  auto batch = RecordBatchFromJSON(schema, {R"([{"a": null, "b": 5},
+                                     {"a": 1,    "b": 3},
+                                     {"a": 3,    "b": null},
+                                     {"a": null, "b": null},
+                                     {"a": 2,    "b": 5},
+                                     {"a": 5,    "b": 2},
+                                     {"a": 3,    "b": 2}
+                                    ])"});
+
+  auto c = internal::NestedValuesComparator();
+  ASSERT_OK(c.Prepare(*batch));
+
+  ASSERT_EQ(c.Compare(*batch, 0, 0, 0, 1), -1);
+  ASSERT_EQ(c.Compare(*batch, 0, 0, 2, 3), 1);
+  ASSERT_EQ(c.Compare(*batch, 1, 0, 2, 3), 0);
+  ASSERT_EQ(c.Compare(*batch, 0, 0, 1, 2), -1);
+  ASSERT_EQ(c.Compare(*batch, 0, 0, 0, 2), -1);
+  ASSERT_EQ(c.Compare(*batch, 0, 0, 5, 6), 1);
+  ASSERT_EQ(c.Compare(*batch, 1, 0, 5, 6), 0);
+  ASSERT_EQ(c.Compare(*batch, 0, 0, 5, 1), 1);
+  ASSERT_EQ(c.Compare(*batch, 1, 0, 5, 4), -1);
 }
 
 }  // namespace compute
diff --git a/python/pyarrow/tests/test_array.py b/python/pyarrow/tests/test_array.py
index a13b29bb48f..1a3e1b4d2d5 100644
--- a/python/pyarrow/tests/test_array.py
+++ b/python/pyarrow/tests/test_array.py
@@ -748,19 +748,19 @@ def test_struct_array_sort():
         {"a": 5, "b": "foo"},
     ]
 
-    sorted_arr = arr.sort("descending")
+    sorted_arr = arr.sort("ascending", fieldname="b")
     assert sorted_arr.to_pylist() == [
-        {"a": 35, "b": "foobar"},
-        {"a": 7, "b": "car"},
         {"a": 7, "b": "bar"},
+        {"a": 7, "b": "car"},
         {"a": 5, "b": "foo"},
+        {"a": 35, "b": "foobar"},
     ]
 
-    sorted_arr = arr.sort("ascending")
+    sorted_arr = arr.sort("ascending", fieldname="a")
     assert sorted_arr.to_pylist() == [
         {"a": 5, "b": "foo"},
-        {"a": 7, "b": "bar"},
         {"a": 7, "b": "car"},
+        {"a": 7, "b": "bar"},
         {"a": 35, "b": "foobar"},
     ]
 

From 0fb14a45ca41cb809dc0cee55effef442fb44eff Mon Sep 17 00:00:00 2001
From: Jedi18 <akhiljnair.188@gmail.com>
Date: Mon, 27 Dec 2021 14:51:37 +0530
Subject: [PATCH 10/24] chunked array sort test runs

---
 cpp/src/arrow/compute/kernels/chunked_internal.h  | 15 +++++++++++++++
 cpp/src/arrow/compute/kernels/vector_sort.cc      |  4 ++++
 .../arrow/compute/kernels/vector_sort_internal.h  |  4 ++--
 python/pyarrow/tests/test_array.py                | 12 ++++++------
 4 files changed, 27 insertions(+), 8 deletions(-)

diff --git a/cpp/src/arrow/compute/kernels/chunked_internal.h b/cpp/src/arrow/compute/kernels/chunked_internal.h
index b007d6cbfb8..eedeaba0ae3 100644
--- a/cpp/src/arrow/compute/kernels/chunked_internal.h
+++ b/cpp/src/arrow/compute/kernels/chunked_internal.h
@@ -48,6 +48,21 @@ struct ResolvedChunk {
   LogicalValueType Value() const { return V::LogicalValue(array->GetView(index)); }
 };
 
+// ResolvedChunk specialization for StructArray
+template<>
+struct ResolvedChunk<StructArray> {
+  // The target struct in chunked array.
+  const StructArray* array;
+  // The field index in the target struct.
+  const int64_t index;
+
+  ResolvedChunk(const StructArray* array, int64_t index) : array(array), index(index) {}
+
+  bool IsNull() const { return array->IsNull(index); }
+
+  auto Value() const ->decltype((*array->GetScalar(0))) { return *(array->GetScalar(index)); }
+};
+
 // ResolvedChunk specialization for untyped arrays when all is needed is null lookup
 template <>
 struct ResolvedChunk<Array> {
diff --git a/cpp/src/arrow/compute/kernels/vector_sort.cc b/cpp/src/arrow/compute/kernels/vector_sort.cc
index 0a0399b9993..51f17ee9098 100644
--- a/cpp/src/arrow/compute/kernels/vector_sort.cc
+++ b/cpp/src/arrow/compute/kernels/vector_sort.cc
@@ -188,6 +188,10 @@ class ChunkedArraySorter : public TypeVisitor {
     return Status::OK();
   }
 
+  Status Visit(const StructType& type) override {
+    return SortInternal<StructType>();
+  }
+
  private:
   template <typename Type>
   Status SortInternal() {
diff --git a/cpp/src/arrow/compute/kernels/vector_sort_internal.h b/cpp/src/arrow/compute/kernels/vector_sort_internal.h
index 58372c9b2da..c726f5ae52b 100644
--- a/cpp/src/arrow/compute/kernels/vector_sort_internal.h
+++ b/cpp/src/arrow/compute/kernels/vector_sort_internal.h
@@ -563,7 +563,7 @@ class NestedValuesComparator {
         int n_chunks = chunked_array.num_chunks();
         for(int i = 0; i < n_chunks; i++) {
           auto chunk = chunked_array.chunk(i);
-          if(leftidx < chunk->length()) {
+          if(leftidx < static_cast<uint64_t>(chunk->length())) {
             const FieldArrayType& values = checked_cast<const FieldArrayType&>(*chunk);
             l_value = GetView::LogicalValue(values.GetView(leftidx));
             break;
@@ -573,7 +573,7 @@ class NestedValuesComparator {
 
         for(int i = 0; i < n_chunks; i++) {
           auto chunk = chunked_array.chunk(i);
-          if(rightidx < chunk->length()) {
+          if(rightidx < static_cast<uint64_t>(chunk->length())) {
             const FieldArrayType& values = checked_cast<const FieldArrayType&>(*chunk);
             r_value = GetView::LogicalValue(values.GetView(rightidx));
             break;
diff --git a/python/pyarrow/tests/test_array.py b/python/pyarrow/tests/test_array.py
index 1a3e1b4d2d5..051973ecdbc 100644
--- a/python/pyarrow/tests/test_array.py
+++ b/python/pyarrow/tests/test_array.py
@@ -748,19 +748,19 @@ def test_struct_array_sort():
         {"a": 5, "b": "foo"},
     ]
 
-    sorted_arr = arr.sort("ascending", fieldname="b")
+    sorted_arr = arr.sort("descending")
     assert sorted_arr.to_pylist() == [
-        {"a": 7, "b": "bar"},
+        {"a": 35, "b": "foobar"},
         {"a": 7, "b": "car"},
+        {"a": 7, "b": "bar"},
         {"a": 5, "b": "foo"},
-        {"a": 35, "b": "foobar"},
     ]
 
-    sorted_arr = arr.sort("ascending", fieldname="a")
+    sorted_arr = arr.sort("ascending")
     assert sorted_arr.to_pylist() == [
         {"a": 5, "b": "foo"},
-        {"a": 7, "b": "car"},
         {"a": 7, "b": "bar"},
+        {"a": 7, "b": "car"},
         {"a": 35, "b": "foobar"},
     ]
 
@@ -781,8 +781,8 @@ def test_struct_chunked_array_sort():
     sorted_arr = chunked_arr.sort("descending")
     assert sorted_arr.to_pylist() == [
         {"a": 35, "b": "foobar"},
-        {"a": 7, "b": "car"},
         {"a": 7, "b": "bar"},
+        {"a": 7, "b": "car"},
         {"a": 5, "b": "foo"},
     ]
 

From 011f47b7e261c696e554ff3f0e7ee572ff6955a0 Mon Sep 17 00:00:00 2001
From: Jedi18 <akhiljnair.188@gmail.com>
Date: Thu, 30 Dec 2021 12:49:53 +0530
Subject: [PATCH 11/24] changes to chunked array sort and add test and sort
 function for RecordBatch

---
 .../arrow/compute/kernels/chunked_internal.h  |  9 +++--
 python/pyarrow/table.pxi                      | 16 +++++++++
 python/pyarrow/tests/test_array.py            | 33 +++++++++++++++----
 3 files changed, 50 insertions(+), 8 deletions(-)

diff --git a/cpp/src/arrow/compute/kernels/chunked_internal.h b/cpp/src/arrow/compute/kernels/chunked_internal.h
index eedeaba0ae3..a1606cd4d70 100644
--- a/cpp/src/arrow/compute/kernels/chunked_internal.h
+++ b/cpp/src/arrow/compute/kernels/chunked_internal.h
@@ -58,9 +58,14 @@ struct ResolvedChunk<StructArray> {
 
   ResolvedChunk(const StructArray* array, int64_t index) : array(array), index(index) {}
 
-  bool IsNull() const { return array->IsNull(index); }
+  bool IsNull() const { return array->field(0)->IsNull(index); }
+
+  int64_t Value() const {
+    auto t = array->field(0)->type();
+    auto arr = std::static_pointer_cast<arrow::Int64Array>(array->field(0));
 
-  auto Value() const ->decltype((*array->GetScalar(0))) { return *(array->GetScalar(index)); }
+    return arr->Value(index); 
+  }
 };
 
 // ResolvedChunk specialization for untyped arrays when all is needed is null lookup
diff --git a/python/pyarrow/table.pxi b/python/pyarrow/table.pxi
index 740975a4ec7..c8f9436300a 100644
--- a/python/pyarrow/table.pxi
+++ b/python/pyarrow/table.pxi
@@ -1017,6 +1017,22 @@ cdef class RecordBatch(_PandasConvertible):
         """
         return _pc().drop_null(self)
 
+    def sort(self, sort_key, order="ascending"):
+        """
+        Sort the RecordBatch
+
+        Parameters
+        ----------
+        order : "ascending" or "descending"
+            The order of the sorting.
+
+        Returns
+        -------
+        result : RecordBatch
+        """
+        indices = _pc().sort_indices(self, sort_keys=[(sort_key, order)])
+        return self.take(indices)
+
     def to_pydict(self):
         """
         Convert the RecordBatch to a dict or OrderedDict.
diff --git a/python/pyarrow/tests/test_array.py b/python/pyarrow/tests/test_array.py
index 051973ecdbc..8ec72dbf0c2 100644
--- a/python/pyarrow/tests/test_array.py
+++ b/python/pyarrow/tests/test_array.py
@@ -767,13 +767,13 @@ def test_struct_array_sort():
 
 def test_struct_chunked_array_sort():
     arr1 = pa.StructArray.from_arrays([
-        pa.array([5, 7], type=pa.int64()),
-        pa.array(["foo", "car"])
+        pa.array([5, 7, 8], type=pa.int64()),
+        pa.array(["foo", "car", "far"])
     ], names=["a", "b"])
 
     arr2 = pa.StructArray.from_arrays([
-        pa.array([7, 35], type=pa.int64()),
-        pa.array(["bar", "foobar"])
+        pa.array([7, 35, 9], type=pa.int64()),
+        pa.array(["bar", "foobar", "tar"])
     ], names=["a", "b"])
 
     chunked_arr = pa.chunked_array([arr1, arr2])
@@ -781,20 +781,41 @@ def test_struct_chunked_array_sort():
     sorted_arr = chunked_arr.sort("descending")
     assert sorted_arr.to_pylist() == [
         {"a": 35, "b": "foobar"},
-        {"a": 7, "b": "bar"},
+        {"a": 9, "b": "tar"},
+        {"a": 8, "b": "far"},
         {"a": 7, "b": "car"},
+        {"a": 7, "b": "bar"},
         {"a": 5, "b": "foo"},
     ]
 
     sorted_arr = chunked_arr.sort("ascending")
     assert sorted_arr.to_pylist() == [
         {"a": 5, "b": "foo"},
-        {"a": 7, "b": "bar"},
         {"a": 7, "b": "car"},
+        {"a": 7, "b": "bar"},
+        {"a": 8, "b": "far"},
+        {"a": 9, "b": "tar"},
         {"a": 35, "b": "foobar"},
     ]
 
 
+def test_struct_record_batch_sort():
+    rb = pa.RecordBatch.from_arrays([
+        pa.array([5, 7, 7, 35], type=pa.int64()),
+        pa.array(["foo", "car", "bar", "foobar"])
+    ], names=["a", "b"])
+
+    sorted_rb = rb.sort("a", "descending")
+    sorted_rb_dict = sorted_rb.to_pydict()
+    assert sorted_rb_dict["a"] == [35, 7, 7, 5]
+    assert sorted_rb_dict["b"] == ["foobar", "car", "bar", "foo"]
+
+    sorted_rb = rb.sort("a", "ascending")
+    sorted_rb_dict = sorted_rb.to_pydict()
+    assert sorted_rb_dict["a"] == [5, 7, 7, 35]
+    assert sorted_rb_dict["b"] == ["foo", "car", "bar", "foobar"]
+
+
 def test_dictionary_from_numpy():
     indices = np.repeat([0, 1, 2], 2)
     dictionary = np.array(['foo', 'bar', 'baz'], dtype=object)

From 03539c79a364930c39d67e143d637e1655c1558e Mon Sep 17 00:00:00 2001
From: Jedi18 <akhiljnair.188@gmail.com>
Date: Thu, 30 Dec 2021 22:23:14 +0530
Subject: [PATCH 12/24] add small table sorting test, minor changes to
 RecordBatch sort

---
 .../compute/kernels/vector_array_sort.cc      |  2 +-
 .../compute/kernels/vector_sort_internal.h    |  3 ++-
 python/pyarrow/table.pxi                      | 19 ++++++++++-----
 python/pyarrow/tests/test_array.py            | 23 ++++++++++++++++---
 4 files changed, 36 insertions(+), 11 deletions(-)

diff --git a/cpp/src/arrow/compute/kernels/vector_array_sort.cc b/cpp/src/arrow/compute/kernels/vector_array_sort.cc
index 58fa12569a8..dc47b966c64 100644
--- a/cpp/src/arrow/compute/kernels/vector_array_sort.cc
+++ b/cpp/src/arrow/compute/kernels/vector_array_sort.cc
@@ -195,7 +195,7 @@ class StructArrayCompareSorter {
     bool asc_order = options.order == SortOrder::Ascending;
     std::stable_sort(
           p.non_nulls_begin, p.non_nulls_end,
-          [&offset, &array, &values, asc_order, this](uint64_t left, uint64_t right) {
+          [&offset, &values, asc_order, this](uint64_t left, uint64_t right) {
             // is better to do values.fields.size() or values.schema().num_fields() ?
             for(ArrayVector::size_type fieldidx = 0; fieldidx < values.fields().size(); ++fieldidx) {
               int result = nested_value_comparator_->Compare(values, fieldidx, offset, asc_order ? left : right, asc_order ? right : left);
diff --git a/cpp/src/arrow/compute/kernels/vector_sort_internal.h b/cpp/src/arrow/compute/kernels/vector_sort_internal.h
index c726f5ae52b..e7530a785ae 100644
--- a/cpp/src/arrow/compute/kernels/vector_sort_internal.h
+++ b/cpp/src/arrow/compute/kernels/vector_sort_internal.h
@@ -558,7 +558,8 @@ class NestedValuesComparator {
 
       virtual int Compare(ChunkedArray const &chunked_array, uint64_t offset, 
                           uint64_t leftidx, uint64_t rightidx) {
-        decltype(GetView::LogicalValue(checked_cast<const FieldArrayType&>(chunked_array.chunk(0)).GetView(0))) l_value, r_value;
+        using Val = decltype(GetView::LogicalValue(checked_cast<const FieldArrayType&>(chunked_array.chunk(0)).GetView(0)));
+        Val l_value = 0, r_value = 0;
 
         int n_chunks = chunked_array.num_chunks();
         for(int i = 0; i < n_chunks; i++) {
diff --git a/python/pyarrow/table.pxi b/python/pyarrow/table.pxi
index c8f9436300a..43c248aea2f 100644
--- a/python/pyarrow/table.pxi
+++ b/python/pyarrow/table.pxi
@@ -1017,20 +1017,27 @@ cdef class RecordBatch(_PandasConvertible):
         """
         return _pc().drop_null(self)
 
-    def sort(self, sort_key, order="ascending"):
+    def sort_by(self, sorting):
         """
-        Sort the RecordBatch
+        Sort the RecordBatch by one or multiple columns.
 
         Parameters
         ----------
-        order : "ascending" or "descending"
-            The order of the sorting.
+        sorting : str or list[tuple(name, order)]
+            Name of the column to use to sort (ascending), or
+            a list of multiple sorting conditions where
+            each entry is a tuple with column name
+            and sorting order ("ascending" or "descending")
 
         Returns
         -------
-        result : RecordBatch
+        RecordBatch
+            A new record batch sorted according to the sort keys.
         """
-        indices = _pc().sort_indices(self, sort_keys=[(sort_key, order)])
+        if isinstance(sorting, str):
+            sorting = [(sorting, "ascending")]
+
+        indices = _pc().sort_indices(self, sort_keys=sorting)
         return self.take(indices)
 
     def to_pydict(self):
diff --git a/python/pyarrow/tests/test_array.py b/python/pyarrow/tests/test_array.py
index 8ec72dbf0c2..72b4c84fc96 100644
--- a/python/pyarrow/tests/test_array.py
+++ b/python/pyarrow/tests/test_array.py
@@ -799,23 +799,40 @@ def test_struct_chunked_array_sort():
     ]
 
 
-def test_struct_record_batch_sort():
+def test_record_batch_sort():
     rb = pa.RecordBatch.from_arrays([
         pa.array([5, 7, 7, 35], type=pa.int64()),
         pa.array(["foo", "car", "bar", "foobar"])
     ], names=["a", "b"])
 
-    sorted_rb = rb.sort("a", "descending")
+    sorted_rb = rb.sort_by([("a", "descending")])
     sorted_rb_dict = sorted_rb.to_pydict()
     assert sorted_rb_dict["a"] == [35, 7, 7, 5]
     assert sorted_rb_dict["b"] == ["foobar", "car", "bar", "foo"]
 
-    sorted_rb = rb.sort("a", "ascending")
+    sorted_rb = rb.sort_by([("a", "ascending")])
     sorted_rb_dict = sorted_rb.to_pydict()
     assert sorted_rb_dict["a"] == [5, 7, 7, 35]
     assert sorted_rb_dict["b"] == ["foo", "car", "bar", "foobar"]
 
 
+def test_table_sort():
+    tab = pa.Table.from_arrays([
+        pa.array([5, 7, 7, 35], type=pa.int64()),
+        pa.array(["foo", "car", "bar", "foobar"])
+    ], names=["a", "b"])
+
+    sorted_tab = tab.sort_by([("a", "descending")])
+    sorted_tab_dict = sorted_tab.to_pydict()
+    assert sorted_tab_dict["a"] == [35, 7, 7, 5]
+    assert sorted_tab_dict["b"] == ["foobar", "car", "bar", "foo"]
+
+    sorted_tab = tab.sort_by([("a", "ascending")])
+    sorted_tab_dict = sorted_tab.to_pydict()
+    assert sorted_tab_dict["a"] == [5, 7, 7, 35]
+    assert sorted_tab_dict["b"] == ["foo", "car", "bar", "foobar"]
+
+
 def test_dictionary_from_numpy():
     indices = np.repeat([0, 1, 2], 2)
     dictionary = np.array(['foo', 'bar', 'baz'], dtype=object)

From 1ef9769a73f84cf95e914040101d30a6edd1a425 Mon Sep 17 00:00:00 2001
From: Jedi18 <akhiljnair.188@gmail.com>
Date: Mon, 3 Jan 2022 12:58:33 +0530
Subject: [PATCH 13/24] clang format fix

---
 .../arrow/compute/kernels/chunked_internal.h  |   4 +-
 .../compute/kernels/vector_array_sort.cc      |  33 +-
 cpp/src/arrow/compute/kernels/vector_sort.cc  |   4 +-
 .../compute/kernels/vector_sort_internal.h    | 291 +++++++++---------
 .../arrow/compute/kernels/vector_sort_test.cc |  13 +-
 5 files changed, 169 insertions(+), 176 deletions(-)

diff --git a/cpp/src/arrow/compute/kernels/chunked_internal.h b/cpp/src/arrow/compute/kernels/chunked_internal.h
index a1606cd4d70..30233e68319 100644
--- a/cpp/src/arrow/compute/kernels/chunked_internal.h
+++ b/cpp/src/arrow/compute/kernels/chunked_internal.h
@@ -49,7 +49,7 @@ struct ResolvedChunk {
 };
 
 // ResolvedChunk specialization for StructArray
-template<>
+template <>
 struct ResolvedChunk<StructArray> {
   // The target struct in chunked array.
   const StructArray* array;
@@ -64,7 +64,7 @@ struct ResolvedChunk<StructArray> {
     auto t = array->field(0)->type();
     auto arr = std::static_pointer_cast<arrow::Int64Array>(array->field(0));
 
-    return arr->Value(index); 
+    return arr->Value(index);
   }
 };
 
diff --git a/cpp/src/arrow/compute/kernels/vector_array_sort.cc b/cpp/src/arrow/compute/kernels/vector_array_sort.cc
index dc47b966c64..061118cab24 100644
--- a/cpp/src/arrow/compute/kernels/vector_array_sort.cc
+++ b/cpp/src/arrow/compute/kernels/vector_array_sort.cc
@@ -184,28 +184,31 @@ class StructArrayCompareSorter {
     const auto& values = checked_cast<const ArrayType&>(array);
     nested_value_comparator_ = std::make_shared<NestedValuesComparator>();
 
-    if(nested_value_comparator_->Prepare(values) != Status::OK()) {
+    if (nested_value_comparator_->Prepare(values) != Status::OK()) {
       // TODO: Improve error handling
       return NullPartitionResult();
     }
 
     const auto p = PartitionNulls<ArrayType, StablePartitioner>(
         indices_begin, indices_end, values, offset, options.null_placement);
-    
+
     bool asc_order = options.order == SortOrder::Ascending;
-    std::stable_sort(
-          p.non_nulls_begin, p.non_nulls_end,
-          [&offset, &values, asc_order, this](uint64_t left, uint64_t right) {
-            // is better to do values.fields.size() or values.schema().num_fields() ?
-            for(ArrayVector::size_type fieldidx = 0; fieldidx < values.fields().size(); ++fieldidx) {
-              int result = nested_value_comparator_->Compare(values, fieldidx, offset, asc_order ? left : right, asc_order ? right : left);
-              if(result == -1)
-                return true;
-              else if (result == 1)
-                return false;
-            }
-            return false;
-          });
+    std::stable_sort(p.non_nulls_begin, p.non_nulls_end,
+                     [&offset, &values, asc_order, this](uint64_t left, uint64_t right) {
+                       // is better to do values.fields.size() or
+                       // values.schema().num_fields() ?
+                       for (ArrayVector::size_type fieldidx = 0;
+                            fieldidx < values.fields().size(); ++fieldidx) {
+                         int result = nested_value_comparator_->Compare(
+                             values, fieldidx, offset, asc_order ? left : right,
+                             asc_order ? right : left);
+                         if (result == -1)
+                           return true;
+                         else if (result == 1)
+                           return false;
+                       }
+                       return false;
+                     });
     return p;
   }
 
diff --git a/cpp/src/arrow/compute/kernels/vector_sort.cc b/cpp/src/arrow/compute/kernels/vector_sort.cc
index 51f17ee9098..c91f252b2c9 100644
--- a/cpp/src/arrow/compute/kernels/vector_sort.cc
+++ b/cpp/src/arrow/compute/kernels/vector_sort.cc
@@ -188,9 +188,7 @@ class ChunkedArraySorter : public TypeVisitor {
     return Status::OK();
   }
 
-  Status Visit(const StructType& type) override {
-    return SortInternal<StructType>();
-  }
+  Status Visit(const StructType& type) override { return SortInternal<StructType>(); }
 
  private:
   template <typename Type>
diff --git a/cpp/src/arrow/compute/kernels/vector_sort_internal.h b/cpp/src/arrow/compute/kernels/vector_sort_internal.h
index e7530a785ae..fca511697e3 100644
--- a/cpp/src/arrow/compute/kernels/vector_sort_internal.h
+++ b/cpp/src/arrow/compute/kernels/vector_sort_internal.h
@@ -454,176 +454,171 @@ using ArraySortFunc = std::function<NullPartitionResult(
 Result<ArraySortFunc> GetArraySorter(const DataType& type);
 
 class NestedValuesComparator {
-  public:
-    // StructArray Compare overload
-    int Compare(StructArray const &array, uint64_t field_index, uint64_t offset, 
-                uint64_t leftrowidx, uint64_t rightrowidx) {
-      std::shared_ptr<Array> field = array.field(field_index);
-      return comparators_[field_index]->Compare(*field, offset, 
-                                                leftrowidx, rightrowidx);
-    }
+ public:
+  // StructArray Compare overload
+  int Compare(StructArray const& array, uint64_t field_index, uint64_t offset,
+              uint64_t leftrowidx, uint64_t rightrowidx) {
+    std::shared_ptr<Array> field = array.field(static_cast<int>(field_index));
+    return comparators_[field_index]->Compare(*field, offset, leftrowidx, rightrowidx);
+  }
 
-    // StructArray Prepare overload
-    Status Prepare(StructArray const &array) {
-      auto fields = array.fields();
-      for(auto field = fields.begin(); field != fields.end(); field++) {
-        std::shared_ptr<DataType> physical_type = GetPhysicalType((*field)->type());
-        NestedComparatorFactory comparator_factory = NestedComparatorFactory();
-        std::shared_ptr<NestedValueComparator> current_field_comparator;
-        ARROW_ASSIGN_OR_RAISE(
+  // StructArray Prepare overload
+  Status Prepare(StructArray const& array) {
+    auto fields = array.fields();
+    for (auto field = fields.begin(); field != fields.end(); field++) {
+      std::shared_ptr<DataType> physical_type = GetPhysicalType((*field)->type());
+      NestedComparatorFactory comparator_factory = NestedComparatorFactory();
+      std::shared_ptr<NestedValueComparator> current_field_comparator;
+      ARROW_ASSIGN_OR_RAISE(
           current_field_comparator,
-          NestedComparatorFactory().MakeFieldComparator(*physical_type)
-        );
-        comparators_.push_back(current_field_comparator);
-      }
-      return Status::OK();
+          NestedComparatorFactory().MakeFieldComparator(*physical_type));
+      comparators_.push_back(current_field_comparator);
     }
+    return Status::OK();
+  }
 
-    // Table Compare overload
-    int Compare(Table const &table, uint64_t field_index, uint64_t offset, 
-                uint64_t leftrowidx, uint64_t rightrowidx) {
-      std::shared_ptr<ChunkedArray> field = table.column(field_index);
-      return comparators_[field_index]->Compare(*field, offset, 
-                                                leftrowidx, rightrowidx);
-    }
+  // Table Compare overload
+  int Compare(Table const& table, uint64_t field_index, uint64_t offset,
+              uint64_t leftrowidx, uint64_t rightrowidx) {
+    std::shared_ptr<ChunkedArray> field = table.column(static_cast<int>(field_index));
+    return comparators_[field_index]->Compare(*field, offset, leftrowidx, rightrowidx);
+  }
 
-    // Table Prepare overload
-    Status Prepare(Table const &array) {
-      auto fields = array.columns();
-      for(auto field = fields.cbegin(); field != fields.cend(); field++) {
-        std::shared_ptr<DataType> physical_type = GetPhysicalType((*field)->type());
-        NestedComparatorFactory comparator_factory = NestedComparatorFactory();
-        std::shared_ptr<NestedValueComparator> current_field_comparator;
-        ARROW_ASSIGN_OR_RAISE(
+  // Table Prepare overload
+  Status Prepare(Table const& array) {
+    auto fields = array.columns();
+    for (auto field = fields.begin(); field != fields.end(); field++) {
+      std::shared_ptr<DataType> physical_type = GetPhysicalType((*field)->type());
+      NestedComparatorFactory comparator_factory = NestedComparatorFactory();
+      std::shared_ptr<NestedValueComparator> current_field_comparator;
+      ARROW_ASSIGN_OR_RAISE(
           current_field_comparator,
-          NestedComparatorFactory().MakeFieldComparator(*physical_type)
-        );
-        comparators_.push_back(current_field_comparator);
-      }
-      return Status::OK();
+          NestedComparatorFactory().MakeFieldComparator(*physical_type));
+      comparators_.push_back(current_field_comparator);
     }
+    return Status::OK();
+  }
 
-    // RecordBatch Compare overload
-    int Compare(RecordBatch const &batch, uint64_t field_index, uint64_t offset, 
-                uint64_t leftrowidx, uint64_t rightrowidx) {
-      std::shared_ptr<Array> field = batch.column(field_index);
-      return comparators_[field_index]->Compare(*field, offset, 
-                                                leftrowidx, rightrowidx);
-    }
+  // RecordBatch Compare overload
+  int Compare(RecordBatch const& batch, uint64_t field_index, uint64_t offset,
+              uint64_t leftrowidx, uint64_t rightrowidx) {
+    std::shared_ptr<Array> field = batch.column(static_cast<int>(field_index));
+    return comparators_[field_index]->Compare(*field, offset, leftrowidx, rightrowidx);
+  }
 
-    // RecordBatch Prepare overload
-    Status Prepare(RecordBatch const &batch) {
-      auto fields = batch.columns();
-      for(auto field = fields.begin(); field != fields.end(); field++) {
-        std::shared_ptr<DataType> physical_type = GetPhysicalType((*field)->type());
-        NestedComparatorFactory comparator_factory = NestedComparatorFactory();
-        std::shared_ptr<NestedValueComparator> current_field_comparator;
-        ARROW_ASSIGN_OR_RAISE(
+  // RecordBatch Prepare overload
+  Status Prepare(RecordBatch const& batch) {
+    auto fields = batch.columns();
+    for (auto field = fields.begin(); field != fields.end(); field++) {
+      std::shared_ptr<DataType> physical_type = GetPhysicalType((*field)->type());
+      NestedComparatorFactory comparator_factory = NestedComparatorFactory();
+      std::shared_ptr<NestedValueComparator> current_field_comparator;
+      ARROW_ASSIGN_OR_RAISE(
           current_field_comparator,
-          NestedComparatorFactory().MakeFieldComparator(*physical_type)
-        );
-        comparators_.push_back(current_field_comparator);
-      }
-      return Status::OK();
+          NestedComparatorFactory().MakeFieldComparator(*physical_type));
+      comparators_.push_back(current_field_comparator);
     }
+    return Status::OK();
+  }
 
-  private:
-    struct NestedValueComparator {
-      virtual int Compare(Array const &array, uint64_t offset, 
-                          uint64_t leftidx, uint64_t rightidx) = 0;
-
-      virtual int Compare(ChunkedArray const &array, uint64_t offset, 
-                          uint64_t leftidx, uint64_t rightidx) = 0;
-
-      virtual ~NestedValueComparator() = default;
-    };
-
-    template <typename Type>
-    struct ConcreteNestedValueComparator : NestedValueComparator {
-      using FieldArrayType = typename TypeTraits<Type>::ArrayType;
-      using GetView = GetViewType<Type>;
-
-      virtual int Compare(Array const &array, uint64_t offset, 
-                          uint64_t leftidx, uint64_t rightidx) {
-        const FieldArrayType& values = checked_cast<const FieldArrayType&>(array);
-        auto left_value = GetView::LogicalValue(values.GetView(leftidx - offset));
-        auto right_value = GetView::LogicalValue(values.GetView(rightidx - offset));
-        if (left_value == right_value)
-          return 0;
-        else if (left_value < right_value)
-          return -1;
-        else
-          return 1;
-      }
+ private:
+  struct NestedValueComparator {
+    virtual int Compare(Array const& array, uint64_t offset, uint64_t leftidx,
+                        uint64_t rightidx) = 0;
+
+    virtual int Compare(ChunkedArray const& array, uint64_t offset, uint64_t leftidx,
+                        uint64_t rightidx) = 0;
+
+    virtual ~NestedValueComparator() = default;
+  };
+
+  template <typename Type>
+  struct ConcreteNestedValueComparator : NestedValueComparator {
+    using FieldArrayType = typename TypeTraits<Type>::ArrayType;
+    using GetView = GetViewType<Type>;
+
+    virtual int Compare(Array const& array, uint64_t offset, uint64_t leftidx,
+                        uint64_t rightidx) {
+      const FieldArrayType& values = checked_cast<const FieldArrayType&>(array);
+      auto left_value = GetView::LogicalValue(values.GetView(leftidx - offset));
+      auto right_value = GetView::LogicalValue(values.GetView(rightidx - offset));
+      if (left_value == right_value)
+        return 0;
+      else if (left_value < right_value)
+        return -1;
+      else
+        return 1;
+    }
 
-      virtual int Compare(ChunkedArray const &chunked_array, uint64_t offset, 
-                          uint64_t leftidx, uint64_t rightidx) {
-        using Val = decltype(GetView::LogicalValue(checked_cast<const FieldArrayType&>(chunked_array.chunk(0)).GetView(0)));
-        Val l_value = 0, r_value = 0;
-
-        int n_chunks = chunked_array.num_chunks();
-        for(int i = 0; i < n_chunks; i++) {
-          auto chunk = chunked_array.chunk(i);
-          if(leftidx < static_cast<uint64_t>(chunk->length())) {
-            const FieldArrayType& values = checked_cast<const FieldArrayType&>(*chunk);
-            l_value = GetView::LogicalValue(values.GetView(leftidx));
-            break;
-          }
-          leftidx -= chunk->length();
+    virtual int Compare(ChunkedArray const& chunked_array, uint64_t offset,
+                        uint64_t leftidx, uint64_t rightidx) {
+      using Val = decltype(GetView::LogicalValue(
+          checked_cast<const FieldArrayType&>(chunked_array.chunk(0)).GetView(0)));
+      Val l_value = 0, r_value = 0;
+
+      int n_chunks = chunked_array.num_chunks();
+      for (int i = 0; i < n_chunks; i++) {
+        auto chunk = chunked_array.chunk(i);
+        if (leftidx < static_cast<uint64_t>(chunk->length())) {
+          const FieldArrayType& values = checked_cast<const FieldArrayType&>(*chunk);
+          l_value = GetView::LogicalValue(values.GetView(leftidx));
+          break;
         }
+        leftidx -= chunk->length();
+      }
 
-        for(int i = 0; i < n_chunks; i++) {
-          auto chunk = chunked_array.chunk(i);
-          if(rightidx < static_cast<uint64_t>(chunk->length())) {
-            const FieldArrayType& values = checked_cast<const FieldArrayType&>(*chunk);
-            r_value = GetView::LogicalValue(values.GetView(rightidx));
-            break;
-          }
-          rightidx -= chunk->length();
+      for (int i = 0; i < n_chunks; i++) {
+        auto chunk = chunked_array.chunk(i);
+        if (rightidx < static_cast<uint64_t>(chunk->length())) {
+          const FieldArrayType& values = checked_cast<const FieldArrayType&>(*chunk);
+          r_value = GetView::LogicalValue(values.GetView(rightidx));
+          break;
         }
-
-        if (l_value == r_value)
-          return 0;
-        else if (l_value < r_value)
-          return -1;
-        else
-          return 1;
-      }
-    };
-
-    /** 
-     * Internal use factory whose purpose is to detect the right type
-     * of comparator that should be built to be able to compare two
-     * nested values in the same field or column
-     */
-    struct NestedComparatorFactory {
-      Result<std::shared_ptr<NestedValueComparator>> MakeFieldComparator(DataType const &physical_type) {
-          RETURN_NOT_OK(VisitTypeInline(physical_type, this));
-          DCHECK_NE(current_field_comparator_, nullptr);
-          return std::move(current_field_comparator_);
+        rightidx -= chunk->length();
       }
 
-    #define VISIT(TYPE) \
-      Status Visit(const TYPE& type) { return VisitGeneric(type); }
+      if (l_value == r_value)
+        return 0;
+      else if (l_value < r_value)
+        return -1;
+      else
+        return 1;
+    }
+  };
+
+  /**
+   * Internal use factory whose purpose is to detect the right type
+   * of comparator that should be built to be able to compare two
+   * nested values in the same field or column
+   */
+  struct NestedComparatorFactory {
+    Result<std::shared_ptr<NestedValueComparator>> MakeFieldComparator(
+        DataType const& physical_type) {
+      RETURN_NOT_OK(VisitTypeInline(physical_type, this));
+      DCHECK_NE(current_field_comparator_, nullptr);
+      return std::move(current_field_comparator_);
+    }
 
-        VISIT_SORTABLE_PHYSICAL_TYPES(VISIT)
-    #undef VISIT
+#define VISIT(TYPE) \
+  Status Visit(const TYPE& type) { return VisitGeneric(type); }
 
-      Status Visit(const DataType& type) {
-        return Status::TypeError("Unsupported type for NestedComparatorFactory: ",
-                                type.ToString());
-      }
+    VISIT_SORTABLE_PHYSICAL_TYPES(VISIT)
+#undef VISIT
 
-      template <typename Type>
-      Status VisitGeneric(const Type&) {
-        current_field_comparator_ = std::shared_ptr<NestedValueComparator>(
-          new ConcreteNestedValueComparator<Type>()
-        );
-        return Status::OK();
-      }
+    Status Visit(const DataType& type) {
+      return Status::TypeError("Unsupported type for NestedComparatorFactory: ",
+                               type.ToString());
+    }
+
+    template <typename Type>
+    Status VisitGeneric(const Type&) {
+      current_field_comparator_ = std::shared_ptr<NestedValueComparator>(
+          new ConcreteNestedValueComparator<Type>());
+      return Status::OK();
+    }
 
-      std::shared_ptr<NestedValueComparator> current_field_comparator_;
-    };
+    std::shared_ptr<NestedValueComparator> current_field_comparator_;
+  };
 
   std::vector<std::shared_ptr<NestedValueComparator>> comparators_;
 };
diff --git a/cpp/src/arrow/compute/kernels/vector_sort_test.cc b/cpp/src/arrow/compute/kernels/vector_sort_test.cc
index 9f2bace7fcc..924c2bb201f 100644
--- a/cpp/src/arrow/compute/kernels/vector_sort_test.cc
+++ b/cpp/src/arrow/compute/kernels/vector_sort_test.cc
@@ -30,6 +30,7 @@
 #include "arrow/array/concatenate.h"
 #include "arrow/compute/api_vector.h"
 #include "arrow/compute/kernels/test_util.h"
+#include "arrow/compute/kernels/vector_sort_internal.h"
 #include "arrow/result.h"
 #include "arrow/table.h"
 #include "arrow/testing/gtest_common.h"
@@ -38,8 +39,6 @@
 #include "arrow/testing/util.h"
 #include "arrow/type_traits.h"
 #include "arrow/util/logging.h"
-#include "arrow/compute/kernels/vector_sort_internal.h"
-
 
 namespace arrow {
 
@@ -1965,12 +1964,10 @@ TEST_F(TestNestedValuesComparator, Table) {
 }
 
 TEST_F(TestNestedValuesComparator, StructArray) {
-  std::shared_ptr<StructArray> expected = std::dynamic_pointer_cast<StructArray>(
-    ArrayFromJSON(
-      struct_({field("a", int32()), field("b", utf8()), field("c", int32())}),
-      R"([{"a": 4, "b": "bar", "c": 4}, {"a": 3, "b": "foo", "c": 4}])"
-    )
-  );
+  std::shared_ptr<StructArray> expected =
+      std::dynamic_pointer_cast<StructArray>(ArrayFromJSON(
+          struct_({field("a", int32()), field("b", utf8()), field("c", int32())}),
+          R"([{"a": 4, "b": "bar", "c": 4}, {"a": 3, "b": "foo", "c": 4}])"));
 
   auto c = internal::NestedValuesComparator();
   ASSERT_OK(c.Prepare(*expected));

From 69d27feb2f8e6f5c428d9048269b2259b5d1de96 Mon Sep 17 00:00:00 2001
From: Jedi18 <akhiljnair.188@gmail.com>
Date: Sun, 9 Jan 2022 19:12:28 +0530
Subject: [PATCH 14/24] modify chunked arrayi of struct sorting

---
 .../arrow/compute/kernels/chunked_internal.h  | 98 +++++++++++++++++--
 cpp/src/arrow/compute/kernels/vector_sort.cc  | 14 +--
 python/pyarrow/tests/test_array.py            | 32 ++++++
 3 files changed, 128 insertions(+), 16 deletions(-)

diff --git a/cpp/src/arrow/compute/kernels/chunked_internal.h b/cpp/src/arrow/compute/kernels/chunked_internal.h
index 30233e68319..a47efcb5f7e 100644
--- a/cpp/src/arrow/compute/kernels/chunked_internal.h
+++ b/cpp/src/arrow/compute/kernels/chunked_internal.h
@@ -30,6 +30,25 @@ namespace arrow {
 namespace compute {
 namespace internal {
 
+// Visit all physical types for which sorting is implemented.
+#define VISIT_SORTABLE_PHYSICAL_TYPES(VISIT) \
+  VISIT(BooleanType)                         \
+  VISIT(Int8Type)                            \
+  VISIT(Int16Type)                           \
+  VISIT(Int32Type)                           \
+  VISIT(Int64Type)                           \
+  VISIT(UInt8Type)                           \
+  VISIT(UInt16Type)                          \
+  VISIT(UInt32Type)                          \
+  VISIT(UInt64Type)                          \
+  VISIT(FloatType)                           \
+  VISIT(DoubleType)                          \
+  VISIT(BinaryType)                          \
+  VISIT(LargeBinaryType)                     \
+  VISIT(FixedSizeBinaryType)                 \
+  VISIT(Decimal128Type)                      \
+  VISIT(Decimal256Type)
+
 // The target chunk in a chunked array.
 template <typename ArrayType>
 struct ResolvedChunk {
@@ -59,13 +78,6 @@ struct ResolvedChunk<StructArray> {
   ResolvedChunk(const StructArray* array, int64_t index) : array(array), index(index) {}
 
   bool IsNull() const { return array->field(0)->IsNull(index); }
-
-  int64_t Value() const {
-    auto t = array->field(0)->type();
-    auto arr = std::static_pointer_cast<arrow::Int64Array>(array->field(0));
-
-    return arr->Value(index);
-  }
 };
 
 // ResolvedChunk specialization for untyped arrays when all is needed is null lookup
@@ -182,6 +194,78 @@ inline std::vector<const Array*> GetArrayPointers(const ArrayVector& arrays) {
   return pointers;
 }
 
+template <typename ArrayType>
+struct ResolvedChunkComparator {
+  bool Compare(const ResolvedChunk<ArrayType>& chunk_left,
+               const ResolvedChunk<ArrayType>& chunk_right) const {
+    return chunk_left.Value() < chunk_right.Value();
+  }
+
+  bool CompareChunks(const ChunkedArrayResolver& left_resolver,
+                     const ChunkedArrayResolver& right_resolver, uint64_t left,
+                     uint64_t right) {
+    const auto chunk_left = left_resolver.Resolve<ArrayType>(left);
+    const auto chunk_right = right_resolver.Resolve<ArrayType>(right);
+    return Compare(chunk_left, chunk_right);
+  }
+};
+
+template <>
+struct ResolvedChunkComparator<StructArray> {
+  bool CompareChunks(const ChunkedArrayResolver& left_resolver,
+                     const ChunkedArrayResolver& right_resolver, uint64_t left,
+                     uint64_t right) {
+    const auto chunk_left = left_resolver.Resolve<StructArray>(left);
+    const auto chunk_right = right_resolver.Resolve<StructArray>(right);
+    return Compare(chunk_left, chunk_right);
+  }
+
+  bool Compare(const ResolvedChunk<StructArray>& chunk_left,
+               const ResolvedChunk<StructArray>& chunk_right) const {
+    std::shared_ptr<DataType> field_type = chunk_left.array->field(0)->type();
+    ValComparatorVisitor type_visitor;
+    auto vCompare = type_visitor.Create(*field_type);
+    return vCompare(chunk_left.array->field(0), chunk_left.index,
+                    chunk_right.array->field(0), chunk_right.index);
+  }
+
+  using ValComparator = std::function<bool(std::shared_ptr<arrow::Array>, int64_t,
+                                           std::shared_ptr<arrow::Array>, int64_t)>;
+
+  struct ValComparatorVisitor {
+#define VISIT(TYPE) \
+  Status Visit(const TYPE& type) { return VisitGeneric(type); }
+
+    VISIT_SORTABLE_PHYSICAL_TYPES(VISIT)
+#undef VISIT
+
+    Status Visit(const DataType& type) {
+      return Status::TypeError("Unsupported type for ValComparatorVisitor: ",
+                               type.ToString());
+    }
+
+    template <typename T>
+    Status VisitGeneric(const T&) {
+      using ArrayType = typename TypeTraits<T>::ArrayType;
+
+      out = [](std::shared_ptr<arrow::Array> base, int64_t base_index,
+               std::shared_ptr<arrow::Array> target, int64_t target_index) {
+        auto base_arr = arrow::internal::checked_pointer_cast<ArrayType>(base);
+        auto target_arr = arrow::internal::checked_pointer_cast<ArrayType>(target);
+        return base_arr->Value(base_index) < target_arr->Value(target_index);
+      };
+      return Status::OK();
+    }
+
+    ValComparator Create(const DataType& type) {
+      DCHECK_OK(VisitTypeInline(type, this));
+      return out;
+    }
+
+    ValComparator out;
+  };
+};
+
 }  // namespace internal
 }  // namespace compute
 }  // namespace arrow
diff --git a/cpp/src/arrow/compute/kernels/vector_sort.cc b/cpp/src/arrow/compute/kernels/vector_sort.cc
index 373bc0673bc..9b6f2b02846 100644
--- a/cpp/src/arrow/compute/kernels/vector_sort.cc
+++ b/cpp/src/arrow/compute/kernels/vector_sort.cc
@@ -274,23 +274,19 @@ class ChunkedArraySorter : public TypeVisitor {
                      const std::vector<const Array*>& arrays, uint64_t* temp_indices) {
     const ChunkedArrayResolver left_resolver(arrays);
     const ChunkedArrayResolver right_resolver(arrays);
+    ResolvedChunkComparator<ArrayType> resolved_chunk_comparator;
 
     if (order_ == SortOrder::Ascending) {
       std::merge(range_begin, range_middle, range_middle, range_end, temp_indices,
                  [&](uint64_t left, uint64_t right) {
-                   const auto chunk_left = left_resolver.Resolve<ArrayType>(left);
-                   const auto chunk_right = right_resolver.Resolve<ArrayType>(right);
-                   return chunk_left.Value() < chunk_right.Value();
+                   return resolved_chunk_comparator.CompareChunks(
+                       left_resolver, right_resolver, left, right);
                  });
     } else {
       std::merge(range_begin, range_middle, range_middle, range_end, temp_indices,
                  [&](uint64_t left, uint64_t right) {
-                   const auto chunk_left = left_resolver.Resolve<ArrayType>(left);
-                   const auto chunk_right = right_resolver.Resolve<ArrayType>(right);
-                   // We don't use 'left > right' here to reduce required
-                   // operator. If we use 'right < left' here, '<' is only
-                   // required.
-                   return chunk_right.Value() < chunk_left.Value();
+                   return resolved_chunk_comparator.CompareChunks(
+                       right_resolver, left_resolver, right, left);
                  });
     }
     // Copy back temp area into main buffer
diff --git a/python/pyarrow/tests/test_array.py b/python/pyarrow/tests/test_array.py
index c4b2abec433..ac4d80ffad2 100644
--- a/python/pyarrow/tests/test_array.py
+++ b/python/pyarrow/tests/test_array.py
@@ -798,6 +798,38 @@ def test_struct_chunked_array_sort():
         {"a": 35, "b": "foobar"},
     ]
 
+    arr1 = pa.StructArray.from_arrays([
+        pa.array(["foo", "car", "tar"]),
+        pa.array([5, 7, 8], type=pa.int64())
+    ], names=["a", "b"])
+
+    arr2 = pa.StructArray.from_arrays([
+        pa.array(["bar", "foobar", "far"]),
+        pa.array([7, 35, 9], type=pa.int64())
+    ], names=["a", "b"])
+
+    chunked_arr = pa.chunked_array([arr1, arr2])
+
+    sorted_arr = chunked_arr.sort("ascending")
+    assert sorted_arr.to_pylist() == [
+        {"a": "bar", "b": 7},
+        {"a": "car", "b": 7},
+        {"a": "far", "b": 9},
+        {"a": "foo", "b": 5},
+        {"a": "foobar", "b": 35},
+        {"a": "tar", "b": 8},
+    ]
+
+    sorted_arr = chunked_arr.sort("descending")
+    assert sorted_arr.to_pylist() == [
+        {"a": "tar", "b": 8},
+        {"a": "foobar", "b": 35},
+        {"a": "foo", "b": 5},
+        {"a": "far", "b": 9},
+        {"a": "car", "b": 7},
+        {"a": "bar", "b": 7},
+    ]
+
 
 def test_record_batch_sort():
     rb = pa.RecordBatch.from_arrays([

From 109c2f47f0f52b9aa0c27702ac8de6724a37797d Mon Sep 17 00:00:00 2001
From: Jedi18 <akhiljnair.188@gmail.com>
Date: Sun, 23 Jan 2022 17:04:28 +0530
Subject: [PATCH 15/24] remove internal tests for nestedvaluescomparator

---
 .../compute/kernels/vector_sort_internal.h    |  2 +
 .../arrow/compute/kernels/vector_sort_test.cc | 76 -------------------
 2 files changed, 2 insertions(+), 76 deletions(-)

diff --git a/cpp/src/arrow/compute/kernels/vector_sort_internal.h b/cpp/src/arrow/compute/kernels/vector_sort_internal.h
index 7985c955648..acc32d6b424 100644
--- a/cpp/src/arrow/compute/kernels/vector_sort_internal.h
+++ b/cpp/src/arrow/compute/kernels/vector_sort_internal.h
@@ -457,6 +457,8 @@ Status SortChunkedArray(ExecContext* ctx, uint64_t* indices_begin, uint64_t* ind
                         const ChunkedArray& values, SortOrder sort_order,
                         NullPlacement null_placement);
 
+// Common Comparator for all DataStructures with NestedValues like StructArray,
+// ChunkedArray, Table and RecordBatch
 class NestedValuesComparator {
  public:
   // StructArray Compare overload
diff --git a/cpp/src/arrow/compute/kernels/vector_sort_test.cc b/cpp/src/arrow/compute/kernels/vector_sort_test.cc
index 57deac49a92..50add1aec41 100644
--- a/cpp/src/arrow/compute/kernels/vector_sort_test.cc
+++ b/cpp/src/arrow/compute/kernels/vector_sort_test.cc
@@ -30,7 +30,6 @@
 #include "arrow/array/concatenate.h"
 #include "arrow/compute/api_vector.h"
 #include "arrow/compute/kernels/test_util.h"
-#include "arrow/compute/kernels/vector_sort_internal.h"
 #include "arrow/result.h"
 #include "arrow/table.h"
 #include "arrow/testing/gtest_util.h"
@@ -1956,80 +1955,5 @@ INSTANTIATE_TEST_SUITE_P(AllNull, TestTableSortIndicesRandom,
                          testing::Combine(first_sort_keys, num_sort_keys,
                                           testing::Values(1.0)));
 
-// Test basic cases for table.
-class TestNestedValuesComparator : public ::testing::Test {};
-
-TEST_F(TestNestedValuesComparator, Table) {
-  auto schema = ::arrow::schema({
-      {field("a", uint8())},
-      {field("b", uint32())},
-  });
-  std::shared_ptr<Table> table;
-
-  table = TableFromJSON(schema, {R"([{"a": null, "b": 5},
-                                     {"a": 1,    "b": 3},
-                                     {"a": 3,    "b": null},
-                                     {"a": null, "b": null},
-                                     {"a": 2,    "b": 5},
-                                     {"a": 5,    "b": 2},
-                                     {"a": 3,    "b": 2}
-                                    ])"});
-
-  auto c = internal::NestedValuesComparator();
-  ASSERT_OK(c.Prepare(*table));
-
-  ASSERT_EQ(c.Compare(*table, 0, 0, 0, 1), -1);
-  ASSERT_EQ(c.Compare(*table, 0, 0, 2, 3), 1);
-  ASSERT_EQ(c.Compare(*table, 1, 0, 2, 3), 0);
-  ASSERT_EQ(c.Compare(*table, 0, 0, 1, 2), -1);
-  ASSERT_EQ(c.Compare(*table, 0, 0, 0, 2), -1);
-  ASSERT_EQ(c.Compare(*table, 0, 0, 5, 6), 1);
-  ASSERT_EQ(c.Compare(*table, 1, 0, 5, 6), 0);
-  ASSERT_EQ(c.Compare(*table, 0, 0, 5, 1), 1);
-  ASSERT_EQ(c.Compare(*table, 1, 0, 5, 4), -1);
-}
-
-TEST_F(TestNestedValuesComparator, StructArray) {
-  std::shared_ptr<StructArray> expected =
-      std::dynamic_pointer_cast<StructArray>(ArrayFromJSON(
-          struct_({field("a", int32()), field("b", utf8()), field("c", int32())}),
-          R"([{"a": 4, "b": "bar", "c": 4}, {"a": 3, "b": "foo", "c": 4}])"));
-
-  auto c = internal::NestedValuesComparator();
-  ASSERT_OK(c.Prepare(*expected));
-  ASSERT_EQ(c.Compare(*expected, 0, 0, 0, 1), 1);
-  ASSERT_EQ(c.Compare(*expected, 1, 0, 0, 1), -1);
-  ASSERT_EQ(c.Compare(*expected, 2, 0, 0, 1), 0);
-}
-
-TEST_F(TestNestedValuesComparator, RecordBatch) {
-  auto schema = ::arrow::schema({
-      {field("a", uint8())},
-      {field("b", uint32())},
-  });
-
-  auto batch = RecordBatchFromJSON(schema, {R"([{"a": null, "b": 5},
-                                     {"a": 1,    "b": 3},
-                                     {"a": 3,    "b": null},
-                                     {"a": null, "b": null},
-                                     {"a": 2,    "b": 5},
-                                     {"a": 5,    "b": 2},
-                                     {"a": 3,    "b": 2}
-                                    ])"});
-
-  auto c = internal::NestedValuesComparator();
-  ASSERT_OK(c.Prepare(*batch));
-
-  ASSERT_EQ(c.Compare(*batch, 0, 0, 0, 1), -1);
-  ASSERT_EQ(c.Compare(*batch, 0, 0, 2, 3), 1);
-  ASSERT_EQ(c.Compare(*batch, 1, 0, 2, 3), 0);
-  ASSERT_EQ(c.Compare(*batch, 0, 0, 1, 2), -1);
-  ASSERT_EQ(c.Compare(*batch, 0, 0, 0, 2), -1);
-  ASSERT_EQ(c.Compare(*batch, 0, 0, 5, 6), 1);
-  ASSERT_EQ(c.Compare(*batch, 1, 0, 5, 6), 0);
-  ASSERT_EQ(c.Compare(*batch, 0, 0, 5, 1), 1);
-  ASSERT_EQ(c.Compare(*batch, 1, 0, 5, 4), -1);
-}
-
 }  // namespace compute
 }  // namespace arrow

From 751cb88ed5617946d4193fd822c3b2b4d5adf280 Mon Sep 17 00:00:00 2001
From: Jedi18 <akhiljnair.188@gmail.com>
Date: Mon, 24 Jan 2022 10:48:08 +0530
Subject: [PATCH 16/24] update recordbatch to use nestedvalueocmparator

---
 cpp/src/arrow/compute/kernels/vector_sort.cc | 47 ++++++++++++--------
 1 file changed, 28 insertions(+), 19 deletions(-)

diff --git a/cpp/src/arrow/compute/kernels/vector_sort.cc b/cpp/src/arrow/compute/kernels/vector_sort.cc
index 9b6f2b02846..4639385e085 100644
--- a/cpp/src/arrow/compute/kernels/vector_sort.cc
+++ b/cpp/src/arrow/compute/kernels/vector_sort.cc
@@ -353,19 +353,23 @@ class ConcreteRecordBatchColumnSorter : public RecordBatchColumnSorter {
  public:
   using ArrayType = typename TypeTraits<Type>::ArrayType;
 
-  ConcreteRecordBatchColumnSorter(std::shared_ptr<Array> array, SortOrder order,
-                                  NullPlacement null_placement,
+  ConcreteRecordBatchColumnSorter(const RecordBatch& batch, std::shared_ptr<Array> array, SortOrder order,
+                                  NullPlacement null_placement, uint64_t field_index,
+                                  std::shared_ptr<NestedValuesComparator> nested_values_comparator,
                                   RecordBatchColumnSorter* next_column = nullptr)
       : RecordBatchColumnSorter(next_column),
+        batch_(batch),
         owned_array_(std::move(array)),
+        nested_values_comparator_(std::move(nested_values_comparator)),
         array_(checked_cast<const ArrayType&>(*owned_array_)),
         order_(order),
         null_placement_(null_placement),
-        null_count_(array_.null_count()) {}
+        null_count_(array_.null_count()),
+        field_index_(field_index) {}
 
   NullPartitionResult SortRange(uint64_t* indices_begin, uint64_t* indices_end,
                                 int64_t offset) override {
-    using GetView = GetViewType<Type>;
+    //using GetView = GetViewType<Type>;
 
     NullPartitionResult p;
     if (null_count_ == 0) {
@@ -386,18 +390,12 @@ class ConcreteRecordBatchColumnSorter : public RecordBatchColumnSorter {
     if (order_ == SortOrder::Ascending) {
       std::stable_sort(
           q.non_nulls_begin, q.non_nulls_end, [&](uint64_t left, uint64_t right) {
-            const auto lhs = GetView::LogicalValue(array_.GetView(left - offset));
-            const auto rhs = GetView::LogicalValue(array_.GetView(right - offset));
-            return lhs < rhs;
+            return nested_values_comparator_->Compare(batch_, field_index_, offset, left, right);
           });
     } else {
       std::stable_sort(
           q.non_nulls_begin, q.non_nulls_end, [&](uint64_t left, uint64_t right) {
-            // We don't use 'left > right' here to reduce required operator.
-            // If we use 'right < left' here, '<' is only required.
-            const auto lhs = GetView::LogicalValue(array_.GetView(left - offset));
-            const auto rhs = GetView::LogicalValue(array_.GetView(right - offset));
-            return lhs > rhs;
+            return nested_values_comparator_->Compare(batch_, field_index_, offset, right, left);
           });
     }
 
@@ -424,18 +422,22 @@ class ConcreteRecordBatchColumnSorter : public RecordBatchColumnSorter {
   }
 
  protected:
+  const RecordBatch& batch_;
   const std::shared_ptr<Array> owned_array_;
+  const std::shared_ptr<NestedValuesComparator> nested_values_comparator_;
   const ArrayType& array_;
   const SortOrder order_;
   const NullPlacement null_placement_;
   const int64_t null_count_;
+  const int64_t field_index_;
 };
 
 template <>
 class ConcreteRecordBatchColumnSorter<NullType> : public RecordBatchColumnSorter {
  public:
-  ConcreteRecordBatchColumnSorter(std::shared_ptr<Array> array, SortOrder order,
-                                  NullPlacement null_placement,
+  ConcreteRecordBatchColumnSorter(const RecordBatch& batch, std::shared_ptr<Array> array, SortOrder order,
+                                  NullPlacement null_placement, uint64_t field_index,
+                                  std::shared_ptr<NestedValuesComparator> nested_values_comparator,
                                   RecordBatchColumnSorter* next_column = nullptr)
       : RecordBatchColumnSorter(next_column), null_placement_(null_placement) {}
 
@@ -470,7 +472,7 @@ class RadixRecordBatchSorter {
     std::vector<std::unique_ptr<RecordBatchColumnSorter>> column_sorts(sort_keys.size());
     RecordBatchColumnSorter* next_column = nullptr;
     for (int64_t i = static_cast<int64_t>(sort_keys.size() - 1); i >= 0; --i) {
-      ColumnSortFactory factory(sort_keys[i], options_, next_column);
+      ColumnSortFactory factory(batch_, sort_keys[i], options_, next_column);
       ARROW_ASSIGN_OR_RAISE(column_sorts[i], factory.MakeColumnSort());
       next_column = column_sorts[i].get();
     }
@@ -486,13 +488,18 @@ class RadixRecordBatchSorter {
   };
 
   struct ColumnSortFactory {
-    ColumnSortFactory(const ResolvedSortKey& sort_key, const SortOptions& options,
+    ColumnSortFactory(const RecordBatch& batch, const ResolvedSortKey& sort_key, const SortOptions& options,
                       RecordBatchColumnSorter* next_column)
-        : physical_type(GetPhysicalType(sort_key.array->type())),
+        : batch_(batch),
+          physical_type(GetPhysicalType(sort_key.array->type())),
           array(GetPhysicalArray(*sort_key.array, physical_type)),
           order(sort_key.order),
           null_placement(options.null_placement),
-          next_column(next_column) {}
+          next_column(next_column),
+          nested_values_comparator_(nullptr) {
+            nested_values_comparator_ = std::make_shared<NestedValuesComparator>();
+            nested_values_comparator_->Prepare(batch);
+          }
 
     Result<std::unique_ptr<RecordBatchColumnSorter>> MakeColumnSort() {
       RETURN_NOT_OK(VisitTypeInline(*physical_type, this));
@@ -515,17 +522,19 @@ class RadixRecordBatchSorter {
 
     template <typename Type>
     Status VisitGeneric(const Type&) {
-      result.reset(new ConcreteRecordBatchColumnSorter<Type>(array, order, null_placement,
+      result.reset(new ConcreteRecordBatchColumnSorter<Type>(batch_, array, order, null_placement, 0, nested_values_comparator_,
                                                              next_column));
       return Status::OK();
     }
 
+    const RecordBatch& batch_;
     std::shared_ptr<DataType> physical_type;
     std::shared_ptr<Array> array;
     SortOrder order;
     NullPlacement null_placement;
     RecordBatchColumnSorter* next_column;
     std::unique_ptr<RecordBatchColumnSorter> result;
+    std::shared_ptr<NestedValuesComparator> nested_values_comparator_;
   };
 
   static Result<std::vector<ResolvedSortKey>> ResolveSortKeys(

From 956875944b82143646ad78a71e250a7092075e45 Mon Sep 17 00:00:00 2001
From: Jedi18 <akhiljnair.188@gmail.com>
Date: Tue, 25 Jan 2022 09:40:51 +0530
Subject: [PATCH 17/24] radix record batch sorter working with
 nestedvaluescomparator

---
 cpp/src/arrow/compute/kernels/vector_sort.cc | 70 +++++++++++---------
 python/pyarrow/tests/test_array.py           | 15 +++--
 2 files changed, 48 insertions(+), 37 deletions(-)

diff --git a/cpp/src/arrow/compute/kernels/vector_sort.cc b/cpp/src/arrow/compute/kernels/vector_sort.cc
index 4639385e085..4651c7c250a 100644
--- a/cpp/src/arrow/compute/kernels/vector_sort.cc
+++ b/cpp/src/arrow/compute/kernels/vector_sort.cc
@@ -99,10 +99,10 @@ Result<std::vector<ResolvedSortKey>> ResolveSortKeys(
 template <typename ResolvedSortKey, typename TableOrBatch>
 Result<std::vector<ResolvedSortKey>> ResolveSortKeys(
     const TableOrBatch& table_or_batch, const std::vector<SortKey>& sort_keys) {
-  return ResolveSortKeys<ResolvedSortKey>(
-      *table_or_batch.schema(), sort_keys, [&](const SortField& f) {
-        return ResolvedSortKey{table_or_batch.column(f.field_index), f.order};
-      });
+  return ResolveSortKeys<
+      ResolvedSortKey>(*table_or_batch.schema(), sort_keys, [&](const SortField& f) {
+    return ResolvedSortKey{table_or_batch.column(f.field_index), f.order, f.field_index};
+  });
 }
 
 // Returns nullptr if no column matching `ref` is found, or if the FieldRef is
@@ -353,10 +353,11 @@ class ConcreteRecordBatchColumnSorter : public RecordBatchColumnSorter {
  public:
   using ArrayType = typename TypeTraits<Type>::ArrayType;
 
-  ConcreteRecordBatchColumnSorter(const RecordBatch& batch, std::shared_ptr<Array> array, SortOrder order,
-                                  NullPlacement null_placement, uint64_t field_index,
-                                  std::shared_ptr<NestedValuesComparator> nested_values_comparator,
-                                  RecordBatchColumnSorter* next_column = nullptr)
+  ConcreteRecordBatchColumnSorter(
+      const RecordBatch& batch, std::shared_ptr<Array> array, SortOrder order,
+      NullPlacement null_placement, uint64_t field_index,
+      std::shared_ptr<NestedValuesComparator> nested_values_comparator,
+      RecordBatchColumnSorter* next_column = nullptr)
       : RecordBatchColumnSorter(next_column),
         batch_(batch),
         owned_array_(std::move(array)),
@@ -369,8 +370,6 @@ class ConcreteRecordBatchColumnSorter : public RecordBatchColumnSorter {
 
   NullPartitionResult SortRange(uint64_t* indices_begin, uint64_t* indices_end,
                                 int64_t offset) override {
-    //using GetView = GetViewType<Type>;
-
     NullPartitionResult p;
     if (null_count_ == 0) {
       p = NullPartitionResult::NoNulls(indices_begin, indices_end, null_placement_);
@@ -388,15 +387,17 @@ class ConcreteRecordBatchColumnSorter : public RecordBatchColumnSorter {
     // Also, we would like to use a counting sort if possible.  This requires
     // a counting sort compatible with indirect indexing.
     if (order_ == SortOrder::Ascending) {
-      std::stable_sort(
-          q.non_nulls_begin, q.non_nulls_end, [&](uint64_t left, uint64_t right) {
-            return nested_values_comparator_->Compare(batch_, field_index_, offset, left, right);
-          });
+      std::stable_sort(q.non_nulls_begin, q.non_nulls_end,
+                       [&](uint64_t left, uint64_t right) {
+                         return nested_values_comparator_->Compare(
+                                    batch_, field_index_, offset, left, right) == -1;
+                       });
     } else {
-      std::stable_sort(
-          q.non_nulls_begin, q.non_nulls_end, [&](uint64_t left, uint64_t right) {
-            return nested_values_comparator_->Compare(batch_, field_index_, offset, right, left);
-          });
+      std::stable_sort(q.non_nulls_begin, q.non_nulls_end,
+                       [&](uint64_t left, uint64_t right) {
+                         return nested_values_comparator_->Compare(
+                                    batch_, field_index_, offset, right, left) == -1;
+                       });
     }
 
     if (next_column_ != nullptr) {
@@ -435,10 +436,11 @@ class ConcreteRecordBatchColumnSorter : public RecordBatchColumnSorter {
 template <>
 class ConcreteRecordBatchColumnSorter<NullType> : public RecordBatchColumnSorter {
  public:
-  ConcreteRecordBatchColumnSorter(const RecordBatch& batch, std::shared_ptr<Array> array, SortOrder order,
-                                  NullPlacement null_placement, uint64_t field_index,
-                                  std::shared_ptr<NestedValuesComparator> nested_values_comparator,
-                                  RecordBatchColumnSorter* next_column = nullptr)
+  ConcreteRecordBatchColumnSorter(
+      const RecordBatch& batch, std::shared_ptr<Array> array, SortOrder order,
+      NullPlacement null_placement, uint64_t field_index,
+      std::shared_ptr<NestedValuesComparator> nested_values_comparator,
+      RecordBatchColumnSorter* next_column = nullptr)
       : RecordBatchColumnSorter(next_column), null_placement_(null_placement) {}
 
   NullPartitionResult SortRange(uint64_t* indices_begin, uint64_t* indices_end,
@@ -485,21 +487,23 @@ class RadixRecordBatchSorter {
   struct ResolvedSortKey {
     std::shared_ptr<Array> array;
     SortOrder order;
+    int field_index;
   };
 
   struct ColumnSortFactory {
-    ColumnSortFactory(const RecordBatch& batch, const ResolvedSortKey& sort_key, const SortOptions& options,
-                      RecordBatchColumnSorter* next_column)
+    ColumnSortFactory(const RecordBatch& batch, const ResolvedSortKey& sort_key,
+                      const SortOptions& options, RecordBatchColumnSorter* next_column)
         : batch_(batch),
           physical_type(GetPhysicalType(sort_key.array->type())),
           array(GetPhysicalArray(*sort_key.array, physical_type)),
           order(sort_key.order),
           null_placement(options.null_placement),
           next_column(next_column),
+          field_index_(sort_key.field_index),
           nested_values_comparator_(nullptr) {
-            nested_values_comparator_ = std::make_shared<NestedValuesComparator>();
-            nested_values_comparator_->Prepare(batch);
-          }
+      nested_values_comparator_ = std::make_shared<NestedValuesComparator>();
+      nested_values_comparator_->Prepare(batch);
+    }
 
     Result<std::unique_ptr<RecordBatchColumnSorter>> MakeColumnSort() {
       RETURN_NOT_OK(VisitTypeInline(*physical_type, this));
@@ -522,8 +526,9 @@ class RadixRecordBatchSorter {
 
     template <typename Type>
     Status VisitGeneric(const Type&) {
-      result.reset(new ConcreteRecordBatchColumnSorter<Type>(batch_, array, order, null_placement, 0, nested_values_comparator_,
-                                                             next_column));
+      result.reset(new ConcreteRecordBatchColumnSorter<Type>(
+          batch_, array, order, null_placement, field_index_, nested_values_comparator_,
+          next_column));
       return Status::OK();
     }
 
@@ -533,6 +538,7 @@ class RadixRecordBatchSorter {
     SortOrder order;
     NullPlacement null_placement;
     RecordBatchColumnSorter* next_column;
+    int64_t field_index_;
     std::unique_ptr<RecordBatchColumnSorter> result;
     std::shared_ptr<NestedValuesComparator> nested_values_comparator_;
   };
@@ -697,11 +703,12 @@ class MultipleKeyRecordBatchSorter : public TypeVisitor {
  public:
   // Preprocessed sort key.
   struct ResolvedSortKey {
-    ResolvedSortKey(const std::shared_ptr<Array>& array, SortOrder order)
+    ResolvedSortKey(const std::shared_ptr<Array>& array, SortOrder order, int field_index)
         : type(GetPhysicalType(array->type())),
           owned_array(GetPhysicalArray(*array, type)),
           array(*owned_array),
           order(order),
+          field_index(field_index),
           null_count(array->null_count()) {}
 
     using LocationType = int64_t;
@@ -715,6 +722,7 @@ class MultipleKeyRecordBatchSorter : public TypeVisitor {
     std::shared_ptr<Array> owned_array;
     const Array& array;
     SortOrder order;
+    int field_index;
     int64_t null_count;
   };
 
@@ -1600,7 +1608,7 @@ class RecordBatchSelecter : public TypeVisitor {
     std::vector<ResolvedSortKey> resolved;
     for (const auto& key : sort_keys) {
       auto array = key.target.GetOne(batch).ValueOr(nullptr);
-      resolved.emplace_back(array, key.order);
+      resolved.emplace_back(array, key.order, key.target.field_path()->indices()[0]);
     }
     return resolved;
   }
diff --git a/python/pyarrow/tests/test_array.py b/python/pyarrow/tests/test_array.py
index 9c475361d8a..9dba37e0fed 100644
--- a/python/pyarrow/tests/test_array.py
+++ b/python/pyarrow/tests/test_array.py
@@ -833,19 +833,22 @@ def test_struct_chunked_array_sort():
 
 def test_record_batch_sort():
     rb = pa.RecordBatch.from_arrays([
-        pa.array([5, 7, 7, 35], type=pa.int64()),
+        pa.array([7, 35, 7, 5], type=pa.int64()),
+        pa.array([4, 1, 3, 2], type=pa.int64()),
         pa.array(["foo", "car", "bar", "foobar"])
-    ], names=["a", "b"])
+    ], names=["a", "b", "c"])
 
-    sorted_rb = rb.sort_by([("a", "descending")])
+    sorted_rb = rb.sort_by([("a", "descending"), ("b", "descending")])
     sorted_rb_dict = sorted_rb.to_pydict()
     assert sorted_rb_dict["a"] == [35, 7, 7, 5]
-    assert sorted_rb_dict["b"] == ["foobar", "car", "bar", "foo"]
+    assert sorted_rb_dict["b"] == [1, 4, 3, 2]
+    assert sorted_rb_dict["c"] == ["car", "foo", "bar", "foobar"]
 
-    sorted_rb = rb.sort_by([("a", "ascending")])
+    sorted_rb = rb.sort_by([("a", "ascending"), ("b", "ascending")])
     sorted_rb_dict = sorted_rb.to_pydict()
     assert sorted_rb_dict["a"] == [5, 7, 7, 35]
-    assert sorted_rb_dict["b"] == ["foo", "car", "bar", "foobar"]
+    assert sorted_rb_dict["b"] == [2, 3, 4, 1]
+    assert sorted_rb_dict["c"] == ["foobar", "bar", "foo", "car"]
 
 
 def test_table_sort():

From 7a8f2225b39c6bca84c4cbb8e2ffc95fdfe653ac Mon Sep 17 00:00:00 2001
From: Jedi18 <akhiljnair.188@gmail.com>
Date: Tue, 25 Jan 2022 10:30:37 +0530
Subject: [PATCH 18/24] use nestedvaluescomparator in MultipleKeyRecordSorter
 and add small test

---
 cpp/src/arrow/compute/kernels/vector_sort.cc | 55 +++++++++++---------
 python/pyarrow/tests/test_array.py           | 30 +++++++++++
 2 files changed, 60 insertions(+), 25 deletions(-)

diff --git a/cpp/src/arrow/compute/kernels/vector_sort.cc b/cpp/src/arrow/compute/kernels/vector_sort.cc
index 4651c7c250a..fc3688b6687 100644
--- a/cpp/src/arrow/compute/kernels/vector_sort.cc
+++ b/cpp/src/arrow/compute/kernels/vector_sort.cc
@@ -732,11 +732,14 @@ class MultipleKeyRecordBatchSorter : public TypeVisitor {
  public:
   MultipleKeyRecordBatchSorter(uint64_t* indices_begin, uint64_t* indices_end,
                                const RecordBatch& batch, const SortOptions& options)
-      : indices_begin_(indices_begin),
+      : batch_(batch),
+        indices_begin_(indices_begin),
         indices_end_(indices_end),
         sort_keys_(ResolveSortKeys(batch, options.sort_keys, &status_)),
         null_placement_(options.null_placement),
-        comparator_(sort_keys_, null_placement_) {}
+        comparator_(sort_keys_, null_placement_) {
+    nested_values_comparator_.Prepare(batch);
+  }
 
   // This is optimized for the first sort key. The first sort key sort
   // is processed in this class. The second and following sort keys
@@ -769,34 +772,34 @@ class MultipleKeyRecordBatchSorter : public TypeVisitor {
   template <typename Type>
   enable_if_t<!is_null_type<Type>::value, Status> SortInternal() {
     using ArrayType = typename TypeTraits<Type>::ArrayType;
-    using GetView = GetViewType<Type>;
-
-    auto& comparator = comparator_;
     const auto& first_sort_key = sort_keys_[0];
     const ArrayType& array = checked_cast<const ArrayType&>(first_sort_key.array);
     const auto p = PartitionNullsInternal<Type>(first_sort_key);
 
     // Sort first-key non-nulls
-    std::stable_sort(
-        p.non_nulls_begin, p.non_nulls_end, [&](uint64_t left, uint64_t right) {
-          // Both values are never null nor NaN
-          // (otherwise they've been partitioned away above).
-          const auto value_left = GetView::LogicalValue(array.GetView(left));
-          const auto value_right = GetView::LogicalValue(array.GetView(right));
-          if (value_left != value_right) {
-            bool compared = value_left < value_right;
-            if (first_sort_key.order == SortOrder::Ascending) {
-              return compared;
-            } else {
-              return !compared;
-            }
-          }
-          // If the left value equals to the right value,
-          // we need to compare the second and following
-          // sort keys.
-          return comparator.Compare(left, right, 1);
-        });
-    return comparator_.status();
+    std::stable_sort(p.non_nulls_begin, p.non_nulls_end,
+                     [&](uint64_t left, uint64_t right) {
+                       // Both values are never null nor NaN
+                       -  // (otherwise they've been partitioned away above).
+                           for (const auto& sort_key : sort_keys_) {
+                         int val = nested_values_comparator_.Compare(
+                             batch_, sort_key.field_index, 0, left, right);
+
+                         if (val == 0) {
+                           continue;
+                         }
+
+                         if (sort_key.order == SortOrder::Ascending) {
+                           return val == -1;
+                         } else {
+                           return val == 1;
+                         }
+                       }
+
+                       return true;
+                     });
+
+    return Status::OK();
   }
 
   template <typename Type>
@@ -840,12 +843,14 @@ class MultipleKeyRecordBatchSorter : public TypeVisitor {
     return q;
   }
 
+  const RecordBatch& batch_;
   uint64_t* indices_begin_;
   uint64_t* indices_end_;
   Status status_;
   std::vector<ResolvedSortKey> sort_keys_;
   NullPlacement null_placement_;
   Comparator comparator_;
+  NestedValuesComparator nested_values_comparator_;
 };
 
 // ----------------------------------------------------------------------
diff --git a/python/pyarrow/tests/test_array.py b/python/pyarrow/tests/test_array.py
index 9dba37e0fed..1978b8375e2 100644
--- a/python/pyarrow/tests/test_array.py
+++ b/python/pyarrow/tests/test_array.py
@@ -850,6 +850,36 @@ def test_record_batch_sort():
     assert sorted_rb_dict["b"] == [2, 3, 4, 1]
     assert sorted_rb_dict["c"] == ["foobar", "bar", "foo", "car"]
 
+    # test multi-key record batch sorter (> 8 sort keys)
+    rb1_names = ["a", "b", "c", "d", "e", "f", "g", "h", "i", "j"]
+    rb1 = pa.RecordBatch.from_arrays([
+        pa.array([4, 4, 4, 4], type=pa.int64()),
+        pa.array([4, 4, 4, 4], type=pa.int64()),
+        pa.array([4, 4, 4, 4], type=pa.int64()),
+        pa.array([4, 4, 2, 2], type=pa.int64()),
+        pa.array([4, 4, 2, 2], type=pa.int64()),
+        pa.array([4, 4, 2, 2], type=pa.int64()),
+        pa.array([4, 4, 2, 2], type=pa.int64()),
+        pa.array([4, 4, 2, 2], type=pa.int64()),
+        pa.array([2, 1, 4, 3], type=pa.int64()),
+        pa.array(["foo", "car", "bar", "foobar"])
+    ], names=rb1_names)
+
+    sort_keys_list = [(name, "ascending") for name in rb1_names]
+
+    sorted_rb1 = rb1.sort_by(sort_keys_list)
+    sorted_rb1_dict = sorted_rb1.to_pydict()
+    assert sorted_rb1_dict["a"] == [4, 4, 4, 4]
+    assert sorted_rb1_dict["b"] == [4, 4, 4, 4]
+    assert sorted_rb1_dict["c"] == [4, 4, 4, 4]
+    assert sorted_rb1_dict["d"] == [2, 2, 4, 4]
+    assert sorted_rb1_dict["e"] == [2, 2, 4, 4]
+    assert sorted_rb1_dict["f"] == [2, 2, 4, 4]
+    assert sorted_rb1_dict["g"] == [2, 2, 4, 4]
+    assert sorted_rb1_dict["h"] == [2, 2, 4, 4]
+    assert sorted_rb1_dict["i"] == [3, 4, 1, 2]
+    assert sorted_rb1_dict["j"] == ["foobar", "bar", "car", "foo"]
+
 
 def test_table_sort():
     tab = pa.Table.from_arrays([

From 96a49c1796b5c94f213db87d2588bed7af750bf2 Mon Sep 17 00:00:00 2001
From: Jedi18 <akhiljnair.188@gmail.com>
Date: Sat, 29 Jan 2022 12:39:09 +0530
Subject: [PATCH 19/24] use NestedValuesComparator in
 MultipleKeyRecordBatchSorter

---
 cpp/src/arrow/compute/kernels/vector_sort.cc  | 36 +++++++++++-----
 .../compute/kernels/vector_sort_internal.h    | 43 +++++++++++++++++++
 2 files changed, 68 insertions(+), 11 deletions(-)

diff --git a/cpp/src/arrow/compute/kernels/vector_sort.cc b/cpp/src/arrow/compute/kernels/vector_sort.cc
index fc3688b6687..b8663bf45dd 100644
--- a/cpp/src/arrow/compute/kernels/vector_sort.cc
+++ b/cpp/src/arrow/compute/kernels/vector_sort.cc
@@ -396,7 +396,7 @@ class ConcreteRecordBatchColumnSorter : public RecordBatchColumnSorter {
       std::stable_sort(q.non_nulls_begin, q.non_nulls_end,
                        [&](uint64_t left, uint64_t right) {
                          return nested_values_comparator_->Compare(
-                                    batch_, field_index_, offset, right, left) == -1;
+                                    batch_, field_index_, offset, left, right) == 1;
                        });
     }
 
@@ -773,30 +773,44 @@ class MultipleKeyRecordBatchSorter : public TypeVisitor {
   enable_if_t<!is_null_type<Type>::value, Status> SortInternal() {
     using ArrayType = typename TypeTraits<Type>::ArrayType;
     const auto& first_sort_key = sort_keys_[0];
-    const ArrayType& array = checked_cast<const ArrayType&>(first_sort_key.array);
+    // const ArrayType& array = checked_cast<const ArrayType&>(first_sort_key.array);
     const auto p = PartitionNullsInternal<Type>(first_sort_key);
 
     // Sort first-key non-nulls
     std::stable_sort(p.non_nulls_begin, p.non_nulls_end,
                      [&](uint64_t left, uint64_t right) {
                        // Both values are never null nor NaN
-                       -  // (otherwise they've been partitioned away above).
-                           for (const auto& sort_key : sort_keys_) {
+                       // (otherwise they've been partitioned away above).
+                       int val = nested_values_comparator_.Compare(
+                           batch_, sort_keys_[0].field_index, 0, left, right);
+
+                       if (val != 0) {
+                         // this overload of compare does not take sort order
+                         // so it needs to be handled here
+                         if (sort_keys_[0].order == SortOrder::Ascending) {
+                           return val == -1;
+                         } else {
+                           return val == 1;
+                         }
+                       }
+
+                       auto sort_keys_len = sort_keys_.size();
+                       for (auto i = 1; i < sort_keys_len; i++) {
                          int val = nested_values_comparator_.Compare(
-                             batch_, sort_key.field_index, 0, left, right);
+                             batch_, null_placement_, sort_keys_[i].order,
+                             sort_keys_[i].field_index, 0, left, right);
 
                          if (val == 0) {
                            continue;
                          }
 
-                         if (sort_key.order == SortOrder::Ascending) {
-                           return val == -1;
-                         } else {
-                           return val == 1;
-                         }
+                         // overload of compare taking in sort key order takes care of
+                         // sort order
+                         return val < 0;
                        }
 
-                       return true;
+                       // all column values are equal
+                       return false;
                      });
 
     return Status::OK();
diff --git a/cpp/src/arrow/compute/kernels/vector_sort_internal.h b/cpp/src/arrow/compute/kernels/vector_sort_internal.h
index acc32d6b424..b09c6635ad8 100644
--- a/cpp/src/arrow/compute/kernels/vector_sort_internal.h
+++ b/cpp/src/arrow/compute/kernels/vector_sort_internal.h
@@ -512,6 +512,15 @@ class NestedValuesComparator {
     return comparators_[field_index]->Compare(*field, offset, leftrowidx, rightrowidx);
   }
 
+  // RecordBatch Compare overload
+  int Compare(RecordBatch const& batch, NullPlacement null_placement,
+              SortOrder sort_order, uint64_t field_index, uint64_t offset,
+              uint64_t leftrowidx, uint64_t rightrowidx) {
+    std::shared_ptr<Array> field = batch.column(static_cast<int>(field_index));
+    return comparators_[field_index]->Compare(*field, null_placement, sort_order, offset,
+                                              leftrowidx, rightrowidx);
+  }
+
   // RecordBatch Prepare overload
   Status Prepare(RecordBatch const& batch) {
     auto fields = batch.columns();
@@ -535,6 +544,10 @@ class NestedValuesComparator {
     virtual int Compare(ChunkedArray const& array, uint64_t offset, uint64_t leftidx,
                         uint64_t rightidx) = 0;
 
+    virtual int Compare(Array const& array, NullPlacement null_placement,
+                        SortOrder sort_order, uint64_t offset, uint64_t leftidx,
+                        uint64_t rightidx) = 0;
+
     virtual ~NestedValueComparator() = default;
   };
 
@@ -590,6 +603,36 @@ class NestedValuesComparator {
       else
         return 1;
     }
+
+    virtual int Compare(Array const& array, NullPlacement null_placement,
+                        SortOrder sort_order, uint64_t offset, uint64_t leftidx,
+                        uint64_t rightidx) {
+      // Null values
+      if (array.null_count() > 0) {
+        const bool is_null_left = array.IsNull(leftidx);
+        const bool is_null_right = array.IsNull(rightidx);
+        if (is_null_left && is_null_right) {
+          return 0;
+        } else if (is_null_left) {
+          return null_placement == NullPlacement::AtStart ? -1 : 1;
+        } else if (is_null_right) {
+          return null_placement == NullPlacement::AtStart ? 1 : -1;
+        }
+      }
+
+      const FieldArrayType& values = checked_cast<const FieldArrayType&>(array);
+      auto left_value = GetView::LogicalValue(values.GetView(leftidx - offset));
+      auto right_value = GetView::LogicalValue(values.GetView(rightidx - offset));
+      if (left_value == right_value)
+        return 0;
+      else if (left_value < right_value)
+        return -1;
+      else
+        return 1;
+
+      // Supports NaN values
+      return CompareTypeValues<Type>(left_value, right_value, sort_order, null_placement);
+    }
   };
 
   /**

From 5e93e03fa57d6d61701f593bc2d332c77797e4e4 Mon Sep 17 00:00:00 2001
From: Jedi18 <akhiljnair.188@gmail.com>
Date: Sat, 29 Jan 2022 22:07:10 +0530
Subject: [PATCH 20/24] use nestedvaluescomparator in chunked array struct
 array sorting

---
 .../arrow/compute/kernels/chunked_internal.h  | 72 -------------------
 cpp/src/arrow/compute/kernels/vector_sort.cc  | 15 ++--
 .../compute/kernels/vector_sort_internal.h    | 71 +++++++++++++++++-
 python/pyarrow/tests/test_array.py            | 48 ++++++++++++-
 4 files changed, 120 insertions(+), 86 deletions(-)

diff --git a/cpp/src/arrow/compute/kernels/chunked_internal.h b/cpp/src/arrow/compute/kernels/chunked_internal.h
index a47efcb5f7e..e3f47d84287 100644
--- a/cpp/src/arrow/compute/kernels/chunked_internal.h
+++ b/cpp/src/arrow/compute/kernels/chunked_internal.h
@@ -194,78 +194,6 @@ inline std::vector<const Array*> GetArrayPointers(const ArrayVector& arrays) {
   return pointers;
 }
 
-template <typename ArrayType>
-struct ResolvedChunkComparator {
-  bool Compare(const ResolvedChunk<ArrayType>& chunk_left,
-               const ResolvedChunk<ArrayType>& chunk_right) const {
-    return chunk_left.Value() < chunk_right.Value();
-  }
-
-  bool CompareChunks(const ChunkedArrayResolver& left_resolver,
-                     const ChunkedArrayResolver& right_resolver, uint64_t left,
-                     uint64_t right) {
-    const auto chunk_left = left_resolver.Resolve<ArrayType>(left);
-    const auto chunk_right = right_resolver.Resolve<ArrayType>(right);
-    return Compare(chunk_left, chunk_right);
-  }
-};
-
-template <>
-struct ResolvedChunkComparator<StructArray> {
-  bool CompareChunks(const ChunkedArrayResolver& left_resolver,
-                     const ChunkedArrayResolver& right_resolver, uint64_t left,
-                     uint64_t right) {
-    const auto chunk_left = left_resolver.Resolve<StructArray>(left);
-    const auto chunk_right = right_resolver.Resolve<StructArray>(right);
-    return Compare(chunk_left, chunk_right);
-  }
-
-  bool Compare(const ResolvedChunk<StructArray>& chunk_left,
-               const ResolvedChunk<StructArray>& chunk_right) const {
-    std::shared_ptr<DataType> field_type = chunk_left.array->field(0)->type();
-    ValComparatorVisitor type_visitor;
-    auto vCompare = type_visitor.Create(*field_type);
-    return vCompare(chunk_left.array->field(0), chunk_left.index,
-                    chunk_right.array->field(0), chunk_right.index);
-  }
-
-  using ValComparator = std::function<bool(std::shared_ptr<arrow::Array>, int64_t,
-                                           std::shared_ptr<arrow::Array>, int64_t)>;
-
-  struct ValComparatorVisitor {
-#define VISIT(TYPE) \
-  Status Visit(const TYPE& type) { return VisitGeneric(type); }
-
-    VISIT_SORTABLE_PHYSICAL_TYPES(VISIT)
-#undef VISIT
-
-    Status Visit(const DataType& type) {
-      return Status::TypeError("Unsupported type for ValComparatorVisitor: ",
-                               type.ToString());
-    }
-
-    template <typename T>
-    Status VisitGeneric(const T&) {
-      using ArrayType = typename TypeTraits<T>::ArrayType;
-
-      out = [](std::shared_ptr<arrow::Array> base, int64_t base_index,
-               std::shared_ptr<arrow::Array> target, int64_t target_index) {
-        auto base_arr = arrow::internal::checked_pointer_cast<ArrayType>(base);
-        auto target_arr = arrow::internal::checked_pointer_cast<ArrayType>(target);
-        return base_arr->Value(base_index) < target_arr->Value(target_index);
-      };
-      return Status::OK();
-    }
-
-    ValComparator Create(const DataType& type) {
-      DCHECK_OK(VisitTypeInline(type, this));
-      return out;
-    }
-
-    ValComparator out;
-  };
-};
-
 }  // namespace internal
 }  // namespace compute
 }  // namespace arrow
diff --git a/cpp/src/arrow/compute/kernels/vector_sort.cc b/cpp/src/arrow/compute/kernels/vector_sort.cc
index b8663bf45dd..d5cc76c6ce2 100644
--- a/cpp/src/arrow/compute/kernels/vector_sort.cc
+++ b/cpp/src/arrow/compute/kernels/vector_sort.cc
@@ -272,21 +272,18 @@ class ChunkedArraySorter : public TypeVisitor {
   template <typename ArrayType>
   void MergeNonNulls(uint64_t* range_begin, uint64_t* range_middle, uint64_t* range_end,
                      const std::vector<const Array*>& arrays, uint64_t* temp_indices) {
-    const ChunkedArrayResolver left_resolver(arrays);
-    const ChunkedArrayResolver right_resolver(arrays);
+    const ChunkedArrayResolver chunk_resolver(arrays);
     ResolvedChunkComparator<ArrayType> resolved_chunk_comparator;
 
     if (order_ == SortOrder::Ascending) {
       std::merge(range_begin, range_middle, range_middle, range_end, temp_indices,
                  [&](uint64_t left, uint64_t right) {
-                   return resolved_chunk_comparator.CompareChunks(
-                       left_resolver, right_resolver, left, right);
+                   return resolved_chunk_comparator.Compare(chunk_resolver, left, right);
                  });
     } else {
       std::merge(range_begin, range_middle, range_middle, range_end, temp_indices,
                  [&](uint64_t left, uint64_t right) {
-                   return resolved_chunk_comparator.CompareChunks(
-                       right_resolver, left_resolver, right, left);
+                   return resolved_chunk_comparator.Compare(chunk_resolver, right, left);
                  });
     }
     // Copy back temp area into main buffer
@@ -771,9 +768,7 @@ class MultipleKeyRecordBatchSorter : public TypeVisitor {
 
   template <typename Type>
   enable_if_t<!is_null_type<Type>::value, Status> SortInternal() {
-    using ArrayType = typename TypeTraits<Type>::ArrayType;
     const auto& first_sort_key = sort_keys_[0];
-    // const ArrayType& array = checked_cast<const ArrayType&>(first_sort_key.array);
     const auto p = PartitionNullsInternal<Type>(first_sort_key);
 
     // Sort first-key non-nulls
@@ -794,8 +789,8 @@ class MultipleKeyRecordBatchSorter : public TypeVisitor {
                          }
                        }
 
-                       auto sort_keys_len = sort_keys_.size();
-                       for (auto i = 1; i < sort_keys_len; i++) {
+                       int sort_keys_len = sort_keys_.size();
+                       for (int i = 1; i < sort_keys_len; i++) {
                          int val = nested_values_comparator_.Compare(
                              batch_, null_placement_, sort_keys_[i].order,
                              sort_keys_[i].field_index, 0, left, right);
diff --git a/cpp/src/arrow/compute/kernels/vector_sort_internal.h b/cpp/src/arrow/compute/kernels/vector_sort_internal.h
index b09c6635ad8..0df43c80bd6 100644
--- a/cpp/src/arrow/compute/kernels/vector_sort_internal.h
+++ b/cpp/src/arrow/compute/kernels/vector_sort_internal.h
@@ -21,6 +21,7 @@
 #include <cmath>
 #include <cstdint>
 #include <functional>
+#include <iostream>
 
 #include "arrow/array.h"
 #include "arrow/compute/api_vector.h"
@@ -536,11 +537,32 @@ class NestedValuesComparator {
     return Status::OK();
   }
 
+  // ResolvedChunk<StructArray> Prepare overload
+  Status Prepare(const ResolvedChunk<StructArray>& chunk_left,
+                 const ResolvedChunk<StructArray>& chunk_right) {
+    Prepare(*chunk_left.array);
+    return Status::OK();
+  }
+
+  // ResolvedChunk<StructArray> Compare overload
+  int Compare(const ResolvedChunk<StructArray>& chunk_left,
+              const ResolvedChunk<StructArray>& chunk_right, uint64_t field_index) {
+    std::shared_ptr<Array> field_left =
+        chunk_left.array->field(static_cast<int>(field_index));
+    std::shared_ptr<Array> field_right =
+        chunk_right.array->field(static_cast<int>(field_index));
+    return comparators_[field_index]->Compare(*field_left, *field_right, chunk_left.index,
+                                              chunk_right.index);
+  }
+
  private:
   struct NestedValueComparator {
     virtual int Compare(Array const& array, uint64_t offset, uint64_t leftidx,
                         uint64_t rightidx) = 0;
 
+    virtual int Compare(Array const& left_array, Array const& right_array,
+                        uint64_t leftidx, uint64_t rightidx) = 0;
+
     virtual int Compare(ChunkedArray const& array, uint64_t offset, uint64_t leftidx,
                         uint64_t rightidx) = 0;
 
@@ -623,15 +645,25 @@ class NestedValuesComparator {
       const FieldArrayType& values = checked_cast<const FieldArrayType&>(array);
       auto left_value = GetView::LogicalValue(values.GetView(leftidx - offset));
       auto right_value = GetView::LogicalValue(values.GetView(rightidx - offset));
+
+      // Supports NaN values
+      return CompareTypeValues<Type>(left_value, right_value, sort_order, null_placement);
+    }
+
+    virtual int Compare(Array const& left_array, Array const& right_array,
+                        uint64_t leftidx, uint64_t rightidx) {
+      const FieldArrayType& left_values = checked_cast<const FieldArrayType&>(left_array);
+      const FieldArrayType& right_values =
+          checked_cast<const FieldArrayType&>(right_array);
+      auto left_value = GetView::LogicalValue(left_values.GetView(leftidx));
+      auto right_value = GetView::LogicalValue(right_values.GetView(rightidx));
+
       if (left_value == right_value)
         return 0;
       else if (left_value < right_value)
         return -1;
       else
         return 1;
-
-      // Supports NaN values
-      return CompareTypeValues<Type>(left_value, right_value, sort_order, null_placement);
     }
   };
 
@@ -672,6 +704,39 @@ class NestedValuesComparator {
   std::vector<std::shared_ptr<NestedValueComparator>> comparators_;
 };
 
+template <typename ArrayType>
+struct ResolvedChunkComparator {
+  bool Compare(const ChunkedArrayResolver& chunk_resolver, uint64_t left,
+               uint64_t right) {
+    const auto chunk_left = chunk_resolver.Resolve<ArrayType>(left);
+    const auto chunk_right = chunk_resolver.Resolve<ArrayType>(right);
+    return chunk_left.Value() < chunk_right.Value();
+  }
+};
+
+template <>
+struct ResolvedChunkComparator<StructArray> {
+  bool Compare(const ChunkedArrayResolver& chunk_resolver, uint64_t left,
+               uint64_t right) {
+    const auto chunk_left = chunk_resolver.Resolve<StructArray>(left);
+    const auto chunk_right = chunk_resolver.Resolve<StructArray>(right);
+    NestedValuesComparator nested_values_comparator;
+    nested_values_comparator.Prepare(chunk_left, chunk_right);
+
+    for (int i = 0; i < chunk_left.array->num_fields(); i++) {
+      int val = nested_values_comparator.Compare(chunk_left, chunk_right, i);
+
+      if (val == 0) {
+        continue;
+      }
+
+      return val == -1;
+    }
+
+    return false;
+  }
+};
+
 }  // namespace internal
 }  // namespace compute
 }  // namespace arrow
diff --git a/python/pyarrow/tests/test_array.py b/python/pyarrow/tests/test_array.py
index 1978b8375e2..9eeb265a5d7 100644
--- a/python/pyarrow/tests/test_array.py
+++ b/python/pyarrow/tests/test_array.py
@@ -791,8 +791,8 @@ def test_struct_chunked_array_sort():
     sorted_arr = chunked_arr.sort("ascending")
     assert sorted_arr.to_pylist() == [
         {"a": 5, "b": "foo"},
-        {"a": 7, "b": "car"},
         {"a": 7, "b": "bar"},
+        {"a": 7, "b": "car"},
         {"a": 8, "b": "far"},
         {"a": 9, "b": "tar"},
         {"a": 35, "b": "foobar"},
@@ -830,6 +830,52 @@ def test_struct_chunked_array_sort():
         {"a": "bar", "b": 7},
     ]
 
+    arr1 = pa.StructArray.from_arrays([
+        pa.array([5, 5, 5], type=pa.int64()),
+        pa.array([3, 2, 2], type=pa.int64()),
+        pa.array(["foo", "car", "tar"])
+    ], names=["a", "b", "c"])
+
+    arr2 = pa.StructArray.from_arrays([
+        pa.array([5, 5, 2], type=pa.int64()),
+        pa.array([2, 8, 3], type=pa.int64()),
+        pa.array(["bar", "foobar", "far"])
+    ], names=["a", "b", "c"])
+
+    arr3 = pa.StructArray.from_arrays([
+        pa.array([5, 5, 2], type=pa.int64()),
+        pa.array([2, 8, 1], type=pa.int64()),
+        pa.array(["cat", "dog", "mouse"])
+    ], names=["a", "b", "c"])
+
+    chunked_arr = pa.chunked_array([arr1, arr2, arr3])
+
+    sorted_arr = chunked_arr.sort("ascending")
+    assert sorted_arr.to_pylist() == [
+        {"a": 2, "b":1, "c": "mouse"},
+        {"a": 2, "b":3, "c": "far"},
+        {"a": 5, "b":2, "c": "bar"},
+        {"a": 5, "b":2, "c": "car"},
+        {"a": 5, "b":2, "c": "cat"},
+        {"a": 5, "b":2, "c": "tar"},
+        {"a": 5, "b":3, "c": "foo"},
+        {"a": 5, "b":8, "c": "dog"},
+        {"a": 5, "b":8, "c": "foobar"}
+    ]
+
+    sorted_arr = chunked_arr.sort("descending")
+    assert sorted_arr.to_pylist() == [
+        {"a": 5, "b":8, "c": "foobar"},
+        {"a": 5, "b":8, "c": "dog"},
+        {"a": 5, "b":3, "c": "foo"},
+        {"a": 5, "b":2, "c": "tar"},
+        {"a": 5, "b":2, "c": "cat"},
+        {"a": 5, "b":2, "c": "car"},
+        {"a": 5, "b":2, "c": "bar"},
+        {"a": 2, "b":3, "c": "far"},
+        {"a": 2, "b":1, "c": "mouse"}
+    ]
+
 
 def test_record_batch_sort():
     rb = pa.RecordBatch.from_arrays([

From b9b57e968265be8a3ffaf3925fab63f13163921c Mon Sep 17 00:00:00 2001
From: Jedi18 <akhiljnair.188@gmail.com>
Date: Sun, 30 Jan 2022 13:00:12 +0530
Subject: [PATCH 21/24] add support for nulltype array in
 nestedvaluescomparator

---
 .../arrow/compute/kernels/chunked_internal.h  | 19 ------
 .../compute/kernels/vector_sort_internal.h    | 29 ++++++++
 python/pyarrow/tests/test_array.py            | 66 ++++++++++++++-----
 3 files changed, 77 insertions(+), 37 deletions(-)

diff --git a/cpp/src/arrow/compute/kernels/chunked_internal.h b/cpp/src/arrow/compute/kernels/chunked_internal.h
index e3f47d84287..7e66e9b6403 100644
--- a/cpp/src/arrow/compute/kernels/chunked_internal.h
+++ b/cpp/src/arrow/compute/kernels/chunked_internal.h
@@ -30,25 +30,6 @@ namespace arrow {
 namespace compute {
 namespace internal {
 
-// Visit all physical types for which sorting is implemented.
-#define VISIT_SORTABLE_PHYSICAL_TYPES(VISIT) \
-  VISIT(BooleanType)                         \
-  VISIT(Int8Type)                            \
-  VISIT(Int16Type)                           \
-  VISIT(Int32Type)                           \
-  VISIT(Int64Type)                           \
-  VISIT(UInt8Type)                           \
-  VISIT(UInt16Type)                          \
-  VISIT(UInt32Type)                          \
-  VISIT(UInt64Type)                          \
-  VISIT(FloatType)                           \
-  VISIT(DoubleType)                          \
-  VISIT(BinaryType)                          \
-  VISIT(LargeBinaryType)                     \
-  VISIT(FixedSizeBinaryType)                 \
-  VISIT(Decimal128Type)                      \
-  VISIT(Decimal256Type)
-
 // The target chunk in a chunked array.
 template <typename ArrayType>
 struct ResolvedChunk {
diff --git a/cpp/src/arrow/compute/kernels/vector_sort_internal.h b/cpp/src/arrow/compute/kernels/vector_sort_internal.h
index 0df43c80bd6..25b37c62720 100644
--- a/cpp/src/arrow/compute/kernels/vector_sort_internal.h
+++ b/cpp/src/arrow/compute/kernels/vector_sort_internal.h
@@ -667,6 +667,29 @@ class NestedValuesComparator {
     }
   };
 
+  struct ConcreteNullNestedValueComparator : NestedValueComparator {
+    virtual int Compare(Array const& array, uint64_t offset, uint64_t leftidx,
+                        uint64_t rightidx) {
+      return 0;
+    }
+
+    virtual int Compare(Array const& array, NullPlacement null_placement,
+                        SortOrder sort_order, uint64_t offset, uint64_t leftidx,
+                        uint64_t rightidx) {
+      return 0;
+    }
+
+    virtual int Compare(Array const& left_array, Array const& right_array,
+                        uint64_t leftidx, uint64_t rightidx) {
+      return 0;
+    }
+
+    virtual int Compare(ChunkedArray const& chunked_array, uint64_t offset,
+                        uint64_t leftidx, uint64_t rightidx) {
+      return 0;
+    }
+  };
+
   /**
    * Internal use factory whose purpose is to detect the right type
    * of comparator that should be built to be able to compare two
@@ -698,6 +721,12 @@ class NestedValuesComparator {
       return Status::OK();
     }
 
+    Status Visit(const NullType&) {
+      current_field_comparator_ =
+          std::shared_ptr<NestedValueComparator>(new ConcreteNullNestedValueComparator());
+      return Status::OK();
+    }
+
     std::shared_ptr<NestedValueComparator> current_field_comparator_;
   };
 
diff --git a/python/pyarrow/tests/test_array.py b/python/pyarrow/tests/test_array.py
index 9eeb265a5d7..9a24aa4ae96 100644
--- a/python/pyarrow/tests/test_array.py
+++ b/python/pyarrow/tests/test_array.py
@@ -852,28 +852,28 @@ def test_struct_chunked_array_sort():
 
     sorted_arr = chunked_arr.sort("ascending")
     assert sorted_arr.to_pylist() == [
-        {"a": 2, "b":1, "c": "mouse"},
-        {"a": 2, "b":3, "c": "far"},
-        {"a": 5, "b":2, "c": "bar"},
-        {"a": 5, "b":2, "c": "car"},
-        {"a": 5, "b":2, "c": "cat"},
-        {"a": 5, "b":2, "c": "tar"},
-        {"a": 5, "b":3, "c": "foo"},
-        {"a": 5, "b":8, "c": "dog"},
-        {"a": 5, "b":8, "c": "foobar"}
+        {"a": 2, "b": 1, "c": "mouse"},
+        {"a": 2, "b": 3, "c": "far"},
+        {"a": 5, "b": 2, "c": "bar"},
+        {"a": 5, "b": 2, "c": "car"},
+        {"a": 5, "b": 2, "c": "cat"},
+        {"a": 5, "b": 2, "c": "tar"},
+        {"a": 5, "b": 3, "c": "foo"},
+        {"a": 5, "b": 8, "c": "dog"},
+        {"a": 5, "b": 8, "c": "foobar"}
     ]
 
     sorted_arr = chunked_arr.sort("descending")
     assert sorted_arr.to_pylist() == [
-        {"a": 5, "b":8, "c": "foobar"},
-        {"a": 5, "b":8, "c": "dog"},
-        {"a": 5, "b":3, "c": "foo"},
-        {"a": 5, "b":2, "c": "tar"},
-        {"a": 5, "b":2, "c": "cat"},
-        {"a": 5, "b":2, "c": "car"},
-        {"a": 5, "b":2, "c": "bar"},
-        {"a": 2, "b":3, "c": "far"},
-        {"a": 2, "b":1, "c": "mouse"}
+        {"a": 5, "b": 8, "c": "foobar"},
+        {"a": 5, "b": 8, "c": "dog"},
+        {"a": 5, "b": 3, "c": "foo"},
+        {"a": 5, "b": 2, "c": "tar"},
+        {"a": 5, "b": 2, "c": "cat"},
+        {"a": 5, "b": 2, "c": "car"},
+        {"a": 5, "b": 2, "c": "bar"},
+        {"a": 2, "b": 3, "c": "far"},
+        {"a": 2, "b": 1, "c": "mouse"}
     ]
 
 
@@ -926,6 +926,36 @@ def test_record_batch_sort():
     assert sorted_rb1_dict["i"] == [3, 4, 1, 2]
     assert sorted_rb1_dict["j"] == ["foobar", "bar", "car", "foo"]
 
+    # test radix sort with nulls
+    rb2_names = ["a", "b", "c", "d", "e", "f", "g", "h", "i", "j"]
+    rb2 = pa.RecordBatch.from_arrays([
+        pa.array([None, None, None, None]),
+        pa.array([4, 4, 4, 4], type=pa.int64()),
+        pa.array([4, 4, 4, 4], type=pa.int64()),
+        pa.array([4, 4, 2, 2], type=pa.int64()),
+        pa.array([4, 4, 2, 2], type=pa.int64()),
+        pa.array([4, 4, 2, 2], type=pa.int64()),
+        pa.array([4, 4, 2, 2], type=pa.int64()),
+        pa.array([4, 4, 2, 2], type=pa.int64()),
+        pa.array([2, 1, 4, 3], type=pa.int64()),
+        pa.array([2, 1, 4, 3], type=pa.int64()),
+    ], names=rb2_names)
+
+    sort_keys_list = [("a", "ascending"), ("j", "ascending")]
+    sorted_rb2 = rb2.sort_by(sort_keys_list)
+    sorted_rb2_dict = sorted_rb2.to_pydict()
+
+    assert sorted_rb2_dict["a"] == [None, None, None, None]
+    assert sorted_rb2_dict["b"] == [4, 4, 4, 4]
+    assert sorted_rb2_dict["c"] == [4, 4, 4, 4]
+    assert sorted_rb2_dict["d"] == [4, 4, 2, 2]
+    assert sorted_rb2_dict["e"] == [4, 4, 2, 2]
+    assert sorted_rb2_dict["f"] == [4, 4, 2, 2]
+    assert sorted_rb2_dict["g"] == [4, 4, 2, 2]
+    assert sorted_rb2_dict["h"] == [4, 4, 2, 2]
+    assert sorted_rb2_dict["i"] == [1, 2, 3, 4]
+    assert sorted_rb2_dict["j"] == [1, 2, 3, 4]
+
 
 def test_table_sort():
     tab = pa.Table.from_arrays([

From 8c6602891b7e12bbefc48484540a7849d871300a Mon Sep 17 00:00:00 2001
From: Jedi18 <akhiljnair.188@gmail.com>
Date: Tue, 1 Feb 2022 12:31:08 +0530
Subject: [PATCH 22/24] fix selectkth record batch bug

---
 cpp/src/arrow/compute/kernels/vector_sort.cc | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/cpp/src/arrow/compute/kernels/vector_sort.cc b/cpp/src/arrow/compute/kernels/vector_sort.cc
index d5cc76c6ce2..77e978141a0 100644
--- a/cpp/src/arrow/compute/kernels/vector_sort.cc
+++ b/cpp/src/arrow/compute/kernels/vector_sort.cc
@@ -789,8 +789,8 @@ class MultipleKeyRecordBatchSorter : public TypeVisitor {
                          }
                        }
 
-                       int sort_keys_len = sort_keys_.size();
-                       for (int i = 1; i < sort_keys_len; i++) {
+                       const auto sort_keys_len = sort_keys_.size();
+                       for (size_t i = 1; i < sort_keys_len; i++) {
                          int val = nested_values_comparator_.Compare(
                              batch_, null_placement_, sort_keys_[i].order,
                              sort_keys_[i].field_index, 0, left, right);
@@ -1622,7 +1622,7 @@ class RecordBatchSelecter : public TypeVisitor {
     std::vector<ResolvedSortKey> resolved;
     for (const auto& key : sort_keys) {
       auto array = key.target.GetOne(batch).ValueOr(nullptr);
-      resolved.emplace_back(array, key.order, key.target.field_path()->indices()[0]);
+      resolved.emplace_back(array, key.order, -1);
     }
     return resolved;
   }

From db936ad8f75a892c63a4543581e388466c80f797 Mon Sep 17 00:00:00 2001
From: Jedi18 <akhiljnair.188@gmail.com>
Date: Thu, 3 Feb 2022 17:07:40 +0530
Subject: [PATCH 23/24] handle return status of prepare call

---
 cpp/src/arrow/compute/kernels/vector_sort_internal.h | 11 +++++++----
 1 file changed, 7 insertions(+), 4 deletions(-)

diff --git a/cpp/src/arrow/compute/kernels/vector_sort_internal.h b/cpp/src/arrow/compute/kernels/vector_sort_internal.h
index 25b37c62720..4ed1f50cde5 100644
--- a/cpp/src/arrow/compute/kernels/vector_sort_internal.h
+++ b/cpp/src/arrow/compute/kernels/vector_sort_internal.h
@@ -539,9 +539,8 @@ class NestedValuesComparator {
 
   // ResolvedChunk<StructArray> Prepare overload
   Status Prepare(const ResolvedChunk<StructArray>& chunk_left,
-                 const ResolvedChunk<StructArray>& chunk_right) {
-    Prepare(*chunk_left.array);
-    return Status::OK();
+                 const ResolvedChunk<StructArray>&) {
+    return Prepare(*chunk_left.array);
   }
 
   // ResolvedChunk<StructArray> Compare overload
@@ -750,7 +749,11 @@ struct ResolvedChunkComparator<StructArray> {
     const auto chunk_left = chunk_resolver.Resolve<StructArray>(left);
     const auto chunk_right = chunk_resolver.Resolve<StructArray>(right);
     NestedValuesComparator nested_values_comparator;
-    nested_values_comparator.Prepare(chunk_left, chunk_right);
+    auto status = nested_values_comparator.Prepare(chunk_left, chunk_right);
+
+    if(!status.ok()) {
+      return false;
+    }
 
     for (int i = 0; i < chunk_left.array->num_fields(); i++) {
       int val = nested_values_comparator.Compare(chunk_left, chunk_right, i);

From 87c3041aed5966f70da71f9af201b56edad1a1b9 Mon Sep 17 00:00:00 2001
From: Jedi18 <akhiljnair.188@gmail.com>
Date: Thu, 3 Feb 2022 19:06:19 +0530
Subject: [PATCH 24/24] add visit overloads for temporal types

---
 cpp/src/arrow/compute/kernels/vector_sort.cc  | 10 ++++--
 .../compute/kernels/vector_sort_internal.h    | 32 ++++++++++++++++++-
 2 files changed, 39 insertions(+), 3 deletions(-)

diff --git a/cpp/src/arrow/compute/kernels/vector_sort.cc b/cpp/src/arrow/compute/kernels/vector_sort.cc
index 77e978141a0..c1b55dea765 100644
--- a/cpp/src/arrow/compute/kernels/vector_sort.cc
+++ b/cpp/src/arrow/compute/kernels/vector_sort.cc
@@ -499,7 +499,10 @@ class RadixRecordBatchSorter {
           field_index_(sort_key.field_index),
           nested_values_comparator_(nullptr) {
       nested_values_comparator_ = std::make_shared<NestedValuesComparator>();
-      nested_values_comparator_->Prepare(batch);
+      auto s = nested_values_comparator_->Prepare(batch);
+      if (!s.ok()) {
+        return;
+      }
     }
 
     Result<std::unique_ptr<RecordBatchColumnSorter>> MakeColumnSort() {
@@ -735,7 +738,10 @@ class MultipleKeyRecordBatchSorter : public TypeVisitor {
         sort_keys_(ResolveSortKeys(batch, options.sort_keys, &status_)),
         null_placement_(options.null_placement),
         comparator_(sort_keys_, null_placement_) {
-    nested_values_comparator_.Prepare(batch);
+    auto s = nested_values_comparator_.Prepare(batch);
+    if (!s.ok()) {
+      return;
+    }
   }
 
   // This is optimized for the first sort key. The first sort key sort
diff --git a/cpp/src/arrow/compute/kernels/vector_sort_internal.h b/cpp/src/arrow/compute/kernels/vector_sort_internal.h
index 4ed1f50cde5..72dae776e10 100644
--- a/cpp/src/arrow/compute/kernels/vector_sort_internal.h
+++ b/cpp/src/arrow/compute/kernels/vector_sort_internal.h
@@ -726,6 +726,36 @@ class NestedValuesComparator {
       return Status::OK();
     }
 
+    Status Visit(const Date32Type&) {
+      current_field_comparator_ = std::shared_ptr<NestedValueComparator>(
+          new ConcreteNestedValueComparator<Date32Type>());
+      return Status::OK();
+    }
+
+    Status Visit(const Date64Type&) {
+      current_field_comparator_ = std::shared_ptr<NestedValueComparator>(
+          new ConcreteNestedValueComparator<Date64Type>());
+      return Status::OK();
+    }
+
+    Status Visit(const Time32Type&) {
+      current_field_comparator_ = std::shared_ptr<NestedValueComparator>(
+          new ConcreteNestedValueComparator<Time32Type>());
+      return Status::OK();
+    }
+
+    Status Visit(const Time64Type&) {
+      current_field_comparator_ = std::shared_ptr<NestedValueComparator>(
+          new ConcreteNestedValueComparator<Time64Type>());
+      return Status::OK();
+    }
+
+    Status Visit(const TimestampType&) {
+      current_field_comparator_ = std::shared_ptr<NestedValueComparator>(
+          new ConcreteNestedValueComparator<TimestampType>());
+      return Status::OK();
+    }
+
     std::shared_ptr<NestedValueComparator> current_field_comparator_;
   };
 
@@ -751,7 +781,7 @@ struct ResolvedChunkComparator<StructArray> {
     NestedValuesComparator nested_values_comparator;
     auto status = nested_values_comparator.Prepare(chunk_left, chunk_right);
 
-    if(!status.ok()) {
+    if (!status.ok()) {
       return false;
     }