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

Author: franzpoeschel

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;

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();
     }
 }