Fix serialization of empty enumerations (#4472)

This was an obvious oversight in the original PR. I managed to cover enumeration extension serialization but forgot to include serialization of empty arrays. --- TYPE: BUG DESC: Fix serialization of empty enumerations
TileDB-Inc · Nov 1, 2023 · 9163edf · 9163edf
1 parent af3cbf8
commit 9163edf
Show file tree

Hide file tree

Showing 3 changed files with 46 additions and 12 deletions.
diff --git a/test/src/unit-enumerations.cc b/test/src/unit-enumerations.cc
@@ -2119,6 +2119,39 @@ TEST_CASE_METHOD(
   REQUIRE(loaded_names2.empty());
 }
 
+TEST_CASE_METHOD(
+    EnumerationFx,
+    "Cap'N Proto - ArraySchema Serialization with Empty Enumerations",
+    "[enumeration][capnp][array-schema][empty-enumerations]") {
+  create_array();
+
+  auto client_side = GENERATE(true, false);
+  auto ser_type = GENERATE(SerializationType::CAPNP, SerializationType::JSON);
+
+  auto schema1 = create_schema();
+
+  auto enmr1 = Enumeration::create(
+      "empty_fixed", Datatype::INT32, 1, false, nullptr, 0, nullptr, 0);
+  auto enmr2 = Enumeration::create(
+      "empty_var",
+      Datatype::STRING_ASCII,
+      constants::var_num,
+      false,
+      nullptr,
+      0,
+      nullptr,
+      0);
+
+  schema1->add_enumeration(enmr1);
+  schema1->add_enumeration(enmr2);
+
+  auto schema2 = ser_des_array_schema(schema1, client_side, ser_type);
+
+  auto all_names1 = schema1->get_enumeration_names();
+  auto all_names2 = schema2.get_enumeration_names();
+  REQUIRE(vec_cmp(all_names1, all_names2));
+}
+
 TEST_CASE_METHOD(
     EnumerationFx,
     "Cap'N Proto - Basic ArraySchemaEvolution Serialization",

diff --git a/tiledb/sm/c_api/tiledb.cc b/tiledb/sm/c_api/tiledb.cc
@@ -3595,7 +3595,7 @@ int32_t tiledb_deserialize_array_schema(
   } catch (...) {
     delete *array_schema;
     *array_schema = nullptr;
-    return TILEDB_ERR;
+    throw;
   }
 
   return TILEDB_OK;

diff --git a/tiledb/sm/serialization/enumeration.cc b/tiledb/sm/serialization/enumeration.cc
@@ -58,10 +58,12 @@ void enumeration_to_capnp(
   enmr_builder.setOrdered(enumeration->ordered());
 
   auto dspan = enumeration->data();
-  enmr_builder.setData(::kj::arrayPtr(dspan.data(), dspan.size()));
+  if (dspan.size() > 0) {
+    enmr_builder.setData(::kj::arrayPtr(dspan.data(), dspan.size()));
+  }
 
-  if (enumeration->var_size()) {
-    auto ospan = enumeration->offsets();
+  auto ospan = enumeration->offsets();
+  if (ospan.size() > 0) {
     enmr_builder.setOffsets(::kj::arrayPtr(ospan.data(), ospan.size()));
   }
 }
@@ -73,15 +75,14 @@ shared_ptr<const Enumeration> enumeration_from_capnp(
   Datatype datatype = Datatype::ANY;
   throw_if_not_ok(datatype_enum(reader.getType(), &datatype));
 
-  if (!reader.hasData()) {
-    throw SerializationStatusException(
-        "[Deserialization::enumeration_from_capnp] Deserialization of "
-        "Enumeration is missing its data buffer.");
-  }
+  const void* data = nullptr;
+  uint64_t data_size = 0;
 
-  auto data_reader = reader.getData().asBytes();
-  auto data = data_reader.begin();
-  auto data_size = data_reader.size();
+  if (reader.hasData()) {
+    auto data_reader = reader.getData().asBytes();
+    data = data_reader.begin();
+    data_size = data_reader.size();
+  }
 
   const void* offsets = nullptr;
   uint64_t offsets_size = 0;