From 6fc0d22ff545166a0b171f4be7725807cc933b45 Mon Sep 17 00:00:00 2001
From: Luc Rancourt <lucrancourt@gmail.com>
Date: Wed, 18 Oct 2023 18:44:46 +0200
Subject: [PATCH] Aggregates: change to use size instead of min cell/max cell.

This changes the AggregateWithCount class to compute using a size only instead of min cell/max cell.

---
TYPE: IMPROVEMENT
DESC: Aggregates: change to use size instead of min cell/max cell.
---
 .../readers/aggregators/aggregate_buffer.h     | 18 +++++++-----------
 .../readers/aggregators/aggregate_with_count.h | 12 ++++--------
 2 files changed, 11 insertions(+), 19 deletions(-)

diff --git a/tiledb/sm/query/readers/aggregators/aggregate_buffer.h b/tiledb/sm/query/readers/aggregators/aggregate_buffer.h
index baad19923a7..74cd4806cbb 100644
--- a/tiledb/sm/query/readers/aggregators/aggregate_buffer.h
+++ b/tiledb/sm/query/readers/aggregators/aggregate_buffer.h
@@ -91,14 +91,9 @@ class AggregateBuffer {
     return bitmap_data_.has_value();
   }
 
-  /** Returns the min cell position to aggregate. */
-  uint64_t min_cell() const {
-    return min_cell_;
-  }
-
-  /** Returns the max cell position to aggregate. */
-  uint64_t max_cell() const {
-    return max_cell_;
+  /** Returns the number of cells to aggregate. */
+  uint64_t size() const {
+    return max_cell_ - min_cell_;
   }
 
   /**
@@ -111,7 +106,8 @@ class AggregateBuffer {
    * @return Value.
    */
   template <typename T>
-  inline T value_at(const uint64_t cell_idx) const {
+  inline T value_at(uint64_t cell_idx) const {
+    cell_idx += min_cell_;
     if constexpr (std::is_same_v<T, std::string_view>) {
       if (var_data_.has_value()) {
         auto offsets = static_cast<const uint64_t*>(fixed_data_);
@@ -138,7 +134,7 @@ class AggregateBuffer {
    * @return Validity value.
    */
   inline uint8_t validity_at(const uint64_t cell_idx) const {
-    return validity_data_.value()[cell_idx];
+    return validity_data_.value()[cell_idx + min_cell_];
   }
 
   /**
@@ -150,7 +146,7 @@ class AggregateBuffer {
    */
   template <class BitmapType>
   inline BitmapType bitmap_at(const uint64_t cell_idx) const {
-    return static_cast<BitmapType*>(bitmap_data_.value())[cell_idx];
+    return static_cast<BitmapType*>(bitmap_data_.value())[cell_idx + min_cell_];
   }
 
  private:
diff --git a/tiledb/sm/query/readers/aggregators/aggregate_with_count.h b/tiledb/sm/query/readers/aggregators/aggregate_with_count.h
index e22186f9cd8..082d969767d 100644
--- a/tiledb/sm/query/readers/aggregators/aggregate_with_count.h
+++ b/tiledb/sm/query/readers/aggregators/aggregate_with_count.h
@@ -97,8 +97,7 @@ class AggregateWithCount {
     if (input_data.has_bitmap()) {
       if (field_info_.is_nullable_) {
         // Process for nullable values with bitmap.
-        for (uint64_t c = input_data.min_cell(); c < input_data.max_cell();
-             c++) {
+        for (uint64_t c = 0; c < input_data.size(); c++) {
           auto bitmap_val = input_data.bitmap_at<BITMAP_T>(c);
           if (val_policy.op(input_data.validity_at(c)) && bitmap_val != 0) {
             auto value = value_at(input_data, c);
@@ -110,8 +109,7 @@ class AggregateWithCount {
         }
       } else {
         // Process for non nullable values with bitmap.
-        for (uint64_t c = input_data.min_cell(); c < input_data.max_cell();
-             c++) {
+        for (uint64_t c = 0; c < input_data.size(); c++) {
           auto bitmap_val = input_data.bitmap_at<BITMAP_T>(c);
           auto value = value_at(input_data, c);
           for (BITMAP_T i = 0; i < bitmap_val; i++) {
@@ -123,8 +121,7 @@ class AggregateWithCount {
     } else {
       if (field_info_.is_nullable_) {
         // Process for nullable values with no bitmap.
-        for (uint64_t c = input_data.min_cell(); c < input_data.max_cell();
-             c++) {
+        for (uint64_t c = 0; c < input_data.size(); c++) {
           if (val_policy.op(input_data.validity_at(c))) {
             auto value = value_at(input_data, c);
             agg_policy.op(value, res, count);
@@ -133,8 +130,7 @@ class AggregateWithCount {
         }
       } else {
         // Process for non nullable values with no bitmap.
-        for (uint64_t c = input_data.min_cell(); c < input_data.max_cell();
-             c++) {
+        for (uint64_t c = 0; c < input_data.size(); c++) {
           auto value = value_at(input_data, c);
           agg_policy.op(value, res, count);
           count++;