From 911008c4a8ee7b002c074c1662e84dbd87b12641 Mon Sep 17 00:00:00 2001
From: mytkom <marek.mytkowski.mm@gmail.com>
Date: Sun, 8 Sep 2024 19:30:51 +0000
Subject: [PATCH 1/6] use generic get() in row_helpers

---
 Framework/Core/include/Framework/ASoA.h       |  2 +-
 .../Core/include/Framework/ASoAHelpers.h      | 66 ++-----------------
 .../Core/include/Framework/BinningPolicy.h    | 28 ++++----
 3 files changed, 19 insertions(+), 77 deletions(-)

diff --git a/Framework/Core/include/Framework/ASoA.h b/Framework/Core/include/Framework/ASoA.h
index ccf2cab5e6807..5bfb601169f4e 100644
--- a/Framework/Core/include/Framework/ASoA.h
+++ b/Framework/Core/include/Framework/ASoA.h
@@ -2186,7 +2186,7 @@ typename C::type getSingleRowData(arrow::Table*, T& rowIterator, uint64_t ci = s
   if (globalIndex != std::numeric_limits<uint64_t>::max() && globalIndex != *std::get<0>(rowIterator.getIndices())) {
     rowIterator.setCursor(globalIndex);
   }
-  return rowIterator.template getDynamicColumn<C>();
+  return static_cast<C>(rowIterator).get();
 }
 
 template <typename T, soa::is_index_column C>
diff --git a/Framework/Core/include/Framework/ASoAHelpers.h b/Framework/Core/include/Framework/ASoAHelpers.h
index 5bf474e61f935..2f2d1f06fce1a 100644
--- a/Framework/Core/include/Framework/ASoAHelpers.h
+++ b/Framework/Core/include/Framework/ASoAHelpers.h
@@ -76,77 +76,19 @@ void dataSizeVariesBetweenColumns();
 template <template <typename... Cs> typename BP, typename T, typename... Cs>
 std::vector<BinningIndex> groupTable(const T& table, const BP<Cs...>& binningPolicy, int minCatSize, int outsider)
 {
-  arrow::Table* arrowTable = table.asArrowTable().get();
-  auto rowIterator = table.begin();
-
-  uint64_t ind = 0;
-  uint64_t selInd = 0;
-  gsl::span<int64_t const> selectedRows;
   std::vector<BinningIndex> groupedIndices;
 
-  // Separate check to account for Filtered size different from arrow table
+  // TODO: Check if this check can be now skipped
   if (table.size() == 0) {
     return groupedIndices;
   }
 
-  if constexpr (soa::is_filtered_table<T>) {
-    selectedRows = table.getSelectedRows(); // vector<int64_t>
-  }
-
-  auto persistentColumns = typename BP<Cs...>::persistent_columns_t{};
-  constexpr auto persistentColumnsCount = pack_size(persistentColumns);
-  auto arrowColumns = o2::soa::row_helpers::getArrowColumns(arrowTable, persistentColumns);
-  auto chunksCount = arrowColumns[0]->num_chunks();
-  for (int i = 1; i < persistentColumnsCount; i++) {
-    if (arrowColumns[i]->num_chunks() != chunksCount) {
-      dataSizeVariesBetweenColumns();
-    }
-  }
-
-  for (uint64_t ci = 0; ci < chunksCount; ++ci) {
-    auto chunks = o2::soa::row_helpers::getChunks(arrowTable, persistentColumns, ci);
-    auto chunkLength = std::get<0>(chunks)->length();
-    for_<persistentColumnsCount - 1>([&chunks, &chunkLength](auto i) {
-      if (std::get<i.value + 1>(chunks)->length() != chunkLength) {
-        dataSizeVariesBetweenColumns();
-      }
-    });
-
-    if constexpr (soa::is_filtered_table<T>) {
-      if (selectedRows[ind] >= selInd + chunkLength) {
-        selInd += chunkLength;
-        continue; // Go to the next chunk, no value selected in this chunk
-      }
-    }
-
-    uint64_t ai = 0;
-    while (ai < chunkLength) {
-      if constexpr (soa::is_filtered_table<T>) {
-        ai += selectedRows[ind] - selInd;
-        selInd = selectedRows[ind];
-      }
-
-      auto values = binningPolicy.getBinningValues(rowIterator, arrowTable, ci, ai, ind);
+  for(auto rowIterator : table) {
+      auto values = binningPolicy.getBinningValues(rowIterator);
       auto val = binningPolicy.getBin(values);
       if (val != outsider) {
-        groupedIndices.emplace_back(val, ind);
-      }
-      ind++;
-
-      if constexpr (soa::is_filtered_table<T>) {
-        if (ind >= selectedRows.size()) {
-          break;
-        }
-      } else {
-        ai++;
+        groupedIndices.emplace_back(val, *std::get<1>(rowIterator.getIndices()));
       }
-    }
-
-    if constexpr (soa::is_filtered_table<T>) {
-      if (ind == selectedRows.size()) {
-        break;
-      }
-    }
   }
 
   // Do a stable sort so that same categories entries are
diff --git a/Framework/Core/include/Framework/BinningPolicy.h b/Framework/Core/include/Framework/BinningPolicy.h
index 73548cbd6c6ed..9afd43a6490d7 100644
--- a/Framework/Core/include/Framework/BinningPolicy.h
+++ b/Framework/Core/include/Framework/BinningPolicy.h
@@ -241,7 +241,7 @@ struct FlexibleBinningPolicy<std::tuple<Ls...>, Ts...> : BinningPolicyBase<sizeo
   }
 
   template <typename T, typename T2>
-  auto getBinningValue(T& rowIterator, arrow::Table* table, uint64_t ci = -1, uint64_t ai = -1, uint64_t globalIndex = -1) const
+  auto getBinningValue(T& rowIterator, uint64_t globalIndex = -1) const
   {
     if constexpr (has_type<T2>(pack<Ls...>{})) {
       if (globalIndex != -1) {
@@ -249,20 +249,20 @@ struct FlexibleBinningPolicy<std::tuple<Ls...>, Ts...> : BinningPolicyBase<sizeo
       }
       return std::get<T2>(mBinningFunctions)(rowIterator);
     } else {
-      return soa::row_helpers::getSingleRowData<T, T2>(table, rowIterator, ci, ai, globalIndex);
+      return soa::row_helpers::getSingleRowData<T, T2>(rowIterator, globalIndex);
     }
   }
 
   template <typename T>
-  auto getBinningValues(T& rowIterator, arrow::Table* table, uint64_t ci = -1, uint64_t ai = -1, uint64_t globalIndex = -1) const
+  auto getBinningValues(T& rowIterator, uint64_t globalIndex = -1) const
   {
-    return std::make_tuple(getBinningValue<T, Ts>(rowIterator, table, ci, ai, globalIndex)...);
+    return std::make_tuple(getBinningValue<T, Ts>(rowIterator, globalIndex)...);
   }
 
   template <typename T>
-  auto getBinningValues(typename T::iterator rowIterator, T& table, uint64_t ci = -1, uint64_t ai = -1, uint64_t globalIndex = -1) const
+  auto getBinningValues(typename T::iterator rowIterator, uint64_t globalIndex = -1) const
   {
-    return getBinningValues(rowIterator, table.asArrowTable().get(), ci, ai, globalIndex);
+    return getBinningValues(rowIterator, globalIndex);
   }
 
   template <typename... T2s>
@@ -284,15 +284,15 @@ struct ColumnBinningPolicy : BinningPolicyBase<sizeof...(Ts)> {
   }
 
   template <typename T>
-  auto getBinningValues(T& rowIterator, arrow::Table* table, uint64_t ci = -1, uint64_t ai = -1, uint64_t globalIndex = -1) const
+  auto getBinningValues(T& rowIterator, uint64_t globalIndex = -1) const
   {
-    return std::make_tuple(soa::row_helpers::getSingleRowData<T, Ts>(table, rowIterator, ci, ai, globalIndex)...);
+    return std::make_tuple(soa::row_helpers::getSingleRowData<T, Ts>(rowIterator, globalIndex)...);
   }
 
   template <typename T>
-  auto getBinningValues(typename T::iterator rowIterator, T& table, uint64_t ci = -1, uint64_t ai = -1, uint64_t globalIndex = -1) const
+  auto getBinningValues(typename T::iterator rowIterator, T& table, uint64_t globalIndex = -1) const
   {
-    return getBinningValues(rowIterator, table.asArrowTable().get(), ci, ai, globalIndex);
+    return getBinningValues(rowIterator, globalIndex);
   }
 
   int getBin(std::tuple<typename Ts::type...> const& data) const
@@ -309,15 +309,15 @@ struct NoBinningPolicy {
   NoBinningPolicy() = default;
 
   template <typename T>
-  auto getBinningValues(T& rowIterator, arrow::Table* table, uint64_t ci = -1, uint64_t ai = -1, uint64_t globalIndex = -1) const
+  auto getBinningValues(T& rowIterator, uint64_t globalIndex = -1) const
   {
-    return std::make_tuple(soa::row_helpers::getSingleRowData<T, C>(table, rowIterator, ci, ai, globalIndex));
+    return std::make_tuple(soa::row_helpers::getSingleRowData<T, C>(rowIterator, globalIndex));
   }
 
   template <typename T>
-  auto getBinningValues(typename T::iterator rowIterator, T& table, uint64_t ci = -1, uint64_t ai = -1, uint64_t globalIndex = -1) const
+  auto getBinningValues(typename T::iterator rowIterator, uint64_t globalIndex = -1) const
   {
-    return getBinningValues(rowIterator, table.asArrowTable().get(), ci, ai, globalIndex);
+    return getBinningValues(rowIterator, globalIndex);
   }
 
   int getBin(std::tuple<typename C::type> const& data) const

From 616bfb26ecfb946d3a6e79aa43817da8a14689b2 Mon Sep 17 00:00:00 2001
From: mytkom <marek.mytkowski.mm@gmail.com>
Date: Mon, 9 Sep 2024 21:30:22 +0000
Subject: [PATCH 2/6] backward compatibility for binning policy

---
 Framework/Core/include/Framework/BinningPolicy.h | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/Framework/Core/include/Framework/BinningPolicy.h b/Framework/Core/include/Framework/BinningPolicy.h
index 9afd43a6490d7..58fc42ae63422 100644
--- a/Framework/Core/include/Framework/BinningPolicy.h
+++ b/Framework/Core/include/Framework/BinningPolicy.h
@@ -260,7 +260,7 @@ struct FlexibleBinningPolicy<std::tuple<Ls...>, Ts...> : BinningPolicyBase<sizeo
   }
 
   template <typename T>
-  auto getBinningValues(typename T::iterator rowIterator, uint64_t globalIndex = -1) const
+  auto getBinningValues(typename T::iterator rowIterator, T& table, uint64_t globalIndex = -1) const
   {
     return getBinningValues(rowIterator, globalIndex);
   }
@@ -315,7 +315,7 @@ struct NoBinningPolicy {
   }
 
   template <typename T>
-  auto getBinningValues(typename T::iterator rowIterator, uint64_t globalIndex = -1) const
+  auto getBinningValues(typename T::iterator rowIterator, T& table, uint64_t globalIndex = -1) const
   {
     return getBinningValues(rowIterator, globalIndex);
   }

From 3d926b3bd334e3ac57618c9c39766daa04c2dd35 Mon Sep 17 00:00:00 2001
From: saganatt <8majak8@gmail.com>
Date: Thu, 9 Jan 2025 18:15:47 +0100
Subject: [PATCH 3/6] Fix after rebase

---
 Framework/Core/include/Framework/ASoA.h | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/Framework/Core/include/Framework/ASoA.h b/Framework/Core/include/Framework/ASoA.h
index 5bfb601169f4e..ccf2cab5e6807 100644
--- a/Framework/Core/include/Framework/ASoA.h
+++ b/Framework/Core/include/Framework/ASoA.h
@@ -2186,7 +2186,7 @@ typename C::type getSingleRowData(arrow::Table*, T& rowIterator, uint64_t ci = s
   if (globalIndex != std::numeric_limits<uint64_t>::max() && globalIndex != *std::get<0>(rowIterator.getIndices())) {
     rowIterator.setCursor(globalIndex);
   }
-  return static_cast<C>(rowIterator).get();
+  return rowIterator.template getDynamicColumn<C>();
 }
 
 template <typename T, soa::is_index_column C>

From 6bfcb7a5223737d8536266861e2414ded8fff3d8 Mon Sep 17 00:00:00 2001
From: saganatt <8majak8@gmail.com>
Date: Mon, 13 Jan 2025 09:57:05 +0100
Subject: [PATCH 4/6] Further simplification and compilable code

---
 Framework/Core/include/Framework/ASoA.h       | 51 +------------------
 .../Core/include/Framework/BinningPolicy.h    | 18 ++++---
 2 files changed, 14 insertions(+), 55 deletions(-)

diff --git a/Framework/Core/include/Framework/ASoA.h b/Framework/Core/include/Framework/ASoA.h
index ccf2cab5e6807..c20e6b9e7f5ff 100644
--- a/Framework/Core/include/Framework/ASoA.h
+++ b/Framework/Core/include/Framework/ASoA.h
@@ -2157,61 +2157,14 @@ void emptyColumnLabel();
 
 namespace row_helpers
 {
-template <soa::is_persistent_column... Cs>
-std::array<arrow::ChunkedArray*, sizeof...(Cs)> getArrowColumns(arrow::Table* table, framework::pack<Cs...>)
-{
-  return std::array<arrow::ChunkedArray*, sizeof...(Cs)>{o2::soa::getIndexFromLabel(table, Cs::columnLabel())...};
-}
-
-template <soa::is_persistent_column... Cs>
-std::array<std::shared_ptr<arrow::Array>, sizeof...(Cs)> getChunks(arrow::Table* table, framework::pack<Cs...>, uint64_t ci)
-{
-  return std::array<std::shared_ptr<arrow::Array>, sizeof...(Cs)>{o2::soa::getIndexFromLabel(table, Cs::columnLabel())->chunk(ci)...};
-}
-
-template <typename T, soa::is_persistent_column C>
-typename C::type getSingleRowData(arrow::Table* table, T& rowIterator, uint64_t ci = std::numeric_limits<uint64_t>::max(), uint64_t ai = std::numeric_limits<uint64_t>::max(), uint64_t globalIndex = std::numeric_limits<uint64_t>::max())
-{
-  if (ci == std::numeric_limits<uint64_t>::max() || ai == std::numeric_limits<uint64_t>::max()) {
-    auto colIterator = static_cast<C>(rowIterator).getIterator();
-    ci = colIterator.mCurrentChunk;
-    ai = *(colIterator.mCurrentPos) - colIterator.mFirstIndex;
-  }
-  return std::static_pointer_cast<o2::soa::arrow_array_for_t<typename C::type>>(o2::soa::getIndexFromLabel(table, C::columnLabel())->chunk(ci))->raw_values()[ai];
-}
-
-template <typename T, soa::is_dynamic_column C>
-typename C::type getSingleRowData(arrow::Table*, T& rowIterator, uint64_t ci = std::numeric_limits<uint64_t>::max(), uint64_t ai = std::numeric_limits<uint64_t>::max(), uint64_t globalIndex = std::numeric_limits<uint64_t>::max())
-{
-  if (globalIndex != std::numeric_limits<uint64_t>::max() && globalIndex != *std::get<0>(rowIterator.getIndices())) {
-    rowIterator.setCursor(globalIndex);
-  }
-  return rowIterator.template getDynamicColumn<C>();
-}
-
-template <typename T, soa::is_index_column C>
-typename C::type getSingleRowData(arrow::Table*, T& rowIterator, uint64_t ci = std::numeric_limits<uint64_t>::max(), uint64_t ai = std::numeric_limits<uint64_t>::max(), uint64_t globalIndex = std::numeric_limits<uint64_t>::max())
-{
-  if (globalIndex != std::numeric_limits<uint64_t>::max() && globalIndex != *std::get<0>(rowIterator.getIndices())) {
-    rowIterator.setCursor(globalIndex);
-  }
-  return rowIterator.template getId<C>();
-}
-
-template <typename T, typename... Cs>
-std::tuple<typename Cs::type...> getRowData(arrow::Table* table, T rowIterator, uint64_t ci = std::numeric_limits<uint64_t>::max(), uint64_t ai = std::numeric_limits<uint64_t>::max(), uint64_t globalIndex = std::numeric_limits<uint64_t>::max())
-{
-  return std::make_tuple(getSingleRowData<T, Cs>(table, rowIterator, ci, ai, globalIndex)...);
-}
-
-namespace
-{
 template <typename R, typename T, typename C>
 R getColumnValue(const T& rowIterator)
 {
   return static_cast<R>(static_cast<C>(rowIterator).get());
 }
 
+namespace
+{
 template <typename R, typename T>
 using ColumnGetterFunction = R (*)(const T&);
 
diff --git a/Framework/Core/include/Framework/BinningPolicy.h b/Framework/Core/include/Framework/BinningPolicy.h
index 58fc42ae63422..3e41302c920f8 100644
--- a/Framework/Core/include/Framework/BinningPolicy.h
+++ b/Framework/Core/include/Framework/BinningPolicy.h
@@ -243,13 +243,13 @@ struct FlexibleBinningPolicy<std::tuple<Ls...>, Ts...> : BinningPolicyBase<sizeo
   template <typename T, typename T2>
   auto getBinningValue(T& rowIterator, uint64_t globalIndex = -1) const
   {
+    if (globalIndex != -1) {
+      rowIterator.setCursor(globalIndex);
+    }
     if constexpr (has_type<T2>(pack<Ls...>{})) {
-      if (globalIndex != -1) {
-        rowIterator.setCursor(globalIndex);
-      }
       return std::get<T2>(mBinningFunctions)(rowIterator);
     } else {
-      return soa::row_helpers::getSingleRowData<T, T2>(rowIterator, globalIndex);
+      return soa::row_helpers::getColumnValue<typename T2::type, T, T2>(rowIterator);
     }
   }
 
@@ -286,7 +286,10 @@ struct ColumnBinningPolicy : BinningPolicyBase<sizeof...(Ts)> {
   template <typename T>
   auto getBinningValues(T& rowIterator, uint64_t globalIndex = -1) const
   {
-    return std::make_tuple(soa::row_helpers::getSingleRowData<T, Ts>(rowIterator, globalIndex)...);
+    if (globalIndex != -1) {
+      rowIterator.setCursor(globalIndex);
+    }
+    return std::make_tuple(soa::row_helpers::getColumnValue<typename Ts::type, T, Ts>(rowIterator)...);
   }
 
   template <typename T>
@@ -311,7 +314,10 @@ struct NoBinningPolicy {
   template <typename T>
   auto getBinningValues(T& rowIterator, uint64_t globalIndex = -1) const
   {
-    return std::make_tuple(soa::row_helpers::getSingleRowData<T, C>(rowIterator, globalIndex));
+    if (globalIndex != -1) {
+      rowIterator.setCursor(globalIndex);
+    }
+    return std::make_tuple(soa::row_helpers::getColumnValue<typename C::type, T, C>(rowIterator));
   }
 
   template <typename T>

From 750c0e2009a74e06ef3af3fc7dc2e0aa13d6d226 Mon Sep 17 00:00:00 2001
From: saganatt <8majak8@gmail.com>
Date: Wed, 30 Apr 2025 15:08:45 +0200
Subject: [PATCH 5/6] Remove probably redundant check. To be checked

---
 Framework/Core/include/Framework/ASoAHelpers.h | 5 -----
 1 file changed, 5 deletions(-)

diff --git a/Framework/Core/include/Framework/ASoAHelpers.h b/Framework/Core/include/Framework/ASoAHelpers.h
index 2f2d1f06fce1a..129224017e294 100644
--- a/Framework/Core/include/Framework/ASoAHelpers.h
+++ b/Framework/Core/include/Framework/ASoAHelpers.h
@@ -78,11 +78,6 @@ std::vector<BinningIndex> groupTable(const T& table, const BP<Cs...>& binningPol
 {
   std::vector<BinningIndex> groupedIndices;
 
-  // TODO: Check if this check can be now skipped
-  if (table.size() == 0) {
-    return groupedIndices;
-  }
-
   for(auto rowIterator : table) {
       auto values = binningPolicy.getBinningValues(rowIterator);
       auto val = binningPolicy.getBin(values);

From 781d68c6f41015ff9d4e99946b0570afe17fd5b9 Mon Sep 17 00:00:00 2001
From: ALICE Action Bot <alibuild@cern.ch>
Date: Fri, 11 Jul 2025 11:41:07 +0000
Subject: [PATCH 6/6] Please consider the following formatting changes

---
 Framework/Core/include/Framework/ASoAHelpers.h | 12 ++++++------
 1 file changed, 6 insertions(+), 6 deletions(-)

diff --git a/Framework/Core/include/Framework/ASoAHelpers.h b/Framework/Core/include/Framework/ASoAHelpers.h
index 129224017e294..0449bdfdc2a0c 100644
--- a/Framework/Core/include/Framework/ASoAHelpers.h
+++ b/Framework/Core/include/Framework/ASoAHelpers.h
@@ -78,12 +78,12 @@ std::vector<BinningIndex> groupTable(const T& table, const BP<Cs...>& binningPol
 {
   std::vector<BinningIndex> groupedIndices;
 
-  for(auto rowIterator : table) {
-      auto values = binningPolicy.getBinningValues(rowIterator);
-      auto val = binningPolicy.getBin(values);
-      if (val != outsider) {
-        groupedIndices.emplace_back(val, *std::get<1>(rowIterator.getIndices()));
-      }
+  for (auto rowIterator : table) {
+    auto values = binningPolicy.getBinningValues(rowIterator);
+    auto val = binningPolicy.getBin(values);
+    if (val != outsider) {
+      groupedIndices.emplace_back(val, *std::get<1>(rowIterator.getIndices()));
+    }
   }
 
   // Do a stable sort so that same categories entries are