Update get_enumerations_all_schemas to store enumerations in the latest schema. (#5360)

This stores the loaded enumerations in the latest array schema after
initializing all_array_schemas via `Array::get_enumerations_all_schemas`
and adds two unit tests to cover some edge cases I encountered while
improving testing.

I also added a check to `get_enumerations_all_schemas` to potentially
save a request if all enumerations are already loaded, so it could be
called after array open with `rest_load_enumerations_on_array_open=true`
and we will not spend extra time reloading enumerations.

---
TYPE: NO_HISTORY
DESC: Store enumerations loaded by `Array::get_enumerations_all_schemas`
in the latest array schema.

Author: shaunrd0

test/src/unit-cppapi-enumerations.cc

@@ -1118,103 +1118,32 @@ TEST_CASE_METHOD(
   REQUIRE(schema->is_enumeration_loaded("test_enmr") == true);
 }
 
-TEST_CASE_METHOD(
-    EnumerationFx,
-    "Array - Load All Enumerations - All Schemas",
-    "[enumeration][array][load-all-enumerations][all-schemas]") {
-  create_array();
-  auto array = get_array(QueryType::READ);
-  auto schema = array->array_schema_latest_ptr();
-  REQUIRE(schema->is_enumeration_loaded("test_enmr") == false);
-  std::string schema_name_1 = schema->name();
-
-  // If not using array open v3 just test that the correct exception is thrown
-  if (!array->use_refactored_array_open()) {
-    CHECK_THROWS_WITH(
-        array->load_all_enumerations(true),
-        Catch::Matchers::ContainsSubstring(
-            "The array must be opened using "
-            "`rest.use_refactored_array_open=true`"));
-    return;
-  }
-
-  // Evolve once to add an enumeration.
-  auto ase = make_shared<ArraySchemaEvolution>(HERE(), memory_tracker_);
-  std::vector<std::string> var_values{"one", "two", "three"};
-  auto var_enmr = create_enumeration(
-      var_values, false, Datatype::STRING_ASCII, "ase_var_enmr");
-  ase->add_enumeration(var_enmr);
-  auto attr4 = make_shared<Attribute>(HERE(), "attr4", Datatype::UINT16);
-  attr4->set_enumeration_name("ase_var_enmr");
-  CHECK_NOTHROW(ase->evolve_schema(schema));
-  // Apply evolution to the array and reopen.
-  CHECK_NOTHROW(Array::evolve_array_schema(
-      ctx_.resources(), uri_, ase.get(), array->get_encryption_key()));
-  CHECK(array->reopen().ok());
-  CHECK_NOTHROW(array->load_all_enumerations(true));
-  auto all_schemas = array->array_schemas_all();
-  schema = array->array_schema_latest_ptr();
-  std::string schema_name_2 = schema->name();
-
-  // Check all schemas.
-  CHECK(all_schemas[schema_name_1]->is_enumeration_loaded("test_enmr") == true);
-  CHECK(all_schemas[schema_name_2]->is_enumeration_loaded("test_enmr") == true);
-  CHECK(
-      all_schemas[schema_name_2]->is_enumeration_loaded("ase_var_enmr") ==
-      true);
-
-  // Evolve a second time to drop an enumeration.
-  ase = make_shared<ArraySchemaEvolution>(HERE(), memory_tracker_);
-  ase->drop_enumeration("test_enmr");
-  ase->drop_attribute("attr1");
-  CHECK_NOTHROW(ase->evolve_schema(schema));
-  // Apply evolution to the array and reopen.
-  CHECK_NOTHROW(Array::evolve_array_schema(
-      ctx_.resources(), uri_, ase.get(), array->get_encryption_key()));
-  CHECK(array->reopen().ok());
-  CHECK_NOTHROW(array->load_all_enumerations(true));
-  all_schemas = array->array_schemas_all();
-  schema = array->array_schema_latest_ptr();
-  std::string schema_name_3 = schema->name();
-
-  // Check all schemas.
-  CHECK(all_schemas[schema_name_1]->is_enumeration_loaded("test_enmr") == true);
-  CHECK(all_schemas[schema_name_2]->is_enumeration_loaded("test_enmr") == true);
-  CHECK(
-      all_schemas[schema_name_2]->is_enumeration_loaded("ase_var_enmr") ==
-      true);
-  CHECK_THROWS_WITH(
-      all_schemas[schema_name_3]->is_enumeration_loaded("test_enmr"),
-      Catch::Matchers::ContainsSubstring("No enumeration named"));
-  CHECK(
-      all_schemas[schema_name_3]->is_enumeration_loaded("ase_var_enmr") ==
-      true);
-}
-
 TEST_CASE_METHOD(
     EnumerationFx,
     "Array - Load All Enumerations - Repeated",
     "[enumeration][array][load-all-enumerations][repeated]") {
   create_array();
+  bool all_schemas = GENERATE(true, false);
   auto array = get_array(QueryType::READ);
   auto schema = array->array_schema_latest_ptr();
 
   REQUIRE(schema->is_enumeration_loaded("test_enmr") == false);
 
-  CHECK_NOTHROW(array->load_all_enumerations());
+  CHECK_NOTHROW(array->load_all_enumerations(all_schemas));
   REQUIRE(schema->is_enumeration_loaded("test_enmr") == true);
 
-  CHECK_NOTHROW(array->load_all_enumerations());
+  CHECK_NOTHROW(array->load_all_enumerations(all_schemas));
   REQUIRE(schema->is_enumeration_loaded("test_enmr") == true);
 }
 
 TEST_CASE_METHOD(
     EnumerationFx,
     "Array - Load All Enumerations Error - Not Open",
     "[enumeration][array][error][not-open]") {
+  bool all_schemas = GENERATE(true, false);
   auto array = make_shared<Array>(HERE(), ctx_.resources(), uri_);
   auto matcher = Catch::Matchers::ContainsSubstring("Array is not open");
-  REQUIRE_THROWS(array->load_all_enumerations());
+  REQUIRE_THROWS(array->load_all_enumerations(all_schemas));
 }
 
 /* ********************************* */
@@ -1704,8 +1633,9 @@ TEST_CASE_METHOD(
     "ArraySchemaEvolution - Simple No Enumeration",
     "[enumeration][array-schema-evolution][simple]") {
   create_array();
+  bool all_schemas = GENERATE(true, false);
   auto array = get_array(QueryType::READ);
-  array->load_all_enumerations();
+  array->load_all_enumerations(all_schemas);
 
   auto orig_schema = array->array_schema_latest_ptr();
   auto ase = make_shared<ArraySchemaEvolution>(HERE(), memory_tracker_);
@@ -2558,7 +2488,7 @@ TEST_CASE_METHOD(
   REQUIRE(a1->serialize_enumerations() == (do_load == "true"));
   REQUIRE(
       a1->array_schema_latest_ptr()->get_loaded_enumeration_names().size() ==
-      0);
+      (do_load == "true" ? 2 : 0));
 
   auto a2 = make_shared<Array>(HERE(), ctx.resources(), uri_);
 

test/src/unit-enumerations.cc

@@ -30,10 +30,14 @@
  * Tests end to end enumerations.
  */
 
+#include "test/support/src/array_schema_helpers.h"
 #include "test/support/src/vfs_helpers.h"
 #include "test/support/tdb_catch.h"
+#include "tiledb/api/c_api/array/array_api_internal.h"
+#include "tiledb/api/c_api/array_schema/array_schema_api_internal.h"
 #include "tiledb/api/c_api/enumeration/enumeration_api_internal.h"
 #include "tiledb/sm/array_schema/array_schema.h"
+#include "tiledb/sm/array_schema/array_schema_evolution.h"
 #include "tiledb/sm/c_api/tiledb_struct_def.h"
 #include "tiledb/sm/cpp_api/tiledb"
 #include "tiledb/sm/cpp_api/tiledb_experimental"
@@ -46,6 +50,7 @@ struct RESTEnumerationFx {
   void create_array(const std::string& array_name);
 
   tiledb::test::VFSTestSetup vfs_test_setup_;
+  shared_ptr<sm::MemoryTracker> memory_tracker_;
   std::string uri_;
   Context ctx_;
 };
@@ -145,8 +150,233 @@ TEST_CASE_METHOD(
   new_array.close();
 }
 
+TEST_CASE_METHOD(
+    RESTEnumerationFx,
+    "Load Enumerations - All Schemas",
+    "[enumeration][array][load-all-enumerations][all-schemas][rest]") {
+  uri_ = vfs_test_setup_.array_uri("load_enmrs_all_schemas");
+  auto config = vfs_test_setup_.ctx().config();
+  bool load_enmrs = GENERATE(true, false);
+  config["rest.load_enumerations_on_array_open"] =
+      load_enmrs ? "true" : "false";
+  vfs_test_setup_.update_config(config.ptr().get());
+  ctx_ = vfs_test_setup_.ctx();
+
+  create_array(uri_);
+  Array opened_array(ctx_, uri_, TILEDB_READ);
+  auto array = opened_array.ptr()->array();
+  auto schema = array->array_schema_latest_ptr();
+  REQUIRE(schema->is_enumeration_loaded("my_enum") == load_enmrs);
+  REQUIRE(schema->is_enumeration_loaded("fruit") == load_enmrs);
+
+  // If not using array open v3 just test that the correct exception is thrown
+  if (!array->use_refactored_array_open()) {
+    CHECK_THROWS_WITH(
+        array->load_all_enumerations(true),
+        Catch::Matchers::ContainsSubstring(
+            "The array must be opened using "
+            "`rest.use_refactored_array_open=true`"));
+    return;
+  }
+  // If enumerations were loaded on array open this will not submit an
+  // additional request.
+  auto actual_enmrs = array->get_enumerations_all_schemas();
+  if (!load_enmrs) {
+    REQUIRE(schema->is_enumeration_loaded("my_enum") == true);
+    REQUIRE(schema->is_enumeration_loaded("fruit") == true);
+  }
+
+  // Fetch enumerations created in the initial array schema for validation.
+  auto enmr1 = array->get_enumeration("my_enum");
+  auto enmr2 = array->get_enumeration("fruit");
+  decltype(actual_enmrs) expected_enmrs{{schema->name(), {enmr1, enmr2}}};
+  auto validate_enmrs = [&]() {
+    for (const auto& [schema_name, enmrs] : expected_enmrs) {
+      REQUIRE(actual_enmrs.contains(schema_name));
+      REQUIRE(enmrs.size() == actual_enmrs[schema_name].size());
+      CHECK_THAT(
+          enmrs,
+          Catch::Matchers::UnorderedRangeEquals(
+              actual_enmrs[schema_name], [](const auto& a, const auto& b) {
+                return tiledb::test::is_equivalent_enumeration(*a, *b);
+              }));
+    }
+  };
+  validate_enmrs();
+
+  // Evolve once to add an enumeration.
+  sm::URI uri(uri_);
+  auto ase = make_shared<sm::ArraySchemaEvolution>(HERE(), memory_tracker_);
+  std::vector<std::string> var_values{"one", "two", "three"};
+  auto var_enmr = Enumeration::create(ctx_, "ase_var_enmr", var_values);
+  ase->add_enumeration(var_enmr.ptr()->enumeration());
+  auto attr4 =
+      make_shared<sm::Attribute>(HERE(), "attr4", sm::Datatype::UINT16);
+  attr4->set_enumeration_name("ase_var_enmr");
+  CHECK_NOTHROW(ase->evolve_schema(schema));
+  // Apply evolution to the array and reopen.
+  CHECK_NOTHROW(sm::Array::evolve_array_schema(
+      ctx_.ptr()->resources(), uri, ase.get(), array->get_encryption_key()));
+  CHECK(array->reopen().ok());
+  schema = array->array_schema_latest_ptr();
+  std::string schema_name_2 = schema->name();
+  REQUIRE(schema->is_enumeration_loaded("my_enum") == load_enmrs);
+  REQUIRE(schema->is_enumeration_loaded("fruit") == load_enmrs);
+  REQUIRE(schema->is_enumeration_loaded("ase_var_enmr") == load_enmrs);
+
+  expected_enmrs[schema_name_2] = {enmr1, enmr2, var_enmr.ptr()->enumeration()};
+  actual_enmrs = array->get_enumerations_all_schemas();
+  if (!load_enmrs) {
+    REQUIRE(schema->is_enumeration_loaded("my_enum") == true);
+    REQUIRE(schema->is_enumeration_loaded("fruit") == true);
+    REQUIRE(schema->is_enumeration_loaded("ase_var_enmr") == true);
+  }
+  validate_enmrs();
+
+  // Evolve a second time to drop an enumeration.
+  ase = make_shared<sm::ArraySchemaEvolution>(HERE(), memory_tracker_);
+  ase->drop_enumeration("my_enum");
+  ase->drop_attribute("attr1");
+  CHECK_NOTHROW(ase->evolve_schema(schema));
+  // Apply evolution to the array and reopen.
+  CHECK_NOTHROW(sm::Array::evolve_array_schema(
+      ctx_.ptr()->resources(), uri, ase.get(), array->get_encryption_key()));
+  CHECK(array->reopen().ok());
+  schema = array->array_schema_latest_ptr();
+  std::string schema_name_3 = schema->name();
+  REQUIRE_THROWS_WITH(
+      schema->is_enumeration_loaded("my_enum"),
+      Catch::Matchers::ContainsSubstring("unknown enumeration"));
+  REQUIRE(schema->is_enumeration_loaded("fruit") == load_enmrs);
+  REQUIRE(schema->is_enumeration_loaded("ase_var_enmr") == load_enmrs);
+
+  expected_enmrs[schema_name_3] = {enmr2, var_enmr.ptr()->enumeration()};
+  actual_enmrs = array->get_enumerations_all_schemas();
+  if (!load_enmrs) {
+    REQUIRE_THROWS_WITH(
+        schema->is_enumeration_loaded("my_enum"),
+        Catch::Matchers::ContainsSubstring("unknown enumeration"));
+    REQUIRE(schema->is_enumeration_loaded("fruit") == true);
+    REQUIRE(schema->is_enumeration_loaded("ase_var_enmr") == true);
+  }
+
+  validate_enmrs();
+}
+
+TEST_CASE_METHOD(
+    RESTEnumerationFx,
+    "Load Enumerations - All Schemas partial load",
+    "[enumeration][array][load-all-enumerations][all-schemas][rest]") {
+  uri_ = vfs_test_setup_.array_uri("load_enmrs_all_schemas");
+
+  create_array(uri_);
+  Array opened_array(ctx_, uri_, TILEDB_READ);
+  auto array = opened_array.ptr()->array();
+  auto schema = array->array_schema_latest_ptr();
+  REQUIRE(schema->is_enumeration_loaded("my_enum") == false);
+  REQUIRE(schema->is_enumeration_loaded("fruit") == false);
+  // Fetch one enumeration, intentionally leaving the other unloaded.
+  auto enmr1 = array->get_enumeration("my_enum");
+  REQUIRE(schema->is_enumeration_loaded("my_enum") == true);
+
+  // If not using array open v3 just test that the correct exception is thrown
+  if (!array->use_refactored_array_open()) {
+    CHECK_THROWS_WITH(
+        array->load_all_enumerations(true),
+        Catch::Matchers::ContainsSubstring(
+            "The array must be opened using "
+            "`rest.use_refactored_array_open=true`"));
+    return;
+  }
+
+  // Load all enumerations.
+  auto actual_enmrs = array->get_enumerations_all_schemas();
+  REQUIRE(schema->is_enumeration_loaded("fruit") == true);
+  auto enmr2 = array->get_enumeration("fruit");
+
+  decltype(actual_enmrs) expected_enmrs{{schema->name(), {enmr1, enmr2}}};
+  auto validate_enmrs = [&]() {
+    for (const auto& [schema_name, enmrs] : expected_enmrs) {
+      REQUIRE(actual_enmrs.contains(schema_name));
+      REQUIRE(enmrs.size() == actual_enmrs[schema_name].size());
+      CHECK_THAT(
+          enmrs,
+          Catch::Matchers::UnorderedRangeEquals(
+              actual_enmrs[schema_name], [](const auto& a, const auto& b) {
+                return tiledb::test::is_equivalent_enumeration(*a, *b);
+              }));
+    }
+  };
+  validate_enmrs();
+
+  // Evolve once to add an enumeration.
+  sm::URI uri(uri_);
+  auto ase = make_shared<sm::ArraySchemaEvolution>(HERE(), memory_tracker_);
+  std::vector<std::string> var_values{"one", "two", "three"};
+  auto var_enmr = Enumeration::create(ctx_, "ase_var_enmr", var_values);
+  ase->add_enumeration(var_enmr.ptr()->enumeration());
+  auto attr4 =
+      make_shared<sm::Attribute>(HERE(), "attr4", sm::Datatype::UINT16);
+  attr4->set_enumeration_name("ase_var_enmr");
+  CHECK_NOTHROW(ase->evolve_schema(schema));
+  // Apply evolution to the array and reopen.
+  CHECK_NOTHROW(sm::Array::evolve_array_schema(
+      ctx_.ptr()->resources(), uri, ase.get(), array->get_encryption_key()));
+  CHECK(array->reopen().ok());
+  schema = array->array_schema_latest_ptr();
+  std::string schema_name_2 = schema->name();
+  REQUIRE(schema->is_enumeration_loaded("my_enum") == false);
+  REQUIRE(schema->is_enumeration_loaded("fruit") == false);
+  REQUIRE(schema->is_enumeration_loaded("ase_var_enmr") == false);
+
+  SECTION("Partial load a single evolved enumeration") {
+    // Load all enumerations except the enumeration we added with evolution
+    // above.
+    array->get_enumeration("my_enum");
+    REQUIRE(schema->is_enumeration_loaded("my_enum") == true);
+    array->get_enumeration("fruit");
+    REQUIRE(schema->is_enumeration_loaded("fruit") == true);
+    // Load the remaining `ase_var_enmr` enumeration.
+    actual_enmrs = array->get_enumerations_all_schemas();
+    expected_enmrs[schema_name_2] = {
+        enmr1, enmr2, var_enmr.ptr()->enumeration()};
+    validate_enmrs();
+  }
+
+  SECTION("Partial load multiple enumerations") {
+    // Load all enumerations except the enumeration we added with evolution
+    // above.
+    array->get_enumeration("fruit");
+    REQUIRE(schema->is_enumeration_loaded("fruit") == true);
+    // Load the remaining `my_enum` and `ase_var_enmr` enumerations.
+  }
+
+  actual_enmrs = array->get_enumerations_all_schemas();
+  expected_enmrs[schema_name_2] = {enmr1, enmr2, var_enmr.ptr()->enumeration()};
+  validate_enmrs();
+
+  SECTION("Drop all enumerations and validate earlier schemas") {
+    ase = make_shared<sm::ArraySchemaEvolution>(HERE(), memory_tracker_);
+    ase->drop_enumeration("my_enum");
+    ase->drop_attribute("attr1");
+    ase->drop_enumeration("fruit");
+    ase->drop_attribute("attr3");
+    ase->drop_enumeration("ase_var_enmr");
+    CHECK_NOTHROW(ase->evolve_schema(schema));
+    CHECK_NOTHROW(sm::Array::evolve_array_schema(
+        ctx_.ptr()->resources(), uri, ase.get(), array->get_encryption_key()));
+    CHECK(array->reopen().ok());
+    schema = array->array_schema_latest_ptr();
+    std::string schema_name_3 = schema->name();
+    actual_enmrs = array->get_enumerations_all_schemas();
+    expected_enmrs[schema_name_3] = {};
+    validate_enmrs();
+  }
+}
+
 RESTEnumerationFx::RESTEnumerationFx()
-    : ctx_(vfs_test_setup_.ctx()){};
+    : memory_tracker_(tiledb::test::create_test_memory_tracker())
+    , ctx_(vfs_test_setup_.ctx()){};
 
 void RESTEnumerationFx::create_array(const std::string& array_name) {
   // Create a simple array for testing. This ends up with just five elements in