[Backport release-2.20] Support reading V1 group details with explici…

…t version in the name. (#4744) (#4746) Backport a84789e from #4744. --- TYPE: BUG DESC: Support reading V1 group details with explicit version in the name. --------- Co-authored-by: Theodore Tsirpanis <[email protected]>
TileDB-Inc · Feb 27, 2024 · e294da4 · e294da4
1 parent cfc5456
commit e294da4
Show file tree

Hide file tree

Showing 5 changed files with 61 additions and 1 deletion.
diff --git a/...ts/groups/group_v1/__group/__1708362383727_1708362383727_a464f3a3a7e740c8856a476ee4c66ce1 b/...ts/groups/group_v1/__group/__1708362383727_1708362383727_a464f3a3a7e740c8856a476ee4c66ce1
diff --git a/test/inputs/groups/group_v1/__tiledb_group.tdb b/test/inputs/groups/group_v1/__tiledb_group.tdb
diff --git a/test/inputs/groups/group_v1/subgroup1/__tiledb_group.tdb b/test/inputs/groups/group_v1/subgroup1/__tiledb_group.tdb
diff --git a/test/src/unit-backwards_compat.cc b/test/src/unit-backwards_compat.cc
@@ -34,12 +34,15 @@
 #include <test/support/tdb_catch.h>
 #include "test/support/src/helpers.h"
 #include "test/support/src/serialization_wrappers.h"
+#include "test/support/src/temporary_local_directory.h"
 #include "tiledb/common/common.h"
+#include "tiledb/common/stdx_string.h"
 #include "tiledb/sm/cpp_api/tiledb"
 #include "tiledb/sm/cpp_api/tiledb_experimental"
 #include "tiledb/sm/misc/constants.h"
 
 #include <chrono>
+#include <filesystem>
 #include <iostream>
 #include <sstream>
 #include <thread>
@@ -52,6 +55,9 @@ namespace {
 static const std::string arrays_dir =
     std::string(TILEDB_TEST_INPUTS_DIR) + "/arrays";
 
+static const std::string groups_dir =
+    std::string(TILEDB_TEST_INPUTS_DIR) + "/groups";
+
 template <typename T>
 void set_query_coords(
     const Domain& domain,
@@ -1420,3 +1426,53 @@ TEST_CASE(
         g, "u64", TILEDB_UINT64, 0x7777777777777777);
   }
 }
+
+TEST_CASE(
+    "Backwards compatibility: Test v1 groups",
+    "[backwards-compat][group][v1]") {
+  Context ctx;
+  VFS vfs(ctx);
+
+  // Copy the group to a temporary directory because we will be modifying it.
+  tiledb::sm::TemporaryLocalDirectory temp_dir;
+  std::filesystem::copy(
+      groups_dir + "/group_v1",
+      temp_dir.path(),
+      std::filesystem::copy_options::recursive);
+
+  // Read the group
+  {
+    Group g{ctx, temp_dir.path(), TILEDB_READ};
+
+    CHECK(g.dump(false) != "");
+    CHECK(g.member_count() == 1);
+  }
+
+  // Add a member to the group
+  {
+    Group g{ctx, temp_dir.path(), TILEDB_WRITE};
+
+    Group::create(ctx, temp_dir.path() + "/subgroup2");
+
+    g.add_member("subgroup2", true, "subgroup2");
+
+    g.close();
+  }
+
+  // Read the group again
+  {
+    Group g{ctx, temp_dir.path(), TILEDB_READ};
+
+    CHECK(g.dump(false) != "");
+    CHECK(g.member_count() == 2);
+    CHECK(g.member(1).name() == "subgroup2");
+  }
+
+  // Read the raw group details files
+  auto children = vfs.ls(temp_dir.path() + "/__group");
+  CHECK(children.size() == 2);
+  std::sort(children.begin(), children.end());
+  CHECK(!tiledb::sm::utils::parse::ends_with(children[0], "_1"));
+  // This is the file written by this test.
+  CHECK(tiledb::sm::utils::parse::ends_with(children[1], "_1"));
+}
diff --git a/tiledb/sm/storage_manager/storage_manager.cc b/tiledb/sm/storage_manager/storage_manager.cc
@@ -1714,8 +1714,12 @@ StorageManagerCanonical::load_group_details(
 
   // V1 groups did not have the version appended so only have 4 "_"
   // (__<timestamp>_<timestamp>_<uuid>)
+  // Since 2.19, V1 groups also have the version appended so we have
+  // to check for that as well
   auto part = latest_group_uri.last_path_part();
-  if (std::count(part.begin(), part.end(), '_') == 4) {
+  auto underscoreCount = std::count(part.begin(), part.end(), '_');
+  if (underscoreCount == 4 ||
+      (underscoreCount == 5 && utils::parse::ends_with(part, "_1"))) {
     return load_group_from_uri(
         group_directory->uri(), latest_group_uri, encryption_key);
   }