Modify C++ API Query::est_result_size_var

This changes the return type and return values for `Query::est_result_size_var`. Currently, the size estimate for the values are returned in bytes while the size estimate for the offsets are returned in elements. For consistency, this API now returns both the size estimates as bytes. This is breaking behavior. To make the breaking behavior more obvious, we have modified the return type from an `std::pair` to an `std::array`. We chose to make both of these size estimates return as bytes rather than elements to stay consistent with the existing `Query::est_result_size` for fixed-size attributes.
TileDB-Inc · Dec 7, 2020 · 5b6f175 · 5b6f175
1 parent e7a1287
commit 5b6f175
Show file tree

Hide file tree

Showing 5 changed files with 36 additions and 26 deletions.
diff --git a/HISTORY.md b/HISTORY.md
@@ -7,6 +7,7 @@
 ## Breaking behavior
 
 * The tile extent can now be set to null, in which case internally TileDB sets the extent to the dimension domain range. [#1880](https://github.com/TileDB-Inc/TileDB/pull/1880)
+* The C++ API `std::pair<uint64_t, uint64_t> Query::est_result_size_var` has been changed to 1) a return type of `std::array<uint64_t, 2>` and 2) returns the offsets as a size in bytes rather than elements. [#1946](https://github.com/TileDB-Inc/TileDB/pull/1946)
 
 ## New features
 
@@ -54,6 +55,7 @@
 * Added class `FragmentInfo` and functions for getting fragment information. [#1900](https://github.com/TileDB-Inc/TileDB/pull/1900)
 * Added function `Dimension::create` that allows not setting a space tile extent. [#1880](https://github.com/TileDB-Inc/TileDB/pull/1880)
 * Added APIs for getting and setting ranges of queries using a dimension name. [#1920](https://github.com/TileDB-Inc/TileDB/pull/1920)
+* Changed `std::pair<uint64_t, uint64_t> Query::est_result_size_var` to `std::array<uint64_t, 2> Query::est_result_size_var`. Additionally, the size estimate for the offsets have been changed from elements to bytes. [#1946](https://github.com/TileDB-Inc/TileDB/pull/1946)
 
 # TileDB v2.1.0 Release Notes
 

diff --git a/test/src/unit-arrow.cc b/test/src/unit-arrow.cc
@@ -179,11 +179,15 @@ void allocate_query_buffers(std::shared_ptr<tiledb::Query> query) {
       query->set_buffer(name, data, est_size);
     } else {
       auto est_size_var = query->est_result_size_var(attr.name());
-      void* data = std::malloc(est_size_var.second);
+      void* data = std::malloc(std::get<1>(est_size_var));
       uint64_t* offsets = static_cast<uint64_t*>(
-          std::malloc(est_size_var.first * sizeof(uint64_t)));
+          std::malloc(std::get<0>(est_size_var) * sizeof(uint64_t)));
       query->set_buffer(
-          name, offsets, est_size_var.first, data, est_size_var.second);
+          name,
+          offsets,
+          std::get<0>(est_size_var),
+          data,
+          std::get<1>(est_size_var));
     }
   }
 
@@ -196,11 +200,15 @@ void allocate_query_buffers(std::shared_ptr<tiledb::Query> query) {
       query->set_buffer(name, data, est_size);
     } else {
       auto est_size_var = query->est_result_size_var(dim.name());
-      void* data = std::malloc(est_size_var.second);
+      void* data = std::malloc(std::get<1>(est_size_var));
       uint64_t* offsets = static_cast<uint64_t*>(
-          std::malloc(est_size_var.first * sizeof(uint64_t)));
+          std::malloc(std::get<0>(est_size_var) * sizeof(uint64_t)));
       query->set_buffer(
-          name, offsets, est_size_var.first, data, est_size_var.second);
+          name,
+          offsets,
+          std::get<0>(est_size_var),
+          data,
+          std::get<1>(est_size_var));
     }
   }
 }
@@ -367,4 +375,4 @@ TEST_CASE("Arrow IO integration tests", "[arrow]") {
 
     free_query_buffers(query);
   }
-}
+}
diff --git a/test/src/unit-cppapi-fill_values.cc b/test/src/unit-cppapi-fill_values.cc
@@ -544,8 +544,8 @@ TEST_CASE(
     auto est_d = query.est_result_size("d");
     CHECK(est_d == 10 * sizeof(int32_t));
     CHECK(est_a1 == 10 * sizeof(int32_t));
-    CHECK(est_a2.first == 10);
-    CHECK(est_a2.second == 10 * sizeof(char));
+    CHECK(est_a2[0] == 80);
+    CHECK(est_a2[1] == 10 * sizeof(char));
     CHECK(est_a3 == 10 * 2 * sizeof(double));
   }
 
@@ -561,8 +561,8 @@ TEST_CASE(
     auto est_d = query.est_result_size("d");
     CHECK(est_d == 10 * sizeof(int32_t));
     CHECK(est_a1 == 10 * sizeof(int32_t));
-    CHECK(est_a2.first == 10);
-    CHECK(est_a2.second == 10 * 3 * sizeof(char));
+    CHECK(est_a2[0] == 80);
+    CHECK(est_a2[1] == 10 * 3 * sizeof(char));
     CHECK(est_a3 == 10 * 2 * sizeof(double));
   }
 
@@ -579,8 +579,8 @@ TEST_CASE(
     auto est_d = query.est_result_size("d");
     CHECK(est_d == 4 * sizeof(int32_t));
     CHECK(est_a1 == 4 * sizeof(int32_t));
-    CHECK(est_a2.first == 4);
-    CHECK(est_a2.second == 4 * sizeof(char));
+    CHECK(est_a2[0] == 32);
+    CHECK(est_a2[1] == 4 * sizeof(char));
     CHECK(est_a3 == 4 * 2 * sizeof(double));
   }
 
@@ -610,8 +610,8 @@ TEST_CASE(
     auto est_d = query.est_result_size("d");
     CHECK(est_d == 10 * sizeof(int32_t));
     CHECK(est_a1 == 10 * sizeof(int32_t));
-    CHECK(est_a2.first == 10);
-    CHECK(est_a2.second == 10 * sizeof(char));
+    CHECK(est_a2[0] == 80);
+    CHECK(est_a2[1] == 10 * sizeof(char));
     CHECK(est_a3 == 10 * 2 * sizeof(double));
   }
 
@@ -628,8 +628,8 @@ TEST_CASE(
     auto est_d = query.est_result_size("d");
     CHECK(est_d == 10 * sizeof(int32_t));
     CHECK(est_a1 == 10 * sizeof(int32_t));
-    CHECK(est_a2.first == 10);
-    CHECK(est_a2.second == 10 * 3 * sizeof(char));
+    CHECK(est_a2[0] == 80);
+    CHECK(est_a2[1] == 10 * 3 * sizeof(char));
     CHECK(est_a3 == 10 * 2 * sizeof(double));
   }
 
@@ -647,8 +647,8 @@ TEST_CASE(
     auto est_d = query.est_result_size("d");
     CHECK(est_d == 4 * sizeof(int32_t));
     CHECK(est_a1 == 4 * sizeof(int32_t));
-    CHECK(est_a2.first == 4);
-    CHECK(est_a2.second == 4 * sizeof(char));
+    CHECK(est_a2[0] == 32);
+    CHECK(est_a2[1] == 4 * sizeof(char));
     CHECK(est_a3 == 4 * 2 * sizeof(double));
   }
 

diff --git a/test/src/unit-cppapi-subarray.cc b/test/src/unit-cppapi-subarray.cc
@@ -90,7 +90,7 @@ TEST_CASE("C++ API: Test subarray", "[cppapi][sparse][subarray]") {
 
     auto est_size = query.est_result_size("a");
     REQUIRE(est_size == 4);
-    std::pair<uint64_t, uint64_t> est_size_var;
+    std::array<uint64_t, 2> est_size_var;
     CHECK_THROWS(est_size_var = query.est_result_size_var("a"));
 
     std::vector<int> data(est_size);

diff --git a/tiledb/sm/cpp_api/query.h b/tiledb/sm/cpp_api/query.h
@@ -853,16 +853,16 @@ class Query {
    * **Example:**
    *
    * @code{.cpp}
-   * std::pair<uint64_t, uint64_t> est_size =
+   * std::array<uint64_t, 2> est_size =
    *     query.est_result_size_var("attr1");
    * @endcode
    *
    * @param attr_name The attribute name.
-   * @return A pair with first element containing the estimated number of
-   *    result offsets, and second element containing the estimated number of
-   *    result value bytes.
+   * @return A pair with first element containing the estimated size of
+   *    the result offsets in bytes, and second element containing the
+   *    estimated size of the result values in bytes.
    */
-  std::pair<uint64_t, uint64_t> est_result_size_var(
+  std::array<uint64_t, 2> est_result_size_var(
       const std::string& attr_name) const {
     auto& ctx = ctx_.get();
     uint64_t size_off = 0, size_val = 0;
@@ -872,7 +872,7 @@ class Query {
         attr_name.c_str(),
         &size_off,
         &size_val));
-    return std::make_pair(size_off / sizeof(uint64_t), size_val);
+    return {size_off, size_val};
   }
 
   /**