Enable current domain on dense arrays. (#5303)

This PR enables the current domain feature in dense arrays --- TYPE: FEATURE DESC: Support current domain on dense arrays. --------- Co-authored-by: Theodore Tsirpanis <[email protected]>
TileDB-Inc · Oct 15, 2024 · d632325 · d632325
1 parent 39c665a
commit d632325
Show file tree

Hide file tree

Showing 4 changed files with 256 additions and 42 deletions.
diff --git a/test/src/test-cppapi-current-domain.cc b/test/src/test-cppapi-current-domain.cc
@@ -153,13 +153,15 @@ TEST_CASE_METHOD(
     CurrentDomainFx,
     "C++ API: CurrentDomain - Add to ArraySchema",
     "[cppapi][ArraySchema][currentDomain]") {
+  tiledb_array_type_t type = GENERATE(TILEDB_DENSE, TILEDB_SPARSE);
+
   // Create domain.
   tiledb::Domain domain(ctx_);
   auto d = tiledb::Dimension::create<int32_t>(ctx_, "d", {{1, 999}}, 2);
   domain.add_dimension(d);
 
   // Create array schema.
-  tiledb::ArraySchema schema(ctx_, TILEDB_SPARSE);
+  tiledb::ArraySchema schema(ctx_, type);
   schema.set_domain(domain);
   schema.add_attribute(tiledb::Attribute::create<int>(ctx_, "a"));
 
@@ -191,6 +193,8 @@ TEST_CASE_METHOD(
     "[cppapi][ArraySchema][currentDomain]") {
   const std::string array_name = "test_current_domain_expansion";
 
+  tiledb_array_type_t type = GENERATE(TILEDB_DENSE, TILEDB_SPARSE);
+
   tiledb::VFS vfs(ctx_);
   if (vfs.is_dir(array_name)) {
     vfs.remove_dir(array_name);
@@ -202,7 +206,7 @@ TEST_CASE_METHOD(
   domain.add_dimension(d);
 
   // Create array schema.
-  tiledb::ArraySchema schema(ctx_, TILEDB_SPARSE);
+  tiledb::ArraySchema schema(ctx_, type);
   schema.set_domain(domain);
   schema.add_attribute(tiledb::Attribute::create<int>(ctx_, "a"));
 
@@ -365,7 +369,7 @@ TEST_CASE_METHOD(
 TEST_CASE_METHOD(
     CurrentDomainFx,
     "C++ API: CurrentDomain - Read cells written outside of shape",
-    "[cppapi][ArraySchema][currentDomain][luc]") {
+    "[cppapi][ArraySchema][currentDomain]") {
   const std::string array_name = "test_current_domain_read";
 
   tiledb::VFS vfs(ctx_);
@@ -488,3 +492,252 @@ TEST_CASE_METHOD(
     vfs.remove_dir(array_name);
   }
 }
+
+TEST_CASE_METHOD(
+    CurrentDomainFx,
+    "C++ API: CurrentDomain - Dense array basic",
+    "[cppapi][ArraySchema][currentDomain]") {
+  const std::string array_name = "test_current_domain_read_dense";
+
+  tiledb::VFS vfs(ctx_);
+  if (vfs.is_dir(array_name)) {
+    vfs.remove_dir(array_name);
+  }
+
+  // Create domain
+  tiledb::Domain domain(ctx_);
+  auto d1 = tiledb::Dimension::create<int32_t>(ctx_, "dim1", {{1, 10}}, 1);
+  domain.add_dimension(d1);
+
+  // Create array schema.
+  tiledb::ArraySchema schema(ctx_, TILEDB_DENSE);
+  schema.set_domain(domain);
+  schema.add_attribute(tiledb::Attribute::create<int>(ctx_, "a"));
+
+  // Create array
+  tiledb::Array::create(array_name, schema);
+
+  tiledb::Array array_for_writes(ctx_, array_name, TILEDB_WRITE);
+  // Populate array with data from 1 to 10. Some of the data here is outside of
+  // the current domain we will set later.
+  tiledb::Query query_for_writes(ctx_, array_for_writes);
+  query_for_writes.set_layout(TILEDB_ROW_MAJOR);
+  tiledb::Subarray sub_for_writes(ctx_, array_for_writes);
+  sub_for_writes.set_subarray({1, 10});
+  query_for_writes.set_subarray(sub_for_writes);
+  std::vector<int32_t> data = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10};
+  query_for_writes.set_data_buffer("a", data);
+  query_for_writes.submit();
+  array_for_writes.close();
+
+  // Read data to validate
+  tiledb::Array array(ctx_, array_name, TILEDB_READ);
+  tiledb::Subarray sub(ctx_, array);
+  sub.set_subarray({1, 10});
+  std::vector<int32_t> a(10);
+  tiledb::Query query(ctx_, array, TILEDB_READ);
+  query.set_subarray(sub).set_layout(TILEDB_ROW_MAJOR).set_data_buffer("a", a);
+  query.submit();
+  array.close();
+
+  // Check values
+  for (int i = 0; i < 9; i++) {
+    CHECK(a[i] == i + 1);
+  }
+
+  // Create new currentDomain
+  tiledb::CurrentDomain current_domain_ev(ctx_);
+  int range_two[] = {2, 5};
+  tiledb::NDRectangle ndrect_two(ctx_, domain);
+  ndrect_two.set_range(0, range_two[0], range_two[1]);
+  current_domain_ev.set_ndrectangle(ndrect_two);
+
+  // Schema evolution
+  tiledb::ArraySchemaEvolution se(ctx_);
+  se.expand_current_domain(current_domain_ev);
+  se.array_evolve(array_name);
+
+  // Re-read data which is included in the current domain to validate
+  tiledb::Array array_with_cd(ctx_, array_name, TILEDB_READ);
+  tiledb::Subarray sub_for_cd(ctx_, array_with_cd);
+  sub_for_cd.set_subarray({2, 5});
+  std::vector<int32_t> a_with_cd(100);
+  std::vector<int32_t> dim1_with_cd(100);
+  tiledb::Query query_for_cd(ctx_, array_with_cd, TILEDB_READ);
+  query_for_cd.set_subarray(sub_for_cd)
+      .set_layout(TILEDB_ROW_MAJOR)
+      .set_data_buffer("a", a_with_cd)
+      .set_data_buffer("dim1", dim1_with_cd);
+  query_for_cd.submit();
+  array_with_cd.close();
+
+  // Validate we got four results.
+  auto res = query_for_cd.result_buffer_elements();
+  CHECK(res["a"].second == 4);
+  CHECK(res["dim1"].second == 4);
+
+  // Try to read data outside the current domain and fail
+  tiledb::Array array_with_cd2(ctx_, array_name, TILEDB_READ);
+  tiledb::Subarray sub_for_cd_wrong(ctx_, array_with_cd2);
+  sub_for_cd_wrong.set_subarray({2, 6});
+  std::vector<int32_t> a_with_cd2(100);
+  std::vector<int32_t> dim1_with_cd2(100);
+  tiledb::Query query_for_cd2(ctx_, array_with_cd2, TILEDB_READ);
+  query_for_cd2.set_subarray(sub_for_cd_wrong)
+      .set_layout(TILEDB_ROW_MAJOR)
+      .set_data_buffer("a", a_with_cd2)
+      .set_data_buffer("dim1", dim1_with_cd2);
+  auto matcher = Catch::Matchers::ContainsSubstring(
+      "A range was set outside of the current domain.");
+  REQUIRE_THROWS_WITH(query_for_cd2.submit(), matcher);
+  array_with_cd2.close();
+
+  // Clean up.
+  if (vfs.is_dir(array_name)) {
+    vfs.remove_dir(array_name);
+  }
+}
+
+TEST_CASE_METHOD(
+    CurrentDomainFx,
+    "C++ API: CurrentDomain - Dense array current domain expand",
+    "[cppapi][ArraySchema][currentDomain]") {
+  const std::string array_name = "test_current_domain_read_dense";
+
+  std::string matcher_string;
+  bool throws = false;
+  bool shrink = false;
+  SECTION("Expand outside domain bounds") {
+    throws = true;
+    shrink = false;
+    matcher_string =
+        "This array current domain has ranges past the boundaries of the array "
+        "schema domain";
+  }
+
+  SECTION("Shrink domain") {
+    throws = true;
+    shrink = true;
+    matcher_string =
+        "The current domain of an array can only be expanded, please adjust "
+        "your new current domain object";
+  }
+
+  SECTION("Expand correctly") {
+    // do nothing
+  }
+
+  tiledb::VFS vfs(ctx_);
+  if (vfs.is_dir(array_name)) {
+    vfs.remove_dir(array_name);
+  }
+
+  // Create domain
+  tiledb::Domain domain(ctx_);
+  auto d1 = tiledb::Dimension::create<int32_t>(ctx_, "dim1", {{1, 10}}, 1);
+  domain.add_dimension(d1);
+
+  // Create array schema.
+  tiledb::ArraySchema schema(ctx_, TILEDB_DENSE);
+  schema.set_domain(domain);
+  schema.add_attribute(tiledb::Attribute::create<int>(ctx_, "a"));
+
+  // Create and set new currentDomain
+  tiledb::CurrentDomain current_domain(ctx_);
+  int range[] = {2, 5};
+  tiledb::NDRectangle ndrect(ctx_, domain);
+  ndrect.set_range(0, range[0], range[1]);
+  current_domain.set_ndrectangle(ndrect);
+  CHECK_NOTHROW(tiledb::ArraySchemaExperimental::set_current_domain(
+      ctx_, schema, current_domain));
+
+  // Create array
+  tiledb::Array::create(array_name, schema);
+
+  // Create new currentDomain to expand
+  tiledb::CurrentDomain current_domain_ev(ctx_);
+  std::vector<int> range_two;
+  if (throws) {
+    if (shrink) {
+      range_two = {2, 3};
+    } else {
+      range_two = {2, 11};
+    }
+  } else {
+    range_two = {2, 7};
+  }
+  tiledb::NDRectangle ndrect_two(ctx_, domain);
+  ndrect_two.set_range(0, range_two[0], range_two[1]);
+  current_domain_ev.set_ndrectangle(ndrect_two);
+
+  // Schema evolution
+  tiledb::ArraySchemaEvolution se(ctx_);
+  se.expand_current_domain(current_domain_ev);
+
+  // Check the correct exceptions are being thrown
+  if (throws) {
+    auto matcher = Catch::Matchers::ContainsSubstring(matcher_string);
+    REQUIRE_THROWS_WITH(se.array_evolve(array_name), matcher);
+  } else {
+    REQUIRE_NOTHROW(se.array_evolve(array_name));
+  }
+
+  // Clean up.
+  if (vfs.is_dir(array_name)) {
+    vfs.remove_dir(array_name);
+  }
+}
+
+TEST_CASE_METHOD(
+    CurrentDomainFx,
+    "C++ API: CurrentDomain - Dense array write outside current domain",
+    "[cppapi][ArraySchema][currentDomain]") {
+  const std::string array_name = "test_current_domain_read_dense";
+
+  tiledb::VFS vfs(ctx_);
+  if (vfs.is_dir(array_name)) {
+    vfs.remove_dir(array_name);
+  }
+
+  // Create domain
+  tiledb::Domain domain(ctx_);
+  auto d1 = tiledb::Dimension::create<int32_t>(ctx_, "dim1", {{1, 10}}, 1);
+  domain.add_dimension(d1);
+
+  // Create array schema.
+  tiledb::ArraySchema schema(ctx_, TILEDB_DENSE);
+  schema.set_domain(domain);
+  schema.add_attribute(tiledb::Attribute::create<int>(ctx_, "a"));
+
+  // Create and set new currentDomain
+  tiledb::CurrentDomain current_domain(ctx_);
+  int range[] = {2, 5};
+  tiledb::NDRectangle ndrect(ctx_, domain);
+  ndrect.set_range(0, range[0], range[1]);
+  current_domain.set_ndrectangle(ndrect);
+  CHECK_NOTHROW(tiledb::ArraySchemaExperimental::set_current_domain(
+      ctx_, schema, current_domain));
+
+  // Create array
+  tiledb::Array::create(array_name, schema);
+
+  tiledb::Array array_for_writes(ctx_, array_name, TILEDB_WRITE);
+  // Populate array with data from 1 to 10. Some of the data here is outside of
+  // the current domain so we expect to fail.
+  tiledb::Query query_for_writes(ctx_, array_for_writes);
+  query_for_writes.set_layout(TILEDB_ROW_MAJOR);
+  tiledb::Subarray sub_for_writes(ctx_, array_for_writes);
+  sub_for_writes.set_subarray({1, 10});
+  query_for_writes.set_subarray(sub_for_writes);
+  std::vector<int32_t> data = {1, 2, 3, 4, 5, 6, 7, 8, 9, 10};
+  query_for_writes.set_data_buffer("a", data);
+  auto matcher = Catch::Matchers::ContainsSubstring(
+      "Cells are written outside of the defined current domain.");
+  REQUIRE_THROWS_WITH(query_for_writes.submit(), matcher);
+  array_for_writes.close();
+
+  // Clean up.
+  if (vfs.is_dir(array_name)) {
+    vfs.remove_dir(array_name);
+  }
+}
diff --git a/tiledb/api/c_api/array_schema/array_schema_api_experimental.h b/tiledb/api/c_api/array_schema/array_schema_api_experimental.h
@@ -101,9 +101,6 @@ TILEDB_EXPORT capi_return_t tiledb_array_schema_add_enumeration(
 /**
  * Sets the current domain on the array schema.
  *
- * @pre The schema is sparse. current_domain is not yet supported on dense
- * arrays.
- *
  * **Example:**
  *
  * @code{.c}

diff --git a/tiledb/sm/array_schema/array_schema.cc b/tiledb/sm/array_schema/array_schema.cc
@@ -1668,12 +1668,6 @@ void ArraySchema::expand_current_domain(
         "The argument specified for current domain expansion is nullptr.");
   }
 
-  if (this->dense()) {
-    throw ArraySchemaException(
-        "Expanding the current domain on a TileDB dense array is not "
-        "supported.");
-  }
-
   // Check that the new current domain expands the existing one and not shrinks
   // it. Every current domain covers an empty current domain.
   if (!current_domain_->empty() &&
@@ -1699,11 +1693,6 @@ void ArraySchema::set_current_domain(shared_ptr<CurrentDomain> current_domain) {
         "The argument specified for setting the current domain on the "
         "schema is nullptr.");
   }
-  if (this->dense()) {
-    throw ArraySchemaException(
-        "Setting a current domain on a TileDB dense array is not supported.");
-  }
-
   current_domain_ = current_domain;
 }
 

diff --git a/tiledb/sm/array_schema/test/unit_current_domain.cc b/tiledb/sm/array_schema/test/unit_current_domain.cc
@@ -286,31 +286,6 @@ shared_ptr<ArraySchema> CurrentDomainFx<T>::get_array_schema_latest() {
   return array_dir->load_array_schema_latest(enc_key_, memory_tracker_);
 }
 
-TEST_CASE_METHOD(
-    CurrentDomainFx<int32_t>,
-    "Setting CurrentDomain not allowed on Dense",
-    "[current_domain][dense]") {
-  auto schema = create_schema(true);
-  auto current_domain = create_current_domain({}, schema, nullptr, true);
-
-  auto matcher = Catch::Matchers::ContainsSubstring(
-      "Setting a current domain on a TileDB dense array is not supported.");
-
-  REQUIRE_THROWS_WITH(schema->set_current_domain(current_domain), matcher);
-
-  Range r;
-  std::vector<int32_t> rdata = {1, 1000};
-  r = Range(rdata.data(), 2 * sizeof(int32_t));
-  current_domain = create_current_domain({r, r}, schema, nullptr, false);
-  auto ase = make_shared<ArraySchemaEvolution>(HERE(), this->memory_tracker_);
-  ase->expand_current_domain(current_domain);
-
-  auto matcher2 = Catch::Matchers::ContainsSubstring(
-      "Expanding the current domain on a TileDB dense array is not supported.");
-
-  REQUIRE_THROWS_WITH(ase->evolve_schema(schema), matcher2);
-}
-
 TEST_CASE_METHOD(
     CurrentDomainFx<int32_t>,
     "Create Empty CurrentDomain",