From 3d411d2cb864b0d1f048d5d3dea14eac36216880 Mon Sep 17 00:00:00 2001 From: Dimitris Staratzis <33267511+DimitrisStaratzis@users.noreply.github.com> Date: Thu, 12 Sep 2024 20:10:18 +0300 Subject: [PATCH 1/5] Enable current domain for Dense --- test/src/test-cppapi-current-domain.cc | 251 +++++++++++++++++- tiledb/sm/array_schema/array_schema.cc | 11 - .../array_schema/test/unit_current_domain.cc | 25 -- 3 files changed, 250 insertions(+), 37 deletions(-) diff --git a/test/src/test-cppapi-current-domain.cc b/test/src/test-cppapi-current-domain.cc index fad73f8acd2..b939088a056 100644 --- a/test/src/test-cppapi-current-domain.cc +++ b/test/src/test-cppapi-current-domain.cc @@ -365,7 +365,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 +488,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(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(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 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 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 a_with_cd(100); + std::vector 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 a_with_cd2(100); + std::vector 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(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(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 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(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(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 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/sm/array_schema/array_schema.cc b/tiledb/sm/array_schema/array_schema.cc index 1de4e2982f8..8413112b055 100644 --- 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 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 index 551b3181023..d7b09bbb294 100644 --- 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 CurrentDomainFx::get_array_schema_latest() { return array_dir->load_array_schema_latest(enc_key_, memory_tracker_); } -TEST_CASE_METHOD( - CurrentDomainFx, - "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 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(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, "Create Empty CurrentDomain", From 2ae653877a4fbf31078d105140c22fd2d4e2157f Mon Sep 17 00:00:00 2001 From: dimitrisstaratzis Date: Fri, 13 Sep 2024 01:36:14 +0300 Subject: [PATCH 2/5] fix documentation --- tiledb/api/c_api/array_schema/array_schema_api_experimental.h | 3 --- 1 file changed, 3 deletions(-) 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 index d0ca102c865..ab050882366 100644 --- 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} From ea6a9820eca843a4e2fc3f63818fbd11988481ad Mon Sep 17 00:00:00 2001 From: Dimitris Staratzis <33267511+DimitrisStaratzis@users.noreply.github.com> Date: Mon, 16 Sep 2024 13:46:20 +0300 Subject: [PATCH 3/5] add more tests --- test/src/test-cppapi-current-domain.cc | 23 +++++++++++++++++++++-- 1 file changed, 21 insertions(+), 2 deletions(-) diff --git a/test/src/test-cppapi-current-domain.cc b/test/src/test-cppapi-current-domain.cc index b939088a056..79555b1e459 100644 --- a/test/src/test-cppapi-current-domain.cc +++ b/test/src/test-cppapi-current-domain.cc @@ -153,13 +153,23 @@ TEST_CASE_METHOD( CurrentDomainFx, "C++ API: CurrentDomain - Add to ArraySchema", "[cppapi][ArraySchema][currentDomain]") { + + tiledb_array_type_t type = TILEDB_SPARSE; + SECTION("Dense") { + type = TILEDB_DENSE; + } + + SECTION("Sparse"){ + // do nothing + } + // Create domain. tiledb::Domain domain(ctx_); auto d = tiledb::Dimension::create(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(ctx_, "a")); @@ -191,6 +201,15 @@ TEST_CASE_METHOD( "[cppapi][ArraySchema][currentDomain]") { const std::string array_name = "test_current_domain_expansion"; + tiledb_array_type_t type = TILEDB_SPARSE; + SECTION("Dense") { + type = TILEDB_DENSE; + } + + SECTION("Sparse"){ + // do nothing + } + tiledb::VFS vfs(ctx_); if (vfs.is_dir(array_name)) { vfs.remove_dir(array_name); @@ -202,7 +221,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(ctx_, "a")); From b6e173f65b99bc451737879ded5596d71623de83 Mon Sep 17 00:00:00 2001 From: dimitrisstaratzis Date: Mon, 16 Sep 2024 13:51:39 +0300 Subject: [PATCH 4/5] format --- test/src/test-cppapi-current-domain.cc | 5 ++--- 1 file changed, 2 insertions(+), 3 deletions(-) diff --git a/test/src/test-cppapi-current-domain.cc b/test/src/test-cppapi-current-domain.cc index 79555b1e459..e441acc376d 100644 --- a/test/src/test-cppapi-current-domain.cc +++ b/test/src/test-cppapi-current-domain.cc @@ -153,13 +153,12 @@ TEST_CASE_METHOD( CurrentDomainFx, "C++ API: CurrentDomain - Add to ArraySchema", "[cppapi][ArraySchema][currentDomain]") { - tiledb_array_type_t type = TILEDB_SPARSE; SECTION("Dense") { type = TILEDB_DENSE; } - SECTION("Sparse"){ + SECTION("Sparse") { // do nothing } @@ -206,7 +205,7 @@ TEST_CASE_METHOD( type = TILEDB_DENSE; } - SECTION("Sparse"){ + SECTION("Sparse") { // do nothing } From 3141ccea21fb7c5c787ec259a5c39822ecca172e Mon Sep 17 00:00:00 2001 From: Theodore Tsirpanis Date: Thu, 3 Oct 2024 22:15:37 +0300 Subject: [PATCH 5/5] Use Catch2 generators for dense/sparse test cases. --- test/src/test-cppapi-current-domain.cc | 18 ++---------------- 1 file changed, 2 insertions(+), 16 deletions(-) diff --git a/test/src/test-cppapi-current-domain.cc b/test/src/test-cppapi-current-domain.cc index e441acc376d..38395f58ccc 100644 --- a/test/src/test-cppapi-current-domain.cc +++ b/test/src/test-cppapi-current-domain.cc @@ -153,14 +153,7 @@ TEST_CASE_METHOD( CurrentDomainFx, "C++ API: CurrentDomain - Add to ArraySchema", "[cppapi][ArraySchema][currentDomain]") { - tiledb_array_type_t type = TILEDB_SPARSE; - SECTION("Dense") { - type = TILEDB_DENSE; - } - - SECTION("Sparse") { - // do nothing - } + tiledb_array_type_t type = GENERATE(TILEDB_DENSE, TILEDB_SPARSE); // Create domain. tiledb::Domain domain(ctx_); @@ -200,14 +193,7 @@ TEST_CASE_METHOD( "[cppapi][ArraySchema][currentDomain]") { const std::string array_name = "test_current_domain_expansion"; - tiledb_array_type_t type = TILEDB_SPARSE; - SECTION("Dense") { - type = TILEDB_DENSE; - } - - SECTION("Sparse") { - // do nothing - } + tiledb_array_type_t type = GENERATE(TILEDB_DENSE, TILEDB_SPARSE); tiledb::VFS vfs(ctx_); if (vfs.is_dir(array_name)) {