From 0d7208984fe4addf00d413a3d7f585d2baed3f3b Mon Sep 17 00:00:00 2001 From: Shaun Reed Date: Mon, 18 Nov 2024 14:33:11 -0500 Subject: [PATCH] Update to use new config parameter. --- test/src/unit-capi-config.cc | 3 +++ test/src/unit-cppapi-enumerations.cc | 4 ++-- tiledb/sm/array/array.cc | 19 +++++++++---------- tiledb/sm/array/array.h | 6 +++++- tiledb/sm/c_api/tiledb.cc | 2 +- tiledb/sm/config/config.cc | 5 +++++ tiledb/sm/config/config.h | 11 ++++++++++- tiledb/sm/cpp_api/config.h | 8 ++++++-- tiledb/sm/serialization/array.cc | 3 ++- tiledb/sm/serialization/array_schema.cc | 2 +- tiledb/sm/serialization/array_schema.h | 15 +++++++++++---- 11 files changed, 55 insertions(+), 23 deletions(-) diff --git a/test/src/unit-capi-config.cc b/test/src/unit-capi-config.cc index 36e52835416..37a731d0c70 100644 --- 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 index 03222cc2378..1947c6dcadc 100644 --- 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 index c4e1de27ca2..7b93c116cfe 100644 --- 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( - "rest.load_enumerations_on_array_open", Config::must_find)) { - load_all_enumerations(use_refactored_array_open()); + if (serialize_enumerations()) { + load_all_enumerations(config_.get( + "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("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( + "rest.load_enumerations_on_array_open", Config::must_find) || + config_.get( + "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 index 2861cbd5435..f846848f7c5 100644 --- 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 index ea967c4fd0a..fd929c9689b 100644 --- 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(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 index d879122e6c5..4e15c56702c 100644 --- 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 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 index 1f75e6f97a0..ef8dda564ca 100644 --- 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 index 0bccd534930..2ab91a6338a 100644 --- 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
* **Default**: true * - `rest.load_enumerations_on_array_open`
- * 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`
+ * If true, enumerations will be loaded for all array schemas on array + * open. * **Default**: false * - `rest.use_refactored_array_open`
* 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 index 1958b8406ce..4724cd7c36b 100644 --- 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( + "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 index 4496097ea8e..bc8baa186ba 100644 --- 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 index 54406042b70..aac214c57a9 100644 --- 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( + : include_enumerations_latest_schema_(config.get( + "rest.load_enumerations_on_array_open", Config::must_find)) + , include_enumerations_all_schemas_(config.get( "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