Make buffer ordering predictable.

TileDB-Inc · Sep 4, 2023 · 83a9778 · 83a9778
1 parent 115ba8b
commit 83a9778
Show file tree

Hide file tree

Showing 2 changed files with 26 additions and 21 deletions.
diff --git a/test/src/test-cppapi-aggregates.cc b/test/src/test-cppapi-aggregates.cc
@@ -1901,7 +1901,7 @@ TEST_CASE_METHOD(
 TEST_CASE_METHOD(
     CppAggregatesFx<std::string>,
     "C++ API: Aggregates var overflow",
-    "[cppapi][aggregates][basic][var][overflow]") {
+    "[cppapi][aggregates][var][overflow]") {
   generate_test_params();
   if (!nullable_) {
     return;
@@ -1931,7 +1931,8 @@ TEST_CASE_METHOD(
 
         // Add another aggregator on the second attribute. We will make the
         // first attribute get a var size overflow, which should not impact the
-        // results of the second attribute.
+        // results of the second attribute as we don't request data for the
+        // second attribute.
         query.ptr()->query_->add_aggregator_to_default_channel(
             "NullCount2",
             std::make_shared<tiledb::sm::NullCountAggregator>(
@@ -2054,7 +2055,7 @@ TEST_CASE_METHOD(
 TEST_CASE_METHOD(
     CppAggregatesFx<std::string>,
     "C++ API: Aggregates var overflow, exception",
-    "[cppapi][aggregates][basic][var][overflow-exception]") {
+    "[cppapi][aggregates][var][overflow-exception]") {
   generate_test_params();
   if (!nullable_ || dense_) {
     return;
@@ -2076,21 +2077,19 @@ TEST_CASE_METHOD(
         layout_ = layout;
         Query query(ctx_, array, TILEDB_READ);
 
-        // TODO: Change to real CPPAPI. Add a count aggregator to the query on
-        // the second attribute.
+        // TODO: Change to real CPPAPI. Add a count aggregator to the query.
         query.ptr()->query_->add_aggregator_to_default_channel(
-            "NullCount2",
+            "NullCount",
             std::make_shared<tiledb::sm::NullCountAggregator>(
-                tiledb::sm::FieldInfo("a2", true, nullable_, TILEDB_VAR_NUM)));
+                tiledb::sm::FieldInfo("a1", true, nullable_, TILEDB_VAR_NUM)));
 
-        // Add the count that will overflow in second, this will cause an
-        // exception because the result of the second attribute will be
-        // aggregated before the overflow and we don't have the recompute logic
-        // yet.
+        // Add another aggregator on the second attribute. We will make this
+        // attribute get a var size overflow, which should impact the result of
+        // the first one hence throw an exception.
         query.ptr()->query_->add_aggregator_to_default_channel(
-            "NullCount",
+            "NullCount2",
             std::make_shared<tiledb::sm::NullCountAggregator>(
-                tiledb::sm::FieldInfo("a1", true, nullable_, TILEDB_VAR_NUM)));
+                tiledb::sm::FieldInfo("a2", true, nullable_, TILEDB_VAR_NUM)));
 
         set_ranges_and_condition_if_needed(array, query, true);
 
@@ -2129,9 +2128,9 @@ TEST_CASE_METHOD(
 
         query.set_data_buffer("d1", dim1.data(), 100);
         query.set_data_buffer("d2", dim2.data(), 100);
-        query.set_data_buffer("a1", a1_data.data(), 10);
+        query.set_data_buffer("a1", a1_data.data(), 100);
         query.set_offsets_buffer("a1", a1_offsets.data(), 100);
-        query.set_data_buffer("a2", a2_data.data(), 100);
+        query.set_data_buffer("a2", a2_data.data(), 10);
         query.set_offsets_buffer("a2", a2_offsets.data(), 100);
 
         if (nullable_) {

diff --git a/tiledb/sm/query/readers/sparse_index_reader_base.cc b/tiledb/sm/query/readers/sparse_index_reader_base.cc
@@ -869,10 +869,18 @@ std::vector<std::string> SparseIndexReaderBase::field_names_to_process() {
   std::vector<std::string> ret;
   std::unordered_set<std::string> added_names;
 
+  // Guarantee the same ordering of buffers over different platform to guarantee
+  // that tests have consistent behaviors.
+  std::vector<std::string> names;
+  names.reserve(buffers_.size());
+  for (auto& buffer : buffers_) {
+    names.emplace_back(buffer.first);
+  }
+  std::sort(names.begin(), names.end());
+
   // First add var fields with no aggregates that need recompute in case of
   // overflow.
-  for (auto& buffer : buffers_) {
-    auto& name = buffer.first;
+  for (auto& name : names) {
     if (!array_schema_.var_size(name)) {
       continue;
     }
@@ -893,17 +901,15 @@ std::vector<std::string> SparseIndexReaderBase::field_names_to_process() {
   }
 
   // Second add the rest of the var fields.
-  for (auto& buffer : buffers_) {
-    auto& name = buffer.first;
+  for (auto& name : names) {
     if (array_schema_.var_size(name) && added_names.count(name) == 0) {
       ret.emplace_back(name);
       added_names.emplace(name);
     }
   }
 
   // Now for the fixed fields.
-  for (auto& buffer : buffers_) {
-    auto& name = buffer.first;
+  for (auto& name : names) {
     if (!array_schema_.var_size(name)) {
       ret.emplace_back(name);
       added_names.emplace(name);