HDF5: Delete and re-create attribute when overwriting with diff. type (…

…#1697) * HDF5: Delete and re-create attribute when overwriting with diff. type * WIP: Change datatype in test Somehow truncation test is broken by this * Add forgotten H5Aclose() call
openPMD · Dec 4, 2024 · 77a55a3 · 77a55a3
1 parent 1bd63c7
commit 77a55a3
Show file tree

Hide file tree

Showing 2 changed files with 47 additions and 5 deletions.
diff --git a/src/IO/HDF5/HDF5IOHandler.cpp b/src/IO/HDF5/HDF5IOHandler.cpp
@@ -1694,8 +1694,7 @@ void HDF5IOHandlerImpl::writeAttribute(
         "[HDF5] Internal error: Failed to get HDF5 datatype during attribute "
         "write");
     std::string name = parameters.name;
-    if (H5Aexists(node_id, name.c_str()) == 0)
-    {
+    auto create_attribute_anew = [&]() {
         hid_t dataspace = getH5DataSpace(att);
         VERIFY(
             dataspace >= 0,
@@ -1717,14 +1716,47 @@ void HDF5IOHandlerImpl::writeAttribute(
             status == 0,
             "[HDF5] Internal error: Failed to close HDF5 dataspace during "
             "attribute write");
-    }
-    else
+    };
+    if (H5Aexists(node_id, name.c_str()) != 0)
     {
         attribute_id = H5Aopen(node_id, name.c_str(), H5P_DEFAULT);
         VERIFY(
-            node_id >= 0,
+            attribute_id >= 0,
             "[HDF5] Internal error: Failed to open HDF5 attribute during "
             "attribute write");
+        /*
+         * Only reuse the old attribute if it had the same type.
+         */
+        hid_t type_id = H5Aget_type(attribute_id);
+        VERIFY(
+            type_id >= 0,
+            "[HDF5] Internal error: Failed to inquire HDF5 attribute type "
+            "during attribute write");
+        auto equal = H5Tequal(type_id, dataType);
+        VERIFY(
+            equal >= 0,
+            "[HDF5] Internal error: Failed to compare HDF5 attribute types "
+            "during attribute write");
+        if (equal == 0) // unequal
+        {
+            status = H5Aclose(attribute_id);
+            VERIFY(
+                status == 0,
+                "[HDF5] Internal error: Failed to close previous HDF5 "
+                "attribute "
+                "during attribute write");
+            status = H5Adelete(node_id, name.c_str());
+            VERIFY(
+                status == 0,
+                "[HDF5] Internal error: Failed to delete previous HDF5 "
+                "attribute "
+                "during attribute write");
+            create_attribute_anew();
+        }
+    }
+    else
+    {
+        create_attribute_anew();
     }
 
     using DT = Datatype;

diff --git a/test/SerialIOTest.cpp b/test/SerialIOTest.cpp
@@ -7522,6 +7522,8 @@ void groupbased_read_write(std::string const &ext)
 
         E_x.setAttribute("updated_in_run", 0);
         E_y.setAttribute("updated_in_run", 0);
+        E_y.setAttribute("changed_datatype_in_run", 0);
+        write.close();
     }
 
     {
@@ -7540,6 +7542,8 @@ void groupbased_read_write(std::string const &ext)
 
         E_x.setAttribute("updated_in_run", 1);
         E_y.setAttribute("updated_in_run", 1);
+        E_y.setAttribute("changed_datatype_in_run", "one");
+        write.close();
     }
 
     {
@@ -7551,6 +7555,9 @@ void groupbased_read_write(std::string const &ext)
         REQUIRE(E_x_0_fromRun0.getAttribute("updated_in_run").get<int>() == 0);
         REQUIRE(E_x_1_fromRun1.getAttribute("updated_in_run").get<int>() == 1);
         REQUIRE(E_y_0_fromRun1.getAttribute("updated_in_run").get<int>() == 1);
+        REQUIRE(
+            E_y_0_fromRun1.getAttribute("changed_datatype_in_run")
+                .get<std::string>() == "one");
 
         auto chunk_E_x_0_fromRun0 = E_x_0_fromRun0.loadChunk<int>({0}, {1});
         auto chunk_E_x_1_fromRun1 = E_x_1_fromRun1.loadChunk<int>({0}, {1});
@@ -7561,6 +7568,7 @@ void groupbased_read_write(std::string const &ext)
         REQUIRE(*chunk_E_x_0_fromRun0 == 0);
         REQUIRE(*chunk_E_x_1_fromRun1 == 1);
         REQUIRE(*chunk_E_y_0_fromRun1 == 1);
+        read.close();
     }
 
     // check that truncation works correctly
@@ -7574,12 +7582,14 @@ void groupbased_read_write(std::string const &ext)
 
         E_x.storeChunkRaw(&data, {0}, {1});
         E_x.setAttribute("updated_in_run", 2);
+        write.close();
     }
 
     {
         Series read(filename, Access::READ_ONLY);
         REQUIRE(read.iterations.size() == 1);
         REQUIRE(read.iterations.count(2) == 1);
+        read.close();
     }
 }