Current domain: disallow reading outside of current domain. (#5168)

This change makes sure a user cannot read data outside of the current
domain. This can happen in array where the current domain was not
initially set and later added through schema evolution.

---
TYPE: FEATURE
DESC: Current domain: disallow reading outside of current domain.

Author: KiterLuc

test/src/test-cppapi-current-domain.cc

@@ -354,3 +354,130 @@ TEST_CASE_METHOD(
     vfs.remove_dir(array_name);
   }
 }
+
+TEST_CASE_METHOD(
+    CurrentDomainFx,
+    "C++ API: CurrentDomain - Read cells written outside of shape",
+    "[cppapi][ArraySchema][currentDomain][luc]") {
+  const std::string array_name = "test_current_domain_read";
+
+  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", {{0, 100}}, 10);
+  auto d2 = tiledb::Dimension::create(
+      ctx_, "dim2", TILEDB_STRING_ASCII, nullptr, nullptr);
+  domain.add_dimension(d1);
+  domain.add_dimension(d2);
+
+  // Create array schema.
+  tiledb::ArraySchema schema(ctx_, TILEDB_SPARSE);
+  schema.set_domain(domain);
+  schema.add_attribute(tiledb::Attribute::create<int>(ctx_, "a"));
+
+  // Create array
+  tiledb::Array::create(array_name, schema);
+
+  tiledb::Array array(ctx_, array_name, TILEDB_WRITE);
+
+  // Some of the data here is outside of the current domain we will set later.
+  std::vector<int32_t> dim1 = {12, 14, 16, 18};
+  std::vector<char> dim2 = {'b', 'a', 'b', 'c'};
+  std::vector<uint64_t> dim2_offsets = {0, 1, 2, 3};
+  std::vector<int> a1 = {1, 2, 3, 4};
+
+  // All data in current domain.
+  tiledb::Query query(ctx_, array, TILEDB_WRITE);
+  query.set_layout(TILEDB_UNORDERED)
+      .set_data_buffer("a", a1)
+      .set_data_buffer("dim1", dim1)
+      .set_offsets_buffer("dim2", dim2_offsets)
+      .set_data_buffer("dim2", dim2);
+  query.submit();
+  array.close();
+
+  // Create new currentDomain to allow all data
+  tiledb::CurrentDomain current_domain(ctx_);
+  int range[] = {10, 16};
+  tiledb::NDRectangle ndrect(ctx_, domain);
+  ndrect.set_range(0, range[0], range[1]);
+  ndrect.set_range(1, std::string("b"), std::string("c"));
+  current_domain.set_ndrectangle(ndrect);
+
+  // Schema evolution
+  tiledb::ArraySchemaEvolution se(ctx_);
+  se.expand_current_domain(current_domain);
+  se.array_evolve(array_name);
+
+  std::vector<int32_t> dim1_read(100);
+  std::vector<char> dim2_read(100);
+  std::vector<uint64_t> dim2_offsets_read(100);
+  std::vector<int> a1_read(100);
+
+  // Now try to read data.
+  tiledb::Array array_read(ctx_, array_name, TILEDB_READ);
+  tiledb::Query query_read(ctx_, array_read, TILEDB_READ);
+  query_read.set_layout(TILEDB_UNORDERED)
+      .set_data_buffer("a", a1_read)
+      .set_data_buffer("dim1", dim1_read)
+      .set_offsets_buffer("dim2", dim2_offsets_read)
+      .set_data_buffer("dim2", dim2_read);
+  query_read.submit();
+
+  // Validate we got two results.
+  auto res = query_read.result_buffer_elements();
+  CHECK(res["a"].second == 2);
+  CHECK(res["dim1"].second == 2);
+  CHECK(res["dim2"].first == 2);
+  CHECK(res["dim2"].second == 2);
+  CHECK(a1_read[0] == 1);
+  CHECK(a1_read[1] == 3);
+  CHECK(dim1_read[0] == 12);
+  CHECK(dim1_read[1] == 16);
+  CHECK(dim2_offsets_read[0] == 0);
+  CHECK(dim2_offsets_read[1] == 1);
+  CHECK(dim2_read[0] == 'b');
+  CHECK(dim2_read[1] == 'b');
+
+  // Now try to read data with ranges outside of current domain
+  tiledb::Query query_read2(ctx_, array_read, TILEDB_READ);
+  tiledb::Subarray subarray_read2(ctx_, array_read);
+
+  subarray_read2.add_range<int32_t>(0, 12, 16).add_range(0, 17, 20);
+  query_read2.set_layout(TILEDB_UNORDERED)
+      .set_data_buffer("a", a1_read)
+      .set_data_buffer("dim1", dim1_read)
+      .set_offsets_buffer("dim2", dim2_offsets_read)
+      .set_data_buffer("dim2", dim2_read)
+      .set_subarray(subarray_read2);
+  CHECK_THROWS_WITH(
+      query_read2.submit(),
+      Catch::Matchers::ContainsSubstring(
+          "A range was set outside of the current domain."));
+
+  tiledb::Query query_read3(ctx_, array_read, TILEDB_READ);
+  tiledb::Subarray subarray_read3(ctx_, array_read);
+
+  subarray_read3.add_range(1, std::string("a"), std::string("b"));
+  query_read3.set_layout(TILEDB_UNORDERED)
+      .set_data_buffer("a", a1_read)
+      .set_data_buffer("dim1", dim1_read)
+      .set_offsets_buffer("dim2", dim2_offsets_read)
+      .set_data_buffer("dim2", dim2_read)
+      .set_subarray(subarray_read3);
+  CHECK_THROWS_WITH(
+      query_read3.submit(),
+      Catch::Matchers::ContainsSubstring(
+          "A range was set outside of the current domain."));
+
+  array_read.close();
+
+  // Clean up.
+  if (vfs.is_dir(array_name)) {
+    vfs.remove_dir(array_name);
+  }
+}

tiledb/sm/array_schema/current_domain.cc

@@ -202,19 +202,29 @@ bool CurrentDomain::covered(const NDRange& ndranges) const {
 }
 
 bool CurrentDomain::includes(const NDRange& non_empty_domain) const {
+  for (dimension_size_type d = 0; d < non_empty_domain.size(); ++d) {
+    if (!includes(d, non_empty_domain[d])) {
+      return false;
+    }
+  }
+
+  return true;
+}
+
+bool CurrentDomain::includes(
+    const dimension_size_type d, const Range& range) const {
   switch (type_) {
     case CurrentDomainType::NDRECTANGLE: {
-      for (unsigned d = 0; d < non_empty_domain.size(); ++d) {
-        auto dim = ndrectangle_->domain()->dimension_ptr(d);
-        if (!dim->covered(non_empty_domain[d], ndrectangle_->get_range(d))) {
-          return false;
-        }
+      auto dim = ndrectangle_->domain()->dimension_ptr(d);
+      if (!dim->covered(range, ndrectangle_->get_range(d))) {
+        return false;
       }
       return true;
     }
     default: {
       throw std::runtime_error(
-          "Unable to execute this current domain operation because one of the "
+          "Unable to execute this current domain operation because one of "
+          "the "
           "current domains passed has an unsupported type " +
           current_domain_type_str(type_));
     }

tiledb/sm/array_schema/current_domain.h

@@ -50,6 +50,15 @@ class Domain;
 /** Defines an array current domain */
 class CurrentDomain {
  public:
+  /**
+   * Size type for the number of dimensions of an array and for dimension
+   * indices.
+   *
+   * Note: This should be the same as `Domain::dimension_size_type`. We're
+   * not including `domain.h`, otherwise we'd use that definition here.
+   */
+  using dimension_size_type = unsigned int;
+
   /* ********************************* */
   /*     CONSTRUCTORS & DESTRUCTORS    */
   /* ********************************* */
@@ -187,6 +196,16 @@ class CurrentDomain {
    */
   bool includes(const NDRange& non_empty_domain) const;
 
+  /**
+   * Checks if this current domain fully contains the range for a specific
+   * dimension.
+   *
+   * @param d Dimension index to check for.
+   * @param range Range to validate.
+   * @return True if the range is included for the specific dimension
+   */
+  bool includes(const dimension_size_type d, const Range& range) const;
+
   /**
    * Perform various checks to ensure the current domain is coherent with the
    * array schema

tiledb/sm/query/query.cc

@@ -37,6 +37,7 @@
 #include "tiledb/common/memory.h"
 #include "tiledb/common/memory_tracker.h"
 #include "tiledb/sm/array/array.h"
+#include "tiledb/sm/array_schema/current_domain.h"
 #include "tiledb/sm/array_schema/dimension_label.h"
 #include "tiledb/sm/enums/query_status.h"
 #include "tiledb/sm/enums/query_type.h"
@@ -835,6 +836,37 @@ Status Query::process() {
     condition_->rewrite_enumeration_conditions(array_schema());
   }
 
+  if (type_ == QueryType::READ) {
+    auto cd = array_schema_->get_current_domain();
+    if (!cd->empty()) {
+      // See if any data was written outside of the current domain.
+      bool all_ned_contained_in_current_domain = true;
+      for (auto& meta : fragment_metadata_) {
+        if (!cd->includes(meta->non_empty_domain())) {
+          all_ned_contained_in_current_domain = false;
+        }
+      }
+
+      for (Domain::dimension_size_type d = 0; d < array_schema_->dim_num();
+           d++) {
+        if (subarray_.is_set(d)) {
+          // Make sure all ranges are contained in the current domain.
+          for (auto& range : subarray_.ranges_for_dim(d)) {
+            if (!cd->includes(d, range)) {
+              throw QueryException(
+                  "A range was set outside of the current domain.");
+            }
+          }
+        } else if (!all_ned_contained_in_current_domain) {
+          // Add ranges to make sure all data read is contained in the current
+          // domain.
+          auto range_copy = cd->ndrectangle()->get_range(d);
+          subarray_.add_range(d, std::move(range_copy));
+        }
+      }
+    }
+  }
+
   // Update query status.
   status_ = QueryStatus::INPROGRESS;