Sparse global order reader: Check correct incomplete reason in case o…

…f too small user buffer (#3170) * Add error message in case of empty result buffer * Use incomplete reason instead of error message
TileDB-Inc · May 17, 2022 · 592185f · 592185f
1 parent 0431773
commit 592185f
Show file tree

Hide file tree

Showing 5 changed files with 194 additions and 23 deletions.
diff --git a/test/src/unit-capi-incomplete.cc b/test/src/unit-capi-incomplete.cc
@@ -1131,6 +1131,11 @@ void IncompleteFx::check_sparse_unsplittable_overflow() {
   CHECK(status == TILEDB_INCOMPLETE);
   CHECK(buffer_sizes[0] == 0);
 
+  tiledb_query_status_details_t details;
+  rc = tiledb_query_get_status_details(ctx_, query, &details);
+  CHECK(rc == TILEDB_OK);
+  CHECK(details.incomplete_reason == TILEDB_REASON_USER_BUFFER_SIZE);
+
   // Finalize query
   rc = tiledb_query_finalize(ctx_, query);
   REQUIRE(rc == TILEDB_OK);

diff --git a/test/src/unit-capi-string_dims.cc b/test/src/unit-capi-string_dims.cc
@@ -151,7 +151,8 @@ struct StringDimsFx {
       std::vector<uint64_t>* d_off,
       std::string* d_val,
       std::vector<int32_t>* a,
-      tiledb_query_status_t* status);
+      tiledb_query_status_t* status,
+      tiledb_query_status_details_t* details);
   void read_array_2d(
       tiledb_ctx_t* ctx,
       tiledb_array_t* array,
@@ -964,7 +965,8 @@ void StringDimsFx::read_array_1d(
     std::vector<uint64_t>* d_off,
     std::string* d_val,
     std::vector<int32_t>* a,
-    tiledb_query_status_t* status) {
+    tiledb_query_status_t* status,
+    tiledb_query_status_details_t* details) {
   // Create query
   tiledb_query_t* query;
   int rc = tiledb_query_alloc(ctx, array, TILEDB_READ, &query);
@@ -1030,6 +1032,10 @@ void StringDimsFx::read_array_1d(
   rc = tiledb_query_get_status(ctx, query, status);
   CHECK(rc == TILEDB_OK);
 
+  // Get details
+  rc = tiledb_query_get_status_details(ctx, query, details);
+  CHECK(rc == TILEDB_OK);
+
   // Resize the result buffers
   d_off->resize(d_off_size / sizeof(uint64_t));
   d_val->resize(d_val_size / sizeof(char));
@@ -1802,8 +1808,18 @@ TEST_CASE_METHOD(
   r_d_val.resize(20);
   std::vector<int32_t> r_a(10);
   tiledb_query_status_t status;
+  tiledb_query_status_details_t details;
   read_array_1d(
-      ctx_, array, layout, "a", "ee", &r_d_off, &r_d_val, &r_a, &status);
+      ctx_,
+      array,
+      layout,
+      "a",
+      "ee",
+      &r_d_off,
+      &r_d_val,
+      &r_a,
+      &status,
+      &details);
   CHECK(status == TILEDB_COMPLETED);
   CHECK(r_d_val == "aabbccdddd");
   std::vector<uint64_t> c_d_off = {0, 2, 4, 6};
@@ -1816,7 +1832,16 @@ TEST_CASE_METHOD(
   r_d_val.resize(20);
   r_a.resize(10);
   read_array_1d(
-      ctx_, array, layout, "aab", "cc", &r_d_off, &r_d_val, &r_a, &status);
+      ctx_,
+      array,
+      layout,
+      "aab",
+      "cc",
+      &r_d_off,
+      &r_d_val,
+      &r_a,
+      &status,
+      &details);
   CHECK(status == TILEDB_COMPLETED);
   CHECK(r_d_val == "bbcc");
   c_d_off = {0, 2};
@@ -1829,24 +1854,44 @@ TEST_CASE_METHOD(
   r_d_val.resize(20);
   r_a.resize(10);
   read_array_1d(
-      ctx_, array, layout, "aa", "cc", &r_d_off, &r_d_val, &r_a, &status);
+      ctx_,
+      array,
+      layout,
+      "aa",
+      "cc",
+      &r_d_off,
+      &r_d_val,
+      &r_a,
+      &status,
+      &details);
   CHECK(status == TILEDB_INCOMPLETE);
   CHECK(r_d_val == "aabb");
   c_d_off = {0, 2};
   CHECK(r_d_off == c_d_off);
   c_a = {1, 2};
   CHECK(r_a == c_a);
+  CHECK(details.incomplete_reason == TILEDB_REASON_USER_BUFFER_SIZE);
 
   // Read [aa, cc] - INCOMPLETE, no result
   r_d_off.resize(1);
   r_d_val.resize(1);
   r_a.resize(10);
   read_array_1d(
-      ctx_, array, layout, "aa", "bb", &r_d_off, &r_d_val, &r_a, &status);
+      ctx_,
+      array,
+      layout,
+      "aa",
+      "bb",
+      &r_d_off,
+      &r_d_val,
+      &r_a,
+      &status,
+      &details);
   CHECK(status == TILEDB_INCOMPLETE);
   CHECK(r_d_val.size() == 0);
   CHECK(r_d_off.size() == 0);
   CHECK(r_a.size() == 0);
+  CHECK(details.incomplete_reason == TILEDB_REASON_USER_BUFFER_SIZE);
 
   // Close array
   rc = tiledb_array_close(ctx_, array);
@@ -1946,8 +1991,18 @@ TEST_CASE_METHOD(
   r_d_val.resize(20);
   std::vector<int32_t> r_a(10);
   tiledb_query_status_t status;
+  tiledb_query_status_details_t details;
   read_array_1d(
-      ctx_, array, layout, "a", "ee", &r_d_off, &r_d_val, &r_a, &status);
+      ctx_,
+      array,
+      layout,
+      "a",
+      "ee",
+      &r_d_off,
+      &r_d_val,
+      &r_a,
+      &status,
+      &details);
   CHECK(status == TILEDB_COMPLETED);
   CHECK(r_d_val == "aaabbbccddddee");
   std::vector<uint64_t> c_d_off = {0, 1, 3, 4, 6, 8, 12};
@@ -1999,7 +2054,16 @@ TEST_CASE_METHOD(
   r_d_val.resize(20);
   r_a.resize(10);
   read_array_1d(
-      ctx_, array, layout, "a", "ee", &r_d_off, &r_d_val, &r_a, &status);
+      ctx_,
+      array,
+      layout,
+      "a",
+      "ee",
+      &r_d_off,
+      &r_d_val,
+      &r_a,
+      &status,
+      &details);
   CHECK(status == TILEDB_COMPLETED);
   CHECK(r_d_val == "aaabbbccddddee");
   CHECK(r_d_off == c_d_off);
@@ -2093,8 +2157,18 @@ TEST_CASE_METHOD(
   r_d_val.resize(20);
   std::vector<int32_t> r_a(10);
   tiledb_query_status_t status;
+  tiledb_query_status_details_t details;
   read_array_1d(
-      ctx_, array, layout, "a", "e", &r_d_off, &r_d_val, &r_a, &status);
+      ctx_,
+      array,
+      layout,
+      "a",
+      "e",
+      &r_d_off,
+      &r_d_val,
+      &r_a,
+      &status,
+      &details);
   CHECK(status == TILEDB_COMPLETED);
   CHECK(r_d_val == "aaccccdddd");
   std::vector<uint64_t> c_d_off = {0, 2, 4, 6};
@@ -2211,8 +2285,18 @@ TEST_CASE_METHOD(
   r_d_val.resize(20);
   std::vector<int32_t> r_a(10);
   tiledb_query_status_t status;
+  tiledb_query_status_details_t details;
   read_array_1d(
-      ctx, array, layout, "a", "e", &r_d_off, &r_d_val, &r_a, &status);
+      ctx,
+      array,
+      layout,
+      "a",
+      "e",
+      &r_d_off,
+      &r_d_val,
+      &r_a,
+      &status,
+      &details);
   CHECK(status == TILEDB_COMPLETED);
   CHECK(r_d_val == "aaccdddd");
   std::vector<uint64_t> c_d_off = {0, 2, 4};
@@ -2587,6 +2671,7 @@ TEST_CASE_METHOD(
   r_d_val.resize(20);
   std::vector<int32_t> r_a(10);
   tiledb_query_status_t status;
+  tiledb_query_status_details_t details;
   read_array_1d(
       ctx_,
       array,
@@ -2596,7 +2681,8 @@ TEST_CASE_METHOD(
       &r_d_off,
       &r_d_val,
       &r_a,
-      &status);
+      &status,
+      &details);
   CHECK(status == TILEDB_COMPLETED);
   CHECK(r_d_val == "ab");
   std::vector<uint64_t> c_d_off = {0, 1};
@@ -2716,6 +2802,7 @@ TEST_CASE_METHOD(
   r_d_val.resize(20);
   std::vector<int32_t> r_a(10);
   tiledb_query_status_t status;
+  tiledb_query_status_details_t details;
   read_array_1d(
       ctx_,
       array,
@@ -2725,7 +2812,8 @@ TEST_CASE_METHOD(
       &r_d_off,
       &r_d_val,
       &r_a,
-      &status);
+      &status,
+      &details);
   CHECK(status == TILEDB_COMPLETED);
   CHECK(r_d_val == "abcde");
   std::vector<uint64_t> c_d_off = {0, 1, 2, 3, 4};

diff --git a/test/src/unit-sparse-global-order-reader.cc b/test/src/unit-sparse-global-order-reader.cc
@@ -33,6 +33,7 @@
 #include "test/src/helpers.h"
 #include "tiledb/sm/c_api/tiledb.h"
 #include "tiledb/sm/c_api/tiledb_struct_def.h"
+#include "tiledb/sm/cpp_api/tiledb"
 #include "tiledb/sm/query/sparse_global_order_reader.h"
 
 #ifdef _WIN32
@@ -43,7 +44,7 @@
 
 #include <catch.hpp>
 
-using namespace tiledb::sm;
+using namespace tiledb;
 using namespace tiledb::test;
 
 /* ********************************* */
@@ -449,7 +450,8 @@ TEST_CASE_METHOD(
   CHECK(rc == TILEDB_OK);
 
   // Check the internal loop count against expected value.
-  auto stats = ((SparseGlobalOrderReader*)query->query_->strategy())->stats();
+  auto stats =
+      ((sm::SparseGlobalOrderReader*)query->query_->strategy())->stats();
   REQUIRE(stats != nullptr);
   auto counters = stats->counters();
   REQUIRE(counters != nullptr);
@@ -643,7 +645,8 @@ TEST_CASE_METHOD(
   CHECK(rc == TILEDB_OK);
 
   // Check the internal loop count against expected value.
-  auto stats = ((SparseGlobalOrderReader*)query->query_->strategy())->stats();
+  auto stats =
+      ((sm::SparseGlobalOrderReader*)query->query_->strategy())->stats();
   REQUIRE(stats != nullptr);
   auto counters = stats->counters();
   REQUIRE(counters != nullptr);
@@ -801,4 +804,83 @@ TEST_CASE_METHOD(
   int data_c[] = {8, 8, 9, 9, 10, 10};
   CHECK(!std::memcmp(coords_c, coords_r, coords_r_size));
   CHECK(!std::memcmp(data_c, data_r, data_r_size));
+}
+
+TEST_CASE(
+    "Sparse global order reader: user buffer cannot fit single cell",
+    "[sparse-global-order][user-buffer][too-small]") {
+  std::string array_name = "test_sparse_global_order";
+  Context ctx;
+  VFS vfs(ctx);
+
+  if (vfs.is_dir(array_name)) {
+    vfs.remove_dir(array_name);
+  }
+
+  // Create array with var-sized attribute.
+  Domain dom(ctx);
+  dom.add_dimension(Dimension::create<int64_t>(ctx, "d1", {{1, 4}}, 2));
+
+  Attribute attr(ctx, "a", TILEDB_STRING_ASCII);
+  attr.set_cell_val_num(TILEDB_VAR_NUM);
+
+  ArraySchema schema(ctx, TILEDB_SPARSE);
+  schema.add_attribute(attr);
+  schema.set_tile_order(TILEDB_ROW_MAJOR);
+  schema.set_cell_order(TILEDB_ROW_MAJOR);
+  schema.set_domain(dom);
+  schema.set_allows_dups(true);
+
+  Array::create(array_name, schema);
+
+  // Write a fragment.
+  Array array(ctx, array_name, TILEDB_WRITE);
+  Query query(ctx, array, TILEDB_WRITE);
+  query.set_layout(TILEDB_GLOBAL_ORDER);
+  std::vector<int64_t> d1 = {1, 2, 3};
+  std::string a1_data{
+      "astringofsize15"
+      "foo"
+      "bar"};
+  std::vector<uint64_t> a1_offsets{0, 15, 18};
+
+  query.set_data_buffer("d1", d1);
+  query.set_data_buffer("a", a1_data);
+  query.set_offsets_buffer("a", a1_offsets);
+  CHECK_NOTHROW(query.submit());
+
+  // Finalize is necessary in global writes, otherwise a no-op
+  query.finalize();
+
+  // Read using a buffer that can't fit a single result
+  Array array2(ctx, array_name, TILEDB_READ);
+  Query query2(ctx, array2, TILEDB_READ);
+
+  std::string attr_val;
+  // The first result is 15 bytes long so it won't fit in 10 byte user buffer
+  attr_val.resize(10);
+  std::vector<uint64_t> attr_off(sizeof(uint64_t));
+
+  auto layout = GENERATE(TILEDB_GLOBAL_ORDER, TILEDB_UNORDERED);
+
+  query2.set_layout(layout);
+  query2.set_data_buffer("a", (char*)attr_val.data(), attr_val.size());
+  query2.set_offsets_buffer("a", attr_off);
+
+  // The user buffer cannot fit a single result so it should return Incomplete
+  // with the right reason
+  auto st = query2.submit();
+  CHECK(st == Query::Status::INCOMPLETE);
+
+  tiledb_query_status_details_t details;
+  int rc = tiledb_query_get_status_details(
+      ctx.ptr().get(), query2.ptr().get(), &details);
+  CHECK(rc == TILEDB_OK);
+  CHECK(details.incomplete_reason == TILEDB_REASON_USER_BUFFER_SIZE);
+
+  array2.close();
+
+  if (vfs.is_dir(array_name)) {
+    vfs.remove_dir(array_name);
+  }
 }
diff --git a/tiledb/sm/enums/query_status_details.h b/tiledb/sm/enums/query_status_details.h
@@ -1,5 +1,5 @@
 /**
- * @file query_status.h
+ * @file query_status_details.h
  *
  * @section LICENSE
  *
@@ -27,8 +27,8 @@
  *
  * @section DESCRIPTION
  *
- * This defines the tiledb QueryStatus enum that maps to tiledb_query_status_t
- * C-api enum.
+ * This defines the tiledb QueryStatusDetailsReason enum that maps to
+ * tiledb_query_status_details_t C-api enum.
  */
 
 #ifndef TILEDB_QUERY_STATUS_DETAILS_H