Update to use new config parameter.

TileDB-Inc · Nov 18, 2024 · 0d72089 · 0d72089
1 parent 11e35dc
commit 0d72089
Show file tree

Hide file tree

Showing 11 changed files with 55 additions and 23 deletions.
diff --git a/test/src/unit-capi-config.cc b/test/src/unit-capi-config.cc
@@ -233,6 +233,7 @@ void check_save_to_file() {
   ss << "rest.curl.verbose false\n";
   ss << "rest.http_compressor any\n";
   ss << "rest.load_enumerations_on_array_open false\n";
+  ss << "rest.load_enumerations_on_array_open_all_schemas false\n";
   ss << "rest.load_metadata_on_array_open true\n";
   ss << "rest.load_non_empty_domain_on_array_open true\n";
   ss << "rest.retry_count 25\n";
@@ -617,6 +618,8 @@ TEST_CASE("C API: Test config iter", "[capi][config]") {
   all_param_values["rest.load_metadata_on_array_open"] = "false";
   all_param_values["rest.load_non_empty_domain_on_array_open"] = "false";
   all_param_values["rest.load_enumerations_on_array_open"] = "false";
+  all_param_values["rest.load_enumerations_on_array_open_all_schemas"] =
+      "false";
   all_param_values["rest.use_refactored_array_open"] = "false";
   all_param_values["rest.use_refactored_array_open_and_query_submit"] = "false";
   all_param_values["rest.payer_namespace"] = "";

diff --git a/test/src/unit-cppapi-enumerations.cc b/test/src/unit-cppapi-enumerations.cc
@@ -434,7 +434,7 @@ TEST_CASE_METHOD(
   // Test with `rest.load_enumerations_on_array_open` enabled and disabled.
   bool load_enmrs = GENERATE(true, false);
   auto config = ctx_.config();
-  config["rest.load_enumerations_on_array_open"] =
+  config["rest.load_enumerations_on_array_open_all_schemas"] =
       load_enmrs ? "true" : "false";
   vfs_test_setup_.update_config(config.ptr().get());
   ctx_ = vfs_test_setup_.ctx();
@@ -639,7 +639,7 @@ TEST_CASE_METHOD(
   // Test with `rest.load_enumerations_on_array_open` enabled and disabled.
   bool load_enmrs = GENERATE(true, false);
   auto config = ctx_.config();
-  config["rest.load_enumerations_on_array_open"] =
+  config["rest.load_enumerations_on_array_open_all_schemas"] =
       load_enmrs ? "true" : "false";
   vfs_test_setup_.update_config(config.ptr().get());
   ctx_ = vfs_test_setup_.ctx();

diff --git a/tiledb/sm/array/array.cc b/tiledb/sm/array/array.cc
@@ -601,9 +601,10 @@ Status Array::open(
     // For fetching remote enumerations REST calls
     // tiledb_handle_load_enumerations_request which loads enumerations. For
     // local arrays we don't call this method.
-    if (config_.get<bool>(
-            "rest.load_enumerations_on_array_open", Config::must_find)) {
-      load_all_enumerations(use_refactored_array_open());
+    if (serialize_enumerations()) {
+      load_all_enumerations(config_.get<bool>(
+          "rest.load_enumerations_on_array_open_all_schemas",
+          Config::must_find));
     }
   }
 
@@ -1615,13 +1616,11 @@ bool Array::serialize_non_empty_domain() const {
 }
 
 bool Array::serialize_enumerations() const {
-  auto serialize = config_.get<bool>("rest.load_enumerations_on_array_open");
-  if (!serialize.has_value()) {
-    throw std::runtime_error(
-        "Cannot get rest.load_enumerations_on_array_open configuration option "
-        "from config");
-  }
-  return serialize.value();
+  return config_.get<bool>(
+             "rest.load_enumerations_on_array_open", Config::must_find) ||
+         config_.get<bool>(
+             "rest.load_enumerations_on_array_open_all_schemas",
+             Config::must_find);
 }
 
 bool Array::serialize_metadata() const {

diff --git a/tiledb/sm/array/array.h b/tiledb/sm/array/array.h
@@ -985,7 +985,11 @@ class Array {
   bool serialize_non_empty_domain() const;
 
   /**
-   * Checks the config to se if enumerations should be serialized on array open.
+   * Checks the config to see if enumerations should be serialized on array
+   * open.
+   *
+   * @return True if either `rest.load_enumerations_on_array_open` or
+   * `rest.load_enumerations_on_array_open_all_schemas` is set, else False.
    */
   bool serialize_enumerations() const;
 

diff --git a/tiledb/sm/c_api/tiledb.cc b/tiledb/sm/c_api/tiledb.cc
@@ -2155,7 +2155,7 @@ capi_return_t tiledb_handle_load_array_schema_request(
           static_cast<tiledb::sm::SerializationType>(serialization_type),
           request->buffer());
 
-  if (load_schema_req.include_enumerations()) {
+  if (load_schema_req.include_enumerations_latest_schema()) {
     array->load_all_enumerations();
   }
 

diff --git a/tiledb/sm/config/config.cc b/tiledb/sm/config/config.cc
@@ -94,6 +94,8 @@ const std::string Config::REST_CURL_VERBOSE = "false";
 const std::string Config::REST_CURL_TCP_KEEPALIVE = "true";
 const std::string Config::REST_CURL_RETRY_ERRORS = "true";
 const std::string Config::REST_LOAD_ENUMERATIONS_ON_ARRAY_OPEN = "false";
+const std::string Config::REST_LOAD_ENUMERATIONS_ON_ARRAY_OPEN_ALL_SCHEMAS =
+    "false";
 const std::string Config::REST_LOAD_METADATA_ON_ARRAY_OPEN = "true";
 const std::string Config::REST_LOAD_NON_EMPTY_DOMAIN_ON_ARRAY_OPEN = "true";
 const std::string Config::REST_USE_REFACTORED_ARRAY_OPEN = "true";
@@ -264,6 +266,9 @@ const std::map<std::string, std::string> default_config_values = {
     std::make_pair(
         "rest.load_enumerations_on_array_open",
         Config::REST_LOAD_ENUMERATIONS_ON_ARRAY_OPEN),
+    std::make_pair(
+        "rest.load_enumerations_on_array_open_all_schemas",
+        Config::REST_LOAD_ENUMERATIONS_ON_ARRAY_OPEN_ALL_SCHEMAS),
     std::make_pair(
         "rest.load_metadata_on_array_open",
         Config::REST_LOAD_METADATA_ON_ARRAY_OPEN),

diff --git a/tiledb/sm/config/config.h b/tiledb/sm/config/config.h
@@ -116,9 +116,18 @@ class Config {
   /** If we should retry Curl errors in requests to REST. */
   static const std::string REST_CURL_RETRY_ERRORS;
 
-  /** If the array enumerations should be loaded on array open */
+  /**
+   * If the array enumerations should be loaded on array open for the latest
+   * array schema only.
+   */
   static const std::string REST_LOAD_ENUMERATIONS_ON_ARRAY_OPEN;
 
+  /**
+   * If the array enumerations should be loaded on array open for all array
+   * schemas.
+   */
+  static const std::string REST_LOAD_ENUMERATIONS_ON_ARRAY_OPEN_ALL_SCHEMAS;
+
   /** If the array metadata should be loaded on array open */
   static const std::string REST_LOAD_METADATA_ON_ARRAY_OPEN;
 

diff --git a/tiledb/sm/cpp_api/config.h b/tiledb/sm/cpp_api/config.h
@@ -928,8 +928,12 @@ class Config {
    *    together with the open array <br>
    *    **Default**: true
    * - `rest.load_enumerations_on_array_open` <br>
-   *    If true, enumerations will be loaded and sent to server together with
-   *    the open array.
+   *    If true, enumerations will be loaded for the latest array schema on
+   *    array open.
+   *    **Default**: false
+   * - `rest.load_enumerations_on_array_open_all_schemas` <br>
+   *    If true, enumerations will be loaded for all array schemas on array
+   *    open.
    *    **Default**: false
    * - `rest.use_refactored_array_open` <br>
    *    If true, the new REST routes and APIs for opening an array will be used

diff --git a/tiledb/sm/serialization/array.cc b/tiledb/sm/serialization/array.cc
@@ -148,7 +148,8 @@ Status array_to_capnp(
   array_builder->setQueryType(query_type_str(array->get_query_type()));
 
   if (array->use_refactored_array_open() && array->serialize_enumerations()) {
-    array->load_all_enumerations(true);
+    array->load_all_enumerations(array->config().get<bool>(
+        "rest.load_enumerations_on_array_open_all_schemas", Config::must_find));
   }
 
   const auto& array_schema_latest = array->array_schema_latest();

diff --git a/tiledb/sm/serialization/array_schema.cc b/tiledb/sm/serialization/array_schema.cc
@@ -1638,7 +1638,7 @@ void load_array_schema_request_to_capnp(
   throw_if_not_ok(config_to_capnp(config, &config_builder));
   // This boolean is only serialized to support clients using TileDB < 2.26.
   // Future options should only be serialized within the Config object above.
-  builder.setIncludeEnumerations(req.include_enumerations());
+  builder.setIncludeEnumerations(req.include_enumerations_latest_schema());
 }
 
 void serialize_load_array_schema_request(

diff --git a/tiledb/sm/serialization/array_schema.h b/tiledb/sm/serialization/array_schema.h
@@ -60,16 +60,23 @@ namespace serialization {
 class LoadArraySchemaRequest {
  public:
   explicit LoadArraySchemaRequest(const Config& config)
-      : include_enumerations_(config.get<bool>(
+      : include_enumerations_latest_schema_(config.get<bool>(
+            "rest.load_enumerations_on_array_open", Config::must_find))
+      , include_enumerations_all_schemas_(config.get<bool>(
             "rest.load_enumerations_on_array_open", Config::must_find)) {
   }
 
-  inline bool include_enumerations() const {
-    return include_enumerations_;
+  inline bool include_enumerations_latest_schema() const {
+    return include_enumerations_latest_schema_;
+  }
+
+  inline bool include_enumerations_all_schemas() const {
+    return include_enumerations_all_schemas_;
   }
 
  private:
-  bool include_enumerations_;
+  bool include_enumerations_latest_schema_;
+  bool include_enumerations_all_schemas_;
 };
 
 #ifdef TILEDB_SERIALIZATION