Skip to content

Commit ef35123

Browse files
KiterLucKiterLuc
committed
Revert "[Backport release-2.26] Fix schema timestamps used when loading enumerations from REST. (#5291) (#5293)"
This reverts commit 9683584.
1 parent 9683584 commit ef35123

File tree

3 files changed

+36
-156
lines changed

3 files changed

+36
-156
lines changed

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

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

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

4544
struct CPPEnumerationFx {
4645
CPPEnumerationFx();
47-
~CPPEnumerationFx() = default;
46+
~CPPEnumerationFx();
4847

4948
template <typename T>
5049
void check_dump(const T& val);
5150

5251
void create_array(bool with_empty_enumeration = false);
52+
void rm_array();
5353

54-
tiledb::test::VFSTestSetup vfs_test_setup_;
5554
std::string uri_;
5655
Context ctx_;
5756
VFS vfs_;
@@ -284,7 +283,7 @@ TEST_CASE_METHOD(
284283
TEST_CASE_METHOD(
285284
CPPEnumerationFx,
286285
"CPP: Enumerations From Disk - Array::get_enumeration",
287-
"[enumeration][array-get-enumeration][rest]") {
286+
"[enumeration][array-get-enumeration]") {
288287
create_array();
289288
auto array = Array(ctx_, uri_, TILEDB_READ);
290289
auto enmr = ArrayExperimental::get_enumeration(ctx_, array, "an_enumeration");
@@ -298,7 +297,7 @@ TEST_CASE_METHOD(
298297
TEST_CASE_METHOD(
299298
CPPEnumerationFx,
300299
"CPP: Enumerations From Disk - Attribute::get_enumeration_name",
301-
"[enumeration][attr-get-enumeration-name][rest]") {
300+
"[enumeration][attr-get-enumeration-name]") {
302301
create_array();
303302
auto schema = Array::load_schema(ctx_, uri_);
304303

@@ -314,7 +313,7 @@ TEST_CASE_METHOD(
314313
TEST_CASE_METHOD(
315314
CPPEnumerationFx,
316315
"CPP: Array::load_all_enumerations",
317-
"[enumeration][array-load-all-enumerations][rest]") {
316+
"[enumeration][array-load-all-enumerations]") {
318317
create_array();
319318
auto array = Array(ctx_, uri_, TILEDB_READ);
320319
REQUIRE_NOTHROW(ArrayExperimental::load_all_enumerations(ctx_, array));
@@ -323,132 +322,11 @@ TEST_CASE_METHOD(
323322
TEST_CASE_METHOD(
324323
CPPEnumerationFx,
325324
"C API: Array load_all_enumerations - Check nullptr",
326-
"[enumeration][array-load-all-enumerations][rest]") {
325+
"[enumeration][array-load-all-enumerations]") {
327326
auto rc = tiledb_array_load_all_enumerations(ctx_.ptr().get(), nullptr);
328327
REQUIRE(rc != TILEDB_OK);
329328
}
330329

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-
452330
TEST_CASE_METHOD(
453331
CPPEnumerationFx,
454332
"CPP: ArraySchemaEvolution - Add Enumeration",
@@ -462,7 +340,7 @@ TEST_CASE_METHOD(
462340
TEST_CASE_METHOD(
463341
CPPEnumerationFx,
464342
"C API: ArraySchemaEvolution - Add Enumeration - Check nullptr",
465-
"[enumeration][array-schema-evolution][error][rest]") {
343+
"[enumeration][array-schema-evolution][error]") {
466344
auto rc = tiledb_array_schema_evolution_add_enumeration(
467345
ctx_.ptr().get(), nullptr, nullptr);
468346
REQUIRE(rc != TILEDB_OK);
@@ -481,7 +359,7 @@ TEST_CASE_METHOD(
481359
TEST_CASE_METHOD(
482360
CPPEnumerationFx,
483361
"C API: ArraySchemaEvolution - Extend Enumeration - Check nullptr",
484-
"[enumeration][array-schema-evolution][drop-enumeration][rest]") {
362+
"[enumeration][array-schema-evolution][drop-enumeration]") {
485363
std::vector<std::string> values = {"fred", "wilma", "barney", "pebbles"};
486364
auto enmr = Enumeration::create(ctx_, enmr_name, values);
487365

@@ -506,7 +384,7 @@ TEST_CASE_METHOD(
506384
TEST_CASE_METHOD(
507385
CPPEnumerationFx,
508386
"C API: ArraySchemaEvolution - Drop Enumeration - Check nullptr",
509-
"[enumeration][array-schema-evolution][drop-enumeration][rest]") {
387+
"[enumeration][array-schema-evolution][drop-enumeration]") {
510388
auto rc = tiledb_array_schema_evolution_drop_enumeration(
511389
ctx_.ptr().get(), nullptr, "foo");
512390
REQUIRE(rc != TILEDB_OK);
@@ -520,7 +398,7 @@ TEST_CASE_METHOD(
520398
TEST_CASE_METHOD(
521399
CPPEnumerationFx,
522400
"CPP: Enumeration Query - Basic",
523-
"[enumeration][query][basic][rest]") {
401+
"[enumeration][query][basic]") {
524402
// Basic smoke test. Check that a simple query condition applied against
525403
// an array returns sane results.
526404
create_array();
@@ -556,7 +434,7 @@ TEST_CASE_METHOD(
556434
TEST_CASE_METHOD(
557435
CPPEnumerationFx,
558436
"CPP: Enumeration Query - Negation",
559-
"[enumeration][query][negation][rest]") {
437+
"[enumeration][query][negation]") {
560438
// Another basic query test, the only twist here is that we're checking
561439
// that query condition negation works as expected.
562440
create_array();
@@ -595,7 +473,7 @@ TEST_CASE_METHOD(
595473
TEST_CASE_METHOD(
596474
CPPEnumerationFx,
597475
"CPP: Enumeration Query - Combination",
598-
"[enumeration][query][combination][rest]") {
476+
"[enumeration][query][combination]") {
599477
// Same test as before except using multi-condition query condtions
600478
create_array();
601479

@@ -651,7 +529,7 @@ TEST_CASE_METHOD(
651529
TEST_CASE_METHOD(
652530
CPPEnumerationFx,
653531
"CPP: Enumeration Query - Invalid Enumeration Value is Always False",
654-
"[enumeration][query][basic][rest]") {
532+
"[enumeration][query][basic]") {
655533
create_array();
656534

657535
// Attempt to query with an enumeration value that isn't in the Enumeration
@@ -685,7 +563,7 @@ TEST_CASE_METHOD(
685563
TEST_CASE_METHOD(
686564
CPPEnumerationFx,
687565
"CPP: Enumeration Query - Invalid Enumeration Value Accepted by EQ",
688-
"[enumeration][query][basic][rest]") {
566+
"[enumeration][query][basic]") {
689567
create_array();
690568

691569
// Attempt to query with an enumeration value that isn't in the Enumeration
@@ -712,7 +590,7 @@ TEST_CASE_METHOD(
712590
TEST_CASE_METHOD(
713591
CPPEnumerationFx,
714592
"CPP: Enumeration Query - Invalid Enumeration Value Accepted by IN",
715-
"[enumeration][query][basic][rest]") {
593+
"[enumeration][query][basic]") {
716594
create_array();
717595

718596
// Attempt to query with an enumeration value that isn't in the Enumeration
@@ -739,7 +617,7 @@ TEST_CASE_METHOD(
739617
TEST_CASE_METHOD(
740618
CPPEnumerationFx,
741619
"CPP: Enumeration Query - Set Use Enumeration",
742-
"[enumeration][query][set-use-enumeration][rest]") {
620+
"[enumeration][query][set-use-enumeration]") {
743621
QueryCondition qc(ctx_);
744622
qc.init("attr1", "fred", 4, TILEDB_EQ);
745623
REQUIRE_NOTHROW(
@@ -751,7 +629,7 @@ TEST_CASE_METHOD(
751629
TEST_CASE_METHOD(
752630
CPPEnumerationFx,
753631
"C API: Enumeration Query - Check nullptr",
754-
"[enumeration][query][check-nullptr][rest]") {
632+
"[enumeration][query][check-nullptr]") {
755633
auto rc =
756634
tiledb_query_condition_set_use_enumeration(ctx_.ptr().get(), nullptr, 0);
757635
REQUIRE(rc != TILEDB_OK);
@@ -760,7 +638,7 @@ TEST_CASE_METHOD(
760638
TEST_CASE_METHOD(
761639
CPPEnumerationFx,
762640
"CPP: Enumeration Query - Attempt to query on empty enumeration",
763-
"[enumeration][query][empty-results][rest]") {
641+
"[enumeration][query][empty-results]") {
764642
create_array(true);
765643

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

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

793675
template <typename T>
@@ -862,3 +744,9 @@ void CPPEnumerationFx::create_array(bool with_empty_enumeration) {
862744
query.finalize();
863745
array.close();
864746
}
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][rest]") {
813+
"[cppapi][schema][evolution][errors]") {
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: 4 additions & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -808,16 +808,8 @@ shared_ptr<const Enumeration> Array::get_enumeration(
808808
throw ArrayException("Unable to load enumerations; Array is not open.");
809809
}
810810

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];
811+
return get_enumerations(
812+
{enumeration_name}, opened_array_->array_schema_latest_ptr())[0];
821813
}
822814

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

856848
loaded = rest_client->post_enumerations_from_rest(
857849
array_uri_,
858-
schema->timestamp_range().first,
859-
schema->timestamp_range().second,
850+
array_dir_timestamp_start_,
851+
array_dir_timestamp_end_,
860852
this,
861853
names_to_load,
862854
memory_tracker_);

0 commit comments

Comments
 (0)