From 822a14976d9873449bd80ff07e03ea01b96a1d39 Mon Sep 17 00:00:00 2001 From: Thomas Madlener Date: Mon, 29 Sep 2025 10:27:21 +0200 Subject: [PATCH 1/8] Add missing docstring --- include/podio/DatamodelRegistry.h | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/include/podio/DatamodelRegistry.h b/include/podio/DatamodelRegistry.h index 760adff19..0b5c1f505 100644 --- a/include/podio/DatamodelRegistry.h +++ b/include/podio/DatamodelRegistry.h @@ -100,6 +100,15 @@ class DatamodelRegistry { /// @returns The name of the datamodel const std::string& getDatamodelName(size_t index) const; + /// Get the version of the datamodel with the given name + /// + /// @note This will return the version of the datamodel library **not the + /// schema version of the datamodel**. + /// + /// @param name The name of the datamodel + /// + /// @returns The version of the datamodel if the datamodel is known to the + /// registry or an empty optional otherwise std::optional getDatamodelVersion(const std::string& name) const; /// Register a datamodel and return its index in the registry. From 6506c7c7e1e29af14da573795ae4497c0826cb78 Mon Sep 17 00:00:00 2001 From: Thomas Madlener Date: Mon, 29 Sep 2025 11:13:15 +0200 Subject: [PATCH 2/8] Extract the schema version from the datamodel definition --- include/podio/DatamodelRegistry.h | 11 +++++++++ src/DatamodelRegistry.cc | 38 +++++++++++++++++++++++++++++++ 2 files changed, 49 insertions(+) diff --git a/include/podio/DatamodelRegistry.h b/include/podio/DatamodelRegistry.h index 0b5c1f505..1d5874437 100644 --- a/include/podio/DatamodelRegistry.h +++ b/include/podio/DatamodelRegistry.h @@ -1,6 +1,7 @@ #ifndef PODIO_DATAMODELREGISTRY_H #define PODIO_DATAMODELREGISTRY_H +#include "podio/SchemaEvolution.h" #include "podio/podioVersion.h" #include @@ -111,6 +112,14 @@ class DatamodelRegistry { /// registry or an empty optional otherwise std::optional getDatamodelVersion(const std::string& name) const; + /// Get the schema version of the datamodel with the given name + /// + /// @param name The name of the datamodel + /// + /// @returns The schema version of the datamodel if the datamodel is known to the + /// registry or an empty optional otherwise + std::optional getSchemaVersion(const std::string& name) const; + /// Register a datamodel and return its index in the registry. /// /// This is the hook that is called during dynamic loading of an EDM to @@ -142,6 +151,8 @@ class DatamodelRegistry { std::unordered_map m_relations{}; std::unordered_map m_datamodelVersions{}; + + std::unordered_map m_schemaVersions{}; }; } // namespace podio diff --git a/src/DatamodelRegistry.cc b/src/DatamodelRegistry.cc index 608d85d05..ee9d67f6d 100644 --- a/src/DatamodelRegistry.cc +++ b/src/DatamodelRegistry.cc @@ -1,6 +1,7 @@ #include "podio/DatamodelRegistry.h" #include +#include #include #include #include @@ -15,12 +16,42 @@ DatamodelRegistry& DatamodelRegistry::mutInstance() { return registryInstance; } +podio::SchemaVersionT extractSchemaVersion(const std::string_view definition) { + // Extract schema_version from JSON definition without full parsing + // Look for "schema_version": followed by a number + constexpr std::string_view schemaVersionKey = "\"schema_version\":"; + if (auto pos = definition.find(schemaVersionKey); pos != std::string_view::npos) { + pos += schemaVersionKey.length(); + // Skip whitespace + while (pos < definition.length() && std::isspace(definition[pos])) { + ++pos; + } + // Extract the number + auto start = pos; + while (pos < definition.length() && std::isdigit(definition[pos])) { + ++pos; + } + if (pos > start) { + // Convert substring to integer using std::from_chars for better error handling + podio::SchemaVersionT schemaVersion = 0; + auto result = std::from_chars(definition.data() + start, definition.data() + pos, schemaVersion); + if (result.ec == std::errc{}) { + return schemaVersion; + } + } + } + + // Return 0 if no valid schema version found + return 0; +} + size_t DatamodelRegistry::registerDatamodel(std::string name, std::string_view definition, const podio::RelationNameMapping& relationNames) { const auto it = std::ranges::find(m_definitions, name, &decltype(m_definitions)::value_type::first); if (it == m_definitions.cend()) { int index = m_definitions.size(); + m_schemaVersions.emplace(name, extractSchemaVersion(definition)); m_definitions.emplace_back(std::move(name), definition); for (const auto& [typeName, relations, vectorMembers] : relationNames) { @@ -104,4 +135,11 @@ std::optional DatamodelRegistry::getDatamodelVersion(co return std::nullopt; } +std::optional DatamodelRegistry::getSchemaVersion(const std::string& name) const { + if (const auto it = m_schemaVersions.find(name); it != m_schemaVersions.end()) { + return it->second; + } + return std::nullopt; +} + } // namespace podio From 01309612036ad731075f2a998215ccc679f6fb50 Mon Sep 17 00:00:00 2001 From: Thomas Madlener Date: Mon, 29 Sep 2025 11:34:10 +0200 Subject: [PATCH 3/8] Also extract schema version when reading definitions back --- .../utilities/DatamodelRegistryIOHelpers.h | 15 +++++++ src/DatamodelRegistry.cc | 33 +-------------- src/DatamodelRegistryIOHelpers.cc | 40 +++++++++++++++++++ 3 files changed, 57 insertions(+), 31 deletions(-) diff --git a/include/podio/utilities/DatamodelRegistryIOHelpers.h b/include/podio/utilities/DatamodelRegistryIOHelpers.h index de847ce42..da7868e2c 100644 --- a/include/podio/utilities/DatamodelRegistryIOHelpers.h +++ b/include/podio/utilities/DatamodelRegistryIOHelpers.h @@ -6,10 +6,18 @@ #include #include +#include #include #include namespace podio { +namespace detail { + /// Extract schema version from a JSON datamodel definition + /// + /// @param definition The JSON definition string + /// @returns The schema version found in the definition, or 0 if not found + podio::SchemaVersionT extractSchemaVersion(const std::string_view definition); +} // namespace detail /// Helper class to collect the datamodel (JSON) definitions that should be /// written. @@ -63,6 +71,13 @@ class DatamodelDefinitionHolder { std::optional getDatamodelVersion(const std::string& name) const; + /// Get the schema version for the given datamodel name by extracting it from + /// the stored datamodel definition. + /// + /// @param name The name of the datamodel + /// @returns The schema version if the datamodel is available, or std::nullopt otherwise + std::optional getSchemaVersion(const std::string& name) const; + protected: MapType m_availEDMDefs{}; VersionList m_edmVersions{}; diff --git a/src/DatamodelRegistry.cc b/src/DatamodelRegistry.cc index ee9d67f6d..70a337f45 100644 --- a/src/DatamodelRegistry.cc +++ b/src/DatamodelRegistry.cc @@ -1,7 +1,7 @@ #include "podio/DatamodelRegistry.h" +#include "podio/utilities/DatamodelRegistryIOHelpers.h" #include -#include #include #include #include @@ -16,42 +16,13 @@ DatamodelRegistry& DatamodelRegistry::mutInstance() { return registryInstance; } -podio::SchemaVersionT extractSchemaVersion(const std::string_view definition) { - // Extract schema_version from JSON definition without full parsing - // Look for "schema_version": followed by a number - constexpr std::string_view schemaVersionKey = "\"schema_version\":"; - if (auto pos = definition.find(schemaVersionKey); pos != std::string_view::npos) { - pos += schemaVersionKey.length(); - // Skip whitespace - while (pos < definition.length() && std::isspace(definition[pos])) { - ++pos; - } - // Extract the number - auto start = pos; - while (pos < definition.length() && std::isdigit(definition[pos])) { - ++pos; - } - if (pos > start) { - // Convert substring to integer using std::from_chars for better error handling - podio::SchemaVersionT schemaVersion = 0; - auto result = std::from_chars(definition.data() + start, definition.data() + pos, schemaVersion); - if (result.ec == std::errc{}) { - return schemaVersion; - } - } - } - - // Return 0 if no valid schema version found - return 0; -} - size_t DatamodelRegistry::registerDatamodel(std::string name, std::string_view definition, const podio::RelationNameMapping& relationNames) { const auto it = std::ranges::find(m_definitions, name, &decltype(m_definitions)::value_type::first); if (it == m_definitions.cend()) { int index = m_definitions.size(); - m_schemaVersions.emplace(name, extractSchemaVersion(definition)); + m_schemaVersions.emplace(name, detail::extractSchemaVersion(definition)); m_definitions.emplace_back(std::move(name), definition); for (const auto& [typeName, relations, vectorMembers] : relationNames) { diff --git a/src/DatamodelRegistryIOHelpers.cc b/src/DatamodelRegistryIOHelpers.cc index 02e1624ec..8118632ff 100644 --- a/src/DatamodelRegistryIOHelpers.cc +++ b/src/DatamodelRegistryIOHelpers.cc @@ -1,8 +1,40 @@ #include "podio/utilities/DatamodelRegistryIOHelpers.h" + #include +#include #include namespace podio { +namespace detail { + podio::SchemaVersionT extractSchemaVersion(const std::string_view definition) { + // Extract schema_version from JSON definition without full parsing + // Look for "schema_version": followed by a number + constexpr std::string_view schemaVersionKey = "\"schema_version\":"; + if (auto pos = definition.find(schemaVersionKey); pos != std::string_view::npos) { + pos += schemaVersionKey.length(); + // Skip whitespace + while (pos < definition.length() && std::isspace(definition[pos])) { + ++pos; + } + // Extract the number + auto start = pos; + while (pos < definition.length() && std::isdigit(definition[pos])) { + ++pos; + } + if (pos > start) { + // Convert substring to integer using std::from_chars for better error handling + podio::SchemaVersionT schemaVersion = 0; + auto result = std::from_chars(definition.data() + start, definition.data() + pos, schemaVersion); + if (result.ec == std::errc{}) { + return schemaVersion; + } + } + } + + // Return 0 if no valid schema version found + return 0; + } +} // namespace detail void DatamodelDefinitionCollector::registerDatamodelDefinition(const podio::CollectionBase* coll, const std::string& name) { @@ -57,4 +89,12 @@ std::optional DatamodelDefinitionHolder::getDatamodelVe return std::nullopt; } +std::optional DatamodelDefinitionHolder::getSchemaVersion(const std::string& name) const { + const auto definition = getDatamodelDefinition(name); + if (definition != "{}") { + return detail::extractSchemaVersion(definition); + } + return std::nullopt; +} + } // namespace podio From 22130f31995a2f5c1ace86fbd757a32fbdfcd8d7 Mon Sep 17 00:00:00 2001 From: Thomas Madlener Date: Mon, 29 Sep 2025 13:00:46 +0200 Subject: [PATCH 4/8] Make readers emit a waning in case we encounter a too new EDM version --- src/RNTupleReader.cc | 6 ++++++ src/ROOTReader.cc | 6 ++++++ src/SIOReader.cc | 5 +++++ src/ioUtils.h | 42 ++++++++++++++++++++++++++++++++++++++++++ 4 files changed, 59 insertions(+) create mode 100644 src/ioUtils.h diff --git a/src/RNTupleReader.cc b/src/RNTupleReader.cc index 2880c2c20..4e700707f 100644 --- a/src/RNTupleReader.cc +++ b/src/RNTupleReader.cc @@ -4,6 +4,8 @@ #include "podio/DatamodelRegistry.h" #include "podio/GenericParameters.h" #include "podio/utilities/RootHelpers.h" + +#include "ioUtils.h" #include "rootUtils.h" #include @@ -94,6 +96,10 @@ void RNTupleReader::openFiles(const std::vector& filenames) { } m_datamodelHolder = DatamodelDefinitionHolder(std::move(edm), std::move(edmVersions)); + for (const auto& warning : io_utils::checkEDMVersionsReadable(m_datamodelHolder)) { + std::cerr << "WARNING: " << warning << std::endl; + } + auto availableCategoriesField = m_metadata->GetView>(root_utils::availableCategories); m_availableCategories = availableCategoriesField(0); diff --git a/src/ROOTReader.cc b/src/ROOTReader.cc index bfc1b1d1a..8d4f16633 100644 --- a/src/ROOTReader.cc +++ b/src/ROOTReader.cc @@ -7,6 +7,8 @@ #include "podio/GenericParameters.h" #include "podio/podioVersion.h" #include "podio/utilities/RootHelpers.h" + +#include "ioUtils.h" #include "rootUtils.h" // ROOT specific includes @@ -326,6 +328,10 @@ void ROOTReader::openFiles(const std::vector& filenames) { } m_datamodelHolder = DatamodelDefinitionHolder(std::move(datamodelDefs), std::move(edmVersions)); + + for (const auto& warning : io_utils::checkEDMVersionsReadable(m_datamodelHolder)) { + std::cerr << "WARNING: " << warning << std::endl; + } } // Do some work up front for setting up categories and setup all the chains diff --git a/src/SIOReader.cc b/src/SIOReader.cc index b7c4a420c..059001735 100644 --- a/src/SIOReader.cc +++ b/src/SIOReader.cc @@ -1,5 +1,6 @@ #include "podio/SIOReader.h" +#include "ioUtils.h" #include "sioUtils.h" #include @@ -129,6 +130,10 @@ void SIOReader::readEDMDefinitions() { auto datamodelDefs = static_cast*>(blocks[0].get()); auto edmVersions = static_cast*>(blocks[1].get()); m_datamodelHolder = DatamodelDefinitionHolder(std::move(datamodelDefs->mapData), std::move(edmVersions->mapData)); + + for (const auto& warning : io_utils::checkEDMVersionsReadable(m_datamodelHolder)) { + std::cerr << "WARNING: " << warning << std::endl; + } } } // namespace podio diff --git a/src/ioUtils.h b/src/ioUtils.h new file mode 100644 index 000000000..e9f50ec5e --- /dev/null +++ b/src/ioUtils.h @@ -0,0 +1,42 @@ +#ifndef PODIO_IO_UTILS_H // NOLINT(llvm-header-guard): internal headers confuse clang-tidy +#define PODIO_IO_UTILS_H // NOLINT(llvm-header-guard): internal headers confuse clang-tidy + +#include "podio/DatamodelRegistry.h" +#include "podio/utilities/DatamodelRegistryIOHelpers.h" + +#include +#include +#include + +namespace podio::io_utils { + +/// Check and verify that all EDM (verions) that have been read from file are +/// readable +/// +/// Effectively this boils down to making sure that the EDM versions on file are +/// not newer than the ones that have been loaded dynamically. +/// +/// Returns all warnings that have been found +inline std::vector checkEDMVersionsReadable(const podio::DatamodelDefinitionHolder& fileEdms) { + std::vector warnings{}; + for (const auto& edmName : fileEdms.getAvailableDatamodels()) { + // There is no way we get an empty optional here + const auto fileSchemaVersion = fileEdms.getSchemaVersion(edmName).value(); + const auto envSchemaVersion = podio::DatamodelRegistry::instance().getSchemaVersion(edmName); + + if (!envSchemaVersion) { + warnings.emplace_back("EDM '" + edmName + + "' exists in file but does not seem to be loaded from the environment by podio"); + } + if (envSchemaVersion.value() < fileSchemaVersion) { + warnings.emplace_back("EDM '" + edmName + "' exists in file with schema version " + + std::to_string(fileSchemaVersion) + " but podio loaded schema version " + + std::to_string(envSchemaVersion.value()) + " from the environment"); + } + } + + return warnings; +} +} // namespace podio::io_utils + +#endif From 7ced4a9f53939e18db6af87c8e52e671d97750b1 Mon Sep 17 00:00:00 2001 From: Thomas Madlener Date: Mon, 29 Sep 2025 13:01:22 +0200 Subject: [PATCH 5/8] Catch exception in podio-dump-tool to exit more gracefully --- tools/src/podio-dump-tool.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/tools/src/podio-dump-tool.cpp b/tools/src/podio-dump-tool.cpp index 988a6c2fe..77699a643 100644 --- a/tools/src/podio-dump-tool.cpp +++ b/tools/src/podio-dump-tool.cpp @@ -12,6 +12,7 @@ #include #include +#include #include #include #include From 6c88377a1a762649c22be6a579fd14c663e3387b Mon Sep 17 00:00:00 2001 From: Thomas Madlener Date: Mon, 29 Sep 2025 13:05:35 +0200 Subject: [PATCH 6/8] Add test that checks that warning is emitted --- .../schema_evolution/code_gen/CMakeLists.txt | 46 +++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/tests/schema_evolution/code_gen/CMakeLists.txt b/tests/schema_evolution/code_gen/CMakeLists.txt index 789be0a0b..f5fc78213 100644 --- a/tests/schema_evolution/code_gen/CMakeLists.txt +++ b/tests/schema_evolution/code_gen/CMakeLists.txt @@ -62,3 +62,49 @@ endif() # ADD_SCHEMA_EVOLUTION_TEST(components_rename_member WITH_EVOLUTION RNTUPLE NO_GENERATE_MODELS) # ADD_SCHEMA_EVOLUTION_TEST(multi_schema_components_rename_member OLD_MODELS old oldest WITH_EVOLUTION RNTUPLE NO_GENERATE_MODELS) # endif() + + +# The following are tests that check whether trying to read files that have a +# **newer** version of the datamodel than is available from the environment. +# First we have to write the file with a new version. We will simply use one of +# the check files of the schema evolution checks above +add_executable(write_file_newer_model components_new_member/check.cpp) +target_link_libraries(write_file_newer_model PRIVATE + components_new_member_newModel + podio::podioIO +) +target_include_directories(write_file_newer_model PRIVATE + ${CMAKE_CURRENT_SOURCE_DIR} +) +target_compile_definitions(write_file_newer_model PRIVATE + PODIO_SCHEMA_EVOLUTION_TEST_WRITE + TEST_CASE="write_file_newer_model" +) +# Write a file with a newer version +add_test(NAME write_file_newer_model COMMAND write_file_newer_model) +set_property(TEST write_file_newer_model + PROPERTY + ENVIRONMENT + ROOT_LIBRARY_PATH=${CMAKE_CURRENT_BINARY_DIR}/components_new_member/new_model + LD_LIBRARY_PATH=${PROJECT_BINARY_DIR}/src:$:$<$:$>:$ENV{LD_LIBRARY_PATH} + +) +set_tests_properties(write_file_newer_model + PROPERTIES + WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/components_new_member + FIXTURES_SETUP write_file_newer_model_setup +) +# Try to read back this file with an older version +add_test(NAME read_file_older_env_edm_version COMMAND podio-dump-tool write_file_newer_model.root) +set_property(TEST read_file_older_env_edm_version + PROPERTY + ENVIRONMENT + ROOT_LIBRARY_PATH=${CMAKE_CURRENT_BINARY_DIR}/components_new_member/old_model + LD_LIBRARY_PATH=${PROJECT_BINARY_DIR}/src:$:$<$:$>:$ENV{LD_LIBRARY_PATH} +) +set_tests_properties(read_file_older_env_edm_version + PROPERTIES + WORKING_DIRECTORY ${CMAKE_CURRENT_BINARY_DIR}/components_new_member + FIXTURES_REQUIRED write_file_newer_model_setup + PASS_REGULAR_EXPRESSION "WARNING: EDM 'datamodel' exists in file with schema version 2 but podio loaded schema version 1 from the environment" +) From 7005b5b747a12f5e98ebd19372bc01a00314d4c3 Mon Sep 17 00:00:00 2001 From: Thomas Madlener Date: Mon, 29 Sep 2025 13:43:06 +0200 Subject: [PATCH 7/8] Homogenize behavior of TTree and RNTuple reader --- src/ROOTReader.cc | 2 ++ 1 file changed, 2 insertions(+) diff --git a/src/ROOTReader.cc b/src/ROOTReader.cc index 8d4f16633..59ae2c2ec 100644 --- a/src/ROOTReader.cc +++ b/src/ROOTReader.cc @@ -160,6 +160,8 @@ std::optional ROOTReader::getCollectionBuffers(ROO auto maybeBuffers = bufferFactory.createBuffers(collType, schemaVersion, isSubsetColl); if (!maybeBuffers) { + std::cerr << "WARNING: Buffers couldn't be created for collection " << name << " of type " << collType + << " and schema version " << schemaVersion << std::endl; return std::nullopt; } From 3987933342221812d89b0d9346150e1bcdb476d9 Mon Sep 17 00:00:00 2001 From: Thomas Madlener Date: Mon, 29 Sep 2025 20:57:24 +0200 Subject: [PATCH 8/8] Do not access optional if it's empty --- src/ioUtils.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/ioUtils.h b/src/ioUtils.h index e9f50ec5e..f97f2971a 100644 --- a/src/ioUtils.h +++ b/src/ioUtils.h @@ -4,7 +4,6 @@ #include "podio/DatamodelRegistry.h" #include "podio/utilities/DatamodelRegistryIOHelpers.h" -#include #include #include @@ -27,6 +26,7 @@ inline std::vector checkEDMVersionsReadable(const podio::DatamodelD if (!envSchemaVersion) { warnings.emplace_back("EDM '" + edmName + "' exists in file but does not seem to be loaded from the environment by podio"); + continue; } if (envSchemaVersion.value() < fileSchemaVersion) { warnings.emplace_back("EDM '" + edmName + "' exists in file with schema version " +