Add test and fix problems

test/src/test-capi-sparse-array-dimension-label.cc

@@ -31,15 +31,16 @@
  * labels.
  */
 
+#include <test/support/tdb_catch.h>
 #include "test/support/src/helpers.h"
+#include "test/support/src/serialization_wrappers.h"
 #include "test/support/src/vfs_helpers.h"
 #include "tiledb/api/c_api/context/context_api_internal.h"
 #include "tiledb/sm/c_api/tiledb.h"
 #include "tiledb/sm/c_api/tiledb_experimental.h"
 #include "tiledb/sm/c_api/tiledb_struct_def.h"
 #include "tiledb/sm/enums/encryption_type.h"
 
-#include <test/support/tdb_catch.h>
 #include <iostream>
 #include <string>
 
@@ -249,6 +250,10 @@ class SparseArrayExample1 : public TemporaryDirectoryFixture {
           ctx, subarray, "x", &ranges[2 * r], &ranges[2 * r + 1], nullptr));
     }
 
+    if (serialize_) {
+      subarray = tiledb_subarray_serialize(ctx, array, subarray);
+    }
+
     // Define label buffer and size.
     std::vector<double> label_data(expected_label_data.size());
     uint64_t label_data_size{label_data.size() * sizeof(double)};

test/support/src/serialization_wrappers.cc

@@ -31,12 +31,17 @@
  * through serialization.
  */
 
-#include <test/support/tdb_catch.h>
-#include <string>
-
 #include "test/support/src/helpers.h"
 #include "tiledb/sm/c_api/tiledb.h"
 #include "tiledb/sm/c_api/tiledb_serialization.h"
+#include "tiledb/sm/c_api/tiledb_struct_def.h"
+#include "tiledb/sm/serialization/query.h"
+
+#include <capnp/message.h>
+#include <test/support/tdb_catch.h>
+#include <string>
+
+using namespace tiledb::sm;
 
 int tiledb_array_create_serialization_wrapper(
     tiledb_ctx_t* ctx,
@@ -179,4 +184,25 @@ int tiledb_fragment_info_serialize(
 
   tiledb_buffer_free(&buffer);
   return rc;
+}
+
+tiledb_subarray_t* tiledb_subarray_serialize(
+    tiledb_ctx_t* ctx, tiledb_array_t* array, tiledb_subarray_t* subarray) {
+  // Serialize
+  ::capnp::MallocMessageBuilder message;
+  tiledb::sm::serialization::capnp::Subarray::Builder builder =
+      message.initRoot<tiledb::sm::serialization::capnp::Subarray>();
+  tiledb_array_schema_t* array_schema = nullptr;
+  tiledb_array_get_schema(ctx, array, &array_schema);
+  REQUIRE(tiledb::sm::serialization::subarray_to_capnp(
+              *(array_schema->array_schema_), subarray->subarray_, &builder)
+              .ok());
+  // Deserialize
+  tiledb_subarray_t* deserialized_subarray;
+  tiledb::test::require_tiledb_ok(
+      ctx, tiledb_subarray_alloc(ctx, array, &deserialized_subarray));
+  REQUIRE(tiledb::sm::serialization::subarray_from_capnp(
+              builder, deserialized_subarray->subarray_)
+              .ok());
+  return deserialized_subarray;
 }

test/support/src/serialization_wrappers.h

@@ -128,4 +128,15 @@ int tiledb_fragment_info_serialize(
     tiledb_fragment_info_t* fragment_info_before_serialization,
     tiledb_fragment_info_t* fragment_info_deserialized,
     tiledb_serialization_type_t serialize_type);
+
+/**
+ * Return a subarray after serializing and deserializing it
+ *
+ * @param ctx tiledb context
+ * @param array the array the subarray belongs to
+ * @param subarray the subarray to serialize
+ * @return the deserialized subarray
+ */
+tiledb_subarray_t* tiledb_subarray_serialize(
+    tiledb_ctx_t* ctx, tiledb_array_t* array, tiledb_subarray_t* subarray);
 #endif  // TILEDB_TEST_SERIALIZATION_WRAPPERS_H

tiledb/sm/serialization/query.cc

@@ -72,7 +72,6 @@
 #include "tiledb/sm/serialization/fragment_metadata.h"
 #include "tiledb/sm/serialization/query.h"
 #include "tiledb/sm/storage_manager/storage_manager_declaration.h"
-#include "tiledb/sm/subarray/subarray.h"
 #include "tiledb/sm/subarray/subarray_partitioner.h"
 
 using namespace tiledb::common;
@@ -207,8 +206,11 @@ Status subarray_to_capnp(
     for (uint32_t i = 0; i < dim_num; i++) {
       if (subarray->has_label_ranges(i)) {
         auto label_range_builder = label_ranges_builder[label_id++];
-        // TODO: Consider throwing instead
-        assert(label_id < label_ranges_num);
+        if (label_id > label_ranges_num) {
+          throw StatusException(
+              Status_SerializationError("Label id exceeds the total number of "
+                                        "label ranges for the subarray."));
+        }
         label_range_builder.setDimensionId(i);
         const auto& label_name = subarray->get_label_name(i);
         label_range_builder.setName(label_name);
@@ -217,9 +219,6 @@ Status subarray_to_capnp(
         const auto datatype{schema.dimension_ptr(i)->type()};
         const auto& label_ranges = subarray->ranges_for_label(label_name);
         range_builder.setType(datatype_str(datatype));
-        // TODO: check with Julia if for dim lables is_implicitely_initialized_
-        // is always false. Should I even set this here?
-        // range_builder.setHasDefaultRange(false);
         range_buffers_to_capnp(label_ranges, range_builder);
       }
     }

tiledb/sm/serialization/query.h

@@ -39,6 +39,7 @@
 #include "tiledb/common/thread_pool.h"
 #include "tiledb/sm/query/query_condition.h"
 #include "tiledb/sm/storage_manager/storage_manager_declaration.h"
+#include "tiledb/sm/subarray/subarray.h"
 
 #ifdef TILEDB_SERIALIZATION
 #include "tiledb/sm/serialization/tiledb-rest.h"
@@ -232,6 +233,14 @@ Status condition_from_capnp(
 Status condition_to_capnp(
     const QueryCondition& condition,
     capnp::Condition::Builder* condition_builder);
+
+Status subarray_to_capnp(
+    const ArraySchema& schema,
+    const Subarray* subarray,
+    capnp::Subarray::Builder* builder);
+
+Status subarray_from_capnp(
+    const capnp::Subarray::Reader& reader, Subarray* subarray);
 #endif
 
 }  // namespace serialization

tiledb/sm/subarray/subarray.cc

@@ -183,7 +183,7 @@ void Subarray::add_label_range(
   const auto dim_idx = dim_label_ref.dimension_index();
   if (label_range_subset_[dim_idx].has_value()) {
     // A label range has already been set on this dimension. Do the following:
-    //  * Check this label is the same label that rangers were already set.
+    //  * Check this label is the same label that ranges were already set.
     if (dim_label_ref.name() != label_range_subset_[dim_idx].value().name_) {
       throw StatusException(Status_SubarrayError(
           "Cannot add label range; Dimension is already to set to use "
@@ -1034,7 +1034,7 @@ bool Subarray::has_label_ranges(const uint32_t dim_index) const {
          !label_range_subset_[dim_index]->ranges_.is_empty();
 }
 
-bool Subarray::label_ranges_num() const {
+int Subarray::label_ranges_num() const {
   return std::count_if(
       std::begin(label_range_subset_),
       std::end(label_range_subset_),
@@ -1829,21 +1829,17 @@ Status Subarray::set_ranges_for_dim(
   return Status::Ok();
 }
 
-// TODO: const in ranges was dropper because add_range expects non-const. See if
-// a const add_range can be added instead
 void Subarray::set_label_ranges_for_dim(
     const uint32_t dim_idx,
     const std::string& name,
-    std::vector<Range>& ranges) {
+    const std::vector<Range>& ranges) {
   auto dim{array_->array_schema_latest().dimension_ptr(dim_idx)};
   label_range_subset_[dim_idx] =
       LabelRangeSubset(name, dim->type(), coalesce_ranges_);
-  for (auto& range : ranges) {
-    // TODO: no idea if impl_ should be updated instead as in
-    // "add_range_unrestricted"
-    auto&& [error_status, oob_warning] =
-        label_range_subset_[dim_idx].value().ranges_.add_range(range);
-    throw_if_not_ok(error_status);
+  for (const auto& range : ranges) {
+    throw_if_not_ok(
+        label_range_subset_[dim_idx].value().ranges_.add_range_unrestricted(
+            range));
   }
 }
 

tiledb/sm/subarray/subarray.h

@@ -439,7 +439,9 @@ class Subarray {
   const std::vector<Range>& get_attribute_ranges(
       const std::string& attr_name) const;
 
-  // TODO: add docstring
+  /**
+   * Get all attribute ranges.
+   */
   inline const std::unordered_map<std::string, std::vector<Range>>&
   get_attribute_ranges() const {
     return attr_range_subset_;
@@ -949,7 +951,10 @@ class Subarray {
    */
   bool has_label_ranges(const uint32_t dim_index) const;
 
-  bool label_ranges_num() const;
+  /**
+   * Returns the number of dimensions that have label ranges set
+   */
+  int label_ranges_num() const;
 
   /**
    * Set default indicator for dimension subarray. Used by serialization only
@@ -1043,11 +1048,21 @@ class Subarray {
    */
   Status set_ranges_for_dim(uint32_t dim_idx, const std::vector<Range>& ranges);
 
-  // TODO: add docstring
+  /**
+   * Directly sets the dimension label ranges for the given dimension index,
+   * making a deep copy.
+   *
+   * @param dim_idx Index of dimension to set
+   * @param name Name of the dimension label to set
+   * @param ranges `Range` vector that will be copied and set
+   * @return Status
+   *
+   * @note Intended for serialization only
+   */
   void set_label_ranges_for_dim(
       const uint32_t dim_idx,
       const std::string& name,
-      std::vector<Range>& ranges);
+      const std::vector<Range>& ranges);
 
   /**
    * Splits the subarray along the splitting dimension and value into
@@ -1217,7 +1232,12 @@ class Subarray {
     return coalesce_ranges_;
   }
 
-  // TODO: docstring
+  /**
+   * Initialize the label ranges vector to nullopt for every
+   * dimension
+   *
+   * @param dim_num Total number of dimensions of the schema
+   */
   void add_default_label_ranges(dimension_size_type dim_num);
 
  private: