Skip to content

Commit 5dcc0cd

Browse files
committed
Throw exception if not using array open v3.
1 parent 1ad6c91 commit 5dcc0cd

File tree

5 files changed

+38
-13
lines changed

5 files changed

+38
-13
lines changed

test/src/unit-cppapi-enumerations.cc

Lines changed: 11 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -349,6 +349,16 @@ TEST_CASE_METHOD(
349349
false);
350350
std::string schema_name_1 = schema.ptr()->array_schema()->name();
351351

352+
// If not using array open v3 just test that the correct exception is thrown
353+
if (!array.ptr()->array()->use_refactored_array_open()) {
354+
CHECK_THROWS_WITH(
355+
ArrayExperimental::load_enumerations_all_schemas(ctx_, array),
356+
Catch::Matchers::ContainsSubstring(
357+
"The array must be opened using "
358+
"`rest.use_refactored_array_open=true`"));
359+
return;
360+
}
361+
352362
// Evolve once to add an enumeration.
353363
ArraySchemaEvolution ase(ctx_);
354364
std::vector<std::string> var_values{"one", "two", "three"};
@@ -359,7 +369,7 @@ TEST_CASE_METHOD(
359369
ase.add_attribute(attr4);
360370
ase.array_evolve(uri_);
361371
array.reopen();
362-
ArrayExperimental::load_enumerations_all_schemas(ctx_, array);
372+
CHECK_NOTHROW(ArrayExperimental::load_enumerations_all_schemas(ctx_, array));
363373
auto all_schemas = array.ptr()->array()->array_schemas_all();
364374
schema = array.load_schema(ctx_, uri_);
365375
std::string schema_name_2 = schema.ptr()->array_schema()->name();

test/src/unit-enumerations.cc

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1128,6 +1128,16 @@ TEST_CASE_METHOD(
11281128
REQUIRE(schema->is_enumeration_loaded("test_enmr") == false);
11291129
std::string schema_name_1 = schema->name();
11301130

1131+
// If not using array open v3 just test that the correct exception is thrown
1132+
if (!array->use_refactored_array_open()) {
1133+
CHECK_THROWS_WITH(
1134+
array->load_all_enumerations(true),
1135+
Catch::Matchers::ContainsSubstring(
1136+
"The array must be opened using "
1137+
"`rest.use_refactored_array_open=true`"));
1138+
return;
1139+
}
1140+
11311141
// Evolve once to add an enumeration.
11321142
auto ase = make_shared<ArraySchemaEvolution>(HERE(), memory_tracker_);
11331143
std::vector<std::string> var_values{"one", "two", "three"};

tiledb/api/c_api/array/array_api.cc

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -666,6 +666,14 @@ capi_return_t tiledb_array_load_all_enumerations(const tiledb_array_t* array) {
666666
capi_return_t tiledb_array_load_enumerations_all_schemas(
667667
const tiledb_array_t* array) {
668668
ensure_array_is_valid(array);
669+
// Array::array_schemas_all_ is only initialized using array open V3, so we
670+
// won't have schemas to store the loaded enumerations unless array open V3 is
671+
// used.
672+
if (!array->array()->use_refactored_array_open()) {
673+
throw CAPIException(
674+
"Unable to load enumerations for all array schemas; The array must be "
675+
"opened using `rest.use_refactored_array_open=true");
676+
}
669677
array->load_all_enumerations(true);
670678
return TILEDB_OK;
671679
}

tiledb/api/c_api/array/array_api_experimental.h

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,9 @@ TILEDB_EXPORT capi_return_t tiledb_array_load_all_enumerations(
108108
/**
109109
* Load all enumerations for all schemas in the array.
110110
*
111+
* This method requires the array to be opened with the config option
112+
* `rest.use_refactored_array_open=true`.
113+
*
111114
* **Example:**
112115
*
113116
* @code{.c}

tiledb/sm/array/array.cc

Lines changed: 6 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -946,19 +946,13 @@ void Array::load_all_enumerations(bool all_schemas) {
946946
}
947947
// Load all enumerations, discarding the returned list of loaded enumerations.
948948
if (all_schemas) {
949-
// Since array open v1 does not initialize array_schemas_all, we need to
950-
// either serialize the full schemas with LoadEnumerationsResponse, or
951-
// reopen the array using array open v2 (only if the config is set to use
952-
// v1) so that we can store the enumerations from the response in the
953-
// correct schemas using the schema names.
954-
955-
// For now, reopen the array if it's found to be using array open v1.
956-
// Once we update to use post_array_schema_from_rest we will always have
957-
// array_schemas_all initialized and this could go away.
949+
// Unless we are using array open V3, Array::array_schemas_all_ will not be
950+
// initialized. We throw an exception since this is required to store the
951+
// loaded enumerations.
958952
if (!use_refactored_array_open()) {
959-
throw_if_not_ok(config_.set("rest.use_refactored_array_open", "true"));
960-
throw_if_not_ok(reopen());
961-
throw_if_not_ok(config_.set("rest.use_refactored_array_open", "false"));
953+
throw ArrayException(
954+
"Unable to load enumerations for all array schemas; The array must "
955+
"be opened using `rest.use_refactored_array_open=true`");
962956
}
963957

964958
get_enumerations_all_schemas();

0 commit comments

Comments
 (0)