Skip to content

Commit 9683584

Browse files
shaunrd0ypatia
shaunrd0
and
ypatia
authored
[Backport release-2.26] Fix schema timestamps used when loading enumerations from REST. (#5291) (#5293)
Backport of #5291 to release-2.26. --- TYPE: NO_HISTORY DESC: Fix schema timestamps used when loading enumerations from REST. --------- Co-authored-by: Ypatia Tsavliri <[email protected]>
1 parent ffb4b8e commit 9683584

File tree

3 files changed

+156
-36
lines changed

3 files changed

+156
-36
lines changed

‎test/src/unit-cppapi-enumerations.cc‎

Lines changed: 143 additions & 31 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@
3333
#include <fstream>
3434

3535
#include <test/support/tdb_catch.h>
36+
#include "test/support/src/vfs_helpers.h"
3637
#include "tiledb/api/c_api/enumeration/enumeration_api_internal.h"
3738
#include "tiledb/sm/array_schema/array_schema.h"
3839
#include "tiledb/sm/c_api/tiledb_struct_def.h"
@@ -43,14 +44,14 @@ using namespace tiledb;
4344

4445
struct CPPEnumerationFx {
4546
CPPEnumerationFx();
46-
~CPPEnumerationFx();
47+
~CPPEnumerationFx() = default;
4748

4849
template <typename T>
4950
void check_dump(const T& val);
5051

5152
void create_array(bool with_empty_enumeration = false);
52-
void rm_array();
5353

54+
tiledb::test::VFSTestSetup vfs_test_setup_;
5455
std::string uri_;
5556
Context ctx_;
5657
VFS vfs_;
@@ -283,7 +284,7 @@ TEST_CASE_METHOD(
283284
TEST_CASE_METHOD(
284285
CPPEnumerationFx,
285286
"CPP: Enumerations From Disk - Array::get_enumeration",
286-
"[enumeration][array-get-enumeration]") {
287+
"[enumeration][array-get-enumeration][rest]") {
287288
create_array();
288289
auto array = Array(ctx_, uri_, TILEDB_READ);
289290
auto enmr = ArrayExperimental::get_enumeration(ctx_, array, "an_enumeration");
@@ -297,7 +298,7 @@ TEST_CASE_METHOD(
297298
TEST_CASE_METHOD(
298299
CPPEnumerationFx,
299300
"CPP: Enumerations From Disk - Attribute::get_enumeration_name",
300-
"[enumeration][attr-get-enumeration-name]") {
301+
"[enumeration][attr-get-enumeration-name][rest]") {
301302
create_array();
302303
auto schema = Array::load_schema(ctx_, uri_);
303304

@@ -313,7 +314,7 @@ TEST_CASE_METHOD(
313314
TEST_CASE_METHOD(
314315
CPPEnumerationFx,
315316
"CPP: Array::load_all_enumerations",
316-
"[enumeration][array-load-all-enumerations]") {
317+
"[enumeration][array-load-all-enumerations][rest]") {
317318
create_array();
318319
auto array = Array(ctx_, uri_, TILEDB_READ);
319320
REQUIRE_NOTHROW(ArrayExperimental::load_all_enumerations(ctx_, array));
@@ -322,11 +323,132 @@ TEST_CASE_METHOD(
322323
TEST_CASE_METHOD(
323324
CPPEnumerationFx,
324325
"C API: Array load_all_enumerations - Check nullptr",
325-
"[enumeration][array-load-all-enumerations]") {
326+
"[enumeration][array-load-all-enumerations][rest]") {
326327
auto rc = tiledb_array_load_all_enumerations(ctx_.ptr().get(), nullptr);
327328
REQUIRE(rc != TILEDB_OK);
328329
}
329330

331+
TEST_CASE_METHOD(
332+
CPPEnumerationFx,
333+
"CPP API: Load All Enumerations - All Schemas",
334+
"[enumeration][array][load-all-enumerations][all-schemas][rest]") {
335+
create_array();
336+
auto array = tiledb::Array(ctx_, uri_, TILEDB_READ);
337+
auto schema = array.load_schema(ctx_, uri_);
338+
REQUIRE(
339+
schema.ptr()->array_schema_->has_enumeration("an_enumeration") == true);
340+
REQUIRE(
341+
schema.ptr()->array_schema_->is_enumeration_loaded("an_enumeration") ==
342+
false);
343+
std::string schema_name_1 = schema.ptr()->array_schema_->name();
344+
345+
// Evolve once to add an enumeration.
346+
ArraySchemaEvolution ase(ctx_);
347+
std::vector<std::string> var_values{"one", "two", "three"};
348+
auto var_enmr = Enumeration::create(ctx_, "ase_var_enmr", var_values);
349+
ase.add_enumeration(var_enmr);
350+
auto attr4 = Attribute::create<uint16_t>(ctx_, "attr4");
351+
AttributeExperimental::set_enumeration_name(ctx_, attr4, "ase_var_enmr");
352+
ase.add_attribute(attr4);
353+
ase.array_evolve(uri_);
354+
array.reopen();
355+
ArrayExperimental::load_all_enumerations(ctx_, array);
356+
auto all_schemas = array.ptr()->array_->array_schemas_all();
357+
schema = array.load_schema(ctx_, uri_);
358+
std::string schema_name_2 = schema.ptr()->array_schema_->name();
359+
360+
// Check all schemas.
361+
CHECK(all_schemas[schema_name_1]->has_enumeration("an_enumeration") == true);
362+
CHECK(
363+
all_schemas[schema_name_1]->is_enumeration_loaded("an_enumeration") ==
364+
true);
365+
CHECK(all_schemas[schema_name_2]->has_enumeration("an_enumeration") == true);
366+
CHECK(
367+
all_schemas[schema_name_2]->is_enumeration_loaded("an_enumeration") ==
368+
true);
369+
CHECK(all_schemas[schema_name_2]->has_enumeration("ase_var_enmr") == true);
370+
CHECK(
371+
all_schemas[schema_name_2]->is_enumeration_loaded("ase_var_enmr") ==
372+
true);
373+
374+
// Evolve a second time to drop an enumeration.
375+
ArraySchemaEvolution ase2(ctx_);
376+
ase2.drop_enumeration("an_enumeration");
377+
ase2.drop_attribute("attr1");
378+
CHECK_NOTHROW(ase2.array_evolve(uri_));
379+
// Apply evolution to the array and reopen.
380+
CHECK_NOTHROW(array.close());
381+
CHECK_NOTHROW(array.open(TILEDB_READ));
382+
ArrayExperimental::load_all_enumerations(ctx_, array);
383+
all_schemas = array.ptr()->array_->array_schemas_all();
384+
schema = array.load_schema(ctx_, uri_);
385+
std::string schema_name_3 = schema.ptr()->array_schema_->name();
386+
387+
// Check all schemas.
388+
CHECK(all_schemas[schema_name_1]->has_enumeration("an_enumeration") == true);
389+
CHECK(
390+
all_schemas[schema_name_1]->is_enumeration_loaded("an_enumeration") ==
391+
true);
392+
CHECK(all_schemas[schema_name_2]->has_enumeration("an_enumeration") == true);
393+
CHECK(
394+
all_schemas[schema_name_2]->is_enumeration_loaded("an_enumeration") ==
395+
true);
396+
CHECK(all_schemas[schema_name_2]->has_enumeration("ase_var_enmr") == true);
397+
CHECK(
398+
all_schemas[schema_name_2]->is_enumeration_loaded("ase_var_enmr") ==
399+
true);
400+
CHECK(all_schemas[schema_name_3]->has_enumeration("an_enumeration") == false);
401+
CHECK(all_schemas[schema_name_3]->has_enumeration("ase_var_enmr") == true);
402+
CHECK(
403+
all_schemas[schema_name_3]->is_enumeration_loaded("ase_var_enmr") ==
404+
true);
405+
406+
// Evolve a third time to add an enumeration with a name equal to a previously
407+
// dropped enumeration.
408+
ArraySchemaEvolution ase3(ctx_);
409+
auto old_enmr = Enumeration::create(ctx_, "an_enumeration", var_values);
410+
ase3.add_enumeration(old_enmr);
411+
auto attr5 = Attribute::create<uint16_t>(ctx_, "attr5");
412+
AttributeExperimental::set_enumeration_name(ctx_, attr5, "an_enumeration");
413+
ase.add_attribute(attr5);
414+
CHECK_NOTHROW(ase3.array_evolve(uri_));
415+
416+
// Apply evolution to the array and reopen.
417+
CHECK_NOTHROW(array.close());
418+
CHECK_NOTHROW(array.open(TILEDB_READ));
419+
ArrayExperimental::load_all_enumerations(ctx_, array);
420+
all_schemas = array.ptr()->array_->array_schemas_all();
421+
schema = array.load_schema(ctx_, uri_);
422+
std::string schema_name_4 = schema.ptr()->array_schema_->name();
423+
424+
// Check all schemas.
425+
CHECK(all_schemas[schema_name_1]->has_enumeration("an_enumeration") == true);
426+
CHECK(
427+
all_schemas[schema_name_1]->is_enumeration_loaded("an_enumeration") ==
428+
true);
429+
CHECK(all_schemas[schema_name_2]->has_enumeration("an_enumeration") == true);
430+
CHECK(
431+
all_schemas[schema_name_2]->is_enumeration_loaded("an_enumeration") ==
432+
true);
433+
CHECK(all_schemas[schema_name_2]->has_enumeration("ase_var_enmr") == true);
434+
CHECK(
435+
all_schemas[schema_name_2]->is_enumeration_loaded("ase_var_enmr") ==
436+
true);
437+
CHECK(all_schemas[schema_name_3]->has_enumeration("an_enumeration") == false);
438+
CHECK(all_schemas[schema_name_3]->has_enumeration("ase_var_enmr") == true);
439+
CHECK(
440+
all_schemas[schema_name_3]->is_enumeration_loaded("ase_var_enmr") ==
441+
true);
442+
CHECK(all_schemas[schema_name_4]->has_enumeration("an_enumeration") == true);
443+
CHECK(
444+
all_schemas[schema_name_4]->is_enumeration_loaded("an_enumeration") ==
445+
true);
446+
CHECK(all_schemas[schema_name_4]->has_enumeration("ase_var_enmr") == true);
447+
CHECK(
448+
all_schemas[schema_name_4]->is_enumeration_loaded("ase_var_enmr") ==
449+
true);
450+
}
451+
330452
TEST_CASE_METHOD(
331453
CPPEnumerationFx,
332454
"CPP: ArraySchemaEvolution - Add Enumeration",
@@ -340,7 +462,7 @@ TEST_CASE_METHOD(
340462
TEST_CASE_METHOD(
341463
CPPEnumerationFx,
342464
"C API: ArraySchemaEvolution - Add Enumeration - Check nullptr",
343-
"[enumeration][array-schema-evolution][error]") {
465+
"[enumeration][array-schema-evolution][error][rest]") {
344466
auto rc = tiledb_array_schema_evolution_add_enumeration(
345467
ctx_.ptr().get(), nullptr, nullptr);
346468
REQUIRE(rc != TILEDB_OK);
@@ -359,7 +481,7 @@ TEST_CASE_METHOD(
359481
TEST_CASE_METHOD(
360482
CPPEnumerationFx,
361483
"C API: ArraySchemaEvolution - Extend Enumeration - Check nullptr",
362-
"[enumeration][array-schema-evolution][drop-enumeration]") {
484+
"[enumeration][array-schema-evolution][drop-enumeration][rest]") {
363485
std::vector<std::string> values = {"fred", "wilma", "barney", "pebbles"};
364486
auto enmr = Enumeration::create(ctx_, enmr_name, values);
365487

@@ -384,7 +506,7 @@ TEST_CASE_METHOD(
384506
TEST_CASE_METHOD(
385507
CPPEnumerationFx,
386508
"C API: ArraySchemaEvolution - Drop Enumeration - Check nullptr",
387-
"[enumeration][array-schema-evolution][drop-enumeration]") {
509+
"[enumeration][array-schema-evolution][drop-enumeration][rest]") {
388510
auto rc = tiledb_array_schema_evolution_drop_enumeration(
389511
ctx_.ptr().get(), nullptr, "foo");
390512
REQUIRE(rc != TILEDB_OK);
@@ -398,7 +520,7 @@ TEST_CASE_METHOD(
398520
TEST_CASE_METHOD(
399521
CPPEnumerationFx,
400522
"CPP: Enumeration Query - Basic",
401-
"[enumeration][query][basic]") {
523+
"[enumeration][query][basic][rest]") {
402524
// Basic smoke test. Check that a simple query condition applied against
403525
// an array returns sane results.
404526
create_array();
@@ -434,7 +556,7 @@ TEST_CASE_METHOD(
434556
TEST_CASE_METHOD(
435557
CPPEnumerationFx,
436558
"CPP: Enumeration Query - Negation",
437-
"[enumeration][query][negation]") {
559+
"[enumeration][query][negation][rest]") {
438560
// Another basic query test, the only twist here is that we're checking
439561
// that query condition negation works as expected.
440562
create_array();
@@ -473,7 +595,7 @@ TEST_CASE_METHOD(
473595
TEST_CASE_METHOD(
474596
CPPEnumerationFx,
475597
"CPP: Enumeration Query - Combination",
476-
"[enumeration][query][combination]") {
598+
"[enumeration][query][combination][rest]") {
477599
// Same test as before except using multi-condition query condtions
478600
create_array();
479601

@@ -529,7 +651,7 @@ TEST_CASE_METHOD(
529651
TEST_CASE_METHOD(
530652
CPPEnumerationFx,
531653
"CPP: Enumeration Query - Invalid Enumeration Value is Always False",
532-
"[enumeration][query][basic]") {
654+
"[enumeration][query][basic][rest]") {
533655
create_array();
534656

535657
// Attempt to query with an enumeration value that isn't in the Enumeration
@@ -563,7 +685,7 @@ TEST_CASE_METHOD(
563685
TEST_CASE_METHOD(
564686
CPPEnumerationFx,
565687
"CPP: Enumeration Query - Invalid Enumeration Value Accepted by EQ",
566-
"[enumeration][query][basic]") {
688+
"[enumeration][query][basic][rest]") {
567689
create_array();
568690

569691
// Attempt to query with an enumeration value that isn't in the Enumeration
@@ -590,7 +712,7 @@ TEST_CASE_METHOD(
590712
TEST_CASE_METHOD(
591713
CPPEnumerationFx,
592714
"CPP: Enumeration Query - Invalid Enumeration Value Accepted by IN",
593-
"[enumeration][query][basic]") {
715+
"[enumeration][query][basic][rest]") {
594716
create_array();
595717

596718
// Attempt to query with an enumeration value that isn't in the Enumeration
@@ -617,7 +739,7 @@ TEST_CASE_METHOD(
617739
TEST_CASE_METHOD(
618740
CPPEnumerationFx,
619741
"CPP: Enumeration Query - Set Use Enumeration",
620-
"[enumeration][query][set-use-enumeration]") {
742+
"[enumeration][query][set-use-enumeration][rest]") {
621743
QueryCondition qc(ctx_);
622744
qc.init("attr1", "fred", 4, TILEDB_EQ);
623745
REQUIRE_NOTHROW(
@@ -629,7 +751,7 @@ TEST_CASE_METHOD(
629751
TEST_CASE_METHOD(
630752
CPPEnumerationFx,
631753
"C API: Enumeration Query - Check nullptr",
632-
"[enumeration][query][check-nullptr]") {
754+
"[enumeration][query][check-nullptr][rest]") {
633755
auto rc =
634756
tiledb_query_condition_set_use_enumeration(ctx_.ptr().get(), nullptr, 0);
635757
REQUIRE(rc != TILEDB_OK);
@@ -638,7 +760,7 @@ TEST_CASE_METHOD(
638760
TEST_CASE_METHOD(
639761
CPPEnumerationFx,
640762
"CPP: Enumeration Query - Attempt to query on empty enumeration",
641-
"[enumeration][query][empty-results]") {
763+
"[enumeration][query][empty-results][rest]") {
642764
create_array(true);
643765

644766
// Attempt to query with an enumeration value that isn't in the Enumeration
@@ -663,13 +785,9 @@ TEST_CASE_METHOD(
663785
}
664786

665787
CPPEnumerationFx::CPPEnumerationFx()
666-
: uri_("enumeration_test_array")
667-
, vfs_(ctx_) {
668-
rm_array();
669-
}
670-
671-
CPPEnumerationFx::~CPPEnumerationFx() {
672-
rm_array();
788+
: uri_(vfs_test_setup_.array_uri("enumeration_test_array"))
789+
, ctx_(vfs_test_setup_.ctx())
790+
, vfs_(vfs_test_setup_.ctx()) {
673791
}
674792

675793
template <typename T>
@@ -744,9 +862,3 @@ void CPPEnumerationFx::create_array(bool with_empty_enumeration) {
744862
query.finalize();
745863
array.close();
746864
}
747-
748-
void CPPEnumerationFx::rm_array() {
749-
if (vfs_.is_dir(uri_)) {
750-
vfs_.remove_dir(uri_);
751-
}
752-
}

‎test/src/unit-cppapi-schema-evolution.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -810,7 +810,7 @@ TEST_CASE(
810810

811811
TEST_CASE(
812812
"SchemaEvolution Error Handling Tests",
813-
"[cppapi][schema][evolution][errors]") {
813+
"[cppapi][schema][evolution][errors][rest]") {
814814
auto ase = make_shared<tiledb::sm::ArraySchemaEvolution>(
815815
HERE(), tiledb::test::create_test_memory_tracker());
816816
REQUIRE_THROWS(ase->evolve_schema(nullptr));

‎tiledb/sm/array/array.cc‎

Lines changed: 12 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -808,8 +808,16 @@ shared_ptr<const Enumeration> Array::get_enumeration(
808808
throw ArrayException("Unable to load enumerations; Array is not open.");
809809
}
810810

811-
return get_enumerations(
812-
{enumeration_name}, opened_array_->array_schema_latest_ptr())[0];
811+
auto schema = opened_array_->array_schema_latest_ptr();
812+
if (!schema->has_enumeration(enumeration_name)) {
813+
throw ArrayException(
814+
"Unable to get enumeration; Enumeration '" + enumeration_name +
815+
"' does not exist.");
816+
} else if (schema->is_enumeration_loaded(enumeration_name)) {
817+
return schema->get_enumeration(enumeration_name);
818+
}
819+
820+
return get_enumerations({enumeration_name}, schema)[0];
813821
}
814822

815823
std::vector<shared_ptr<const Enumeration>> Array::get_enumerations(
@@ -847,8 +855,8 @@ std::vector<shared_ptr<const Enumeration>> Array::get_enumerations(
847855

848856
loaded = rest_client->post_enumerations_from_rest(
849857
array_uri_,
850-
array_dir_timestamp_start_,
851-
array_dir_timestamp_end_,
858+
schema->timestamp_range().first,
859+
schema->timestamp_range().second,
852860
this,
853861
names_to_load,
854862
memory_tracker_);

0 commit comments

Comments
 (0)