Form: Return a Token from registerWrite

form/core/token.cpp

@@ -6,12 +6,26 @@
 
 using namespace form::detail::experimental;
 
-/// Constructor with initialization
-Token::Token(std::string fileName, std::string containerName, technology::Id technology, int id) :
+/// Placement-only constructor; id is left unset
+Token::Token(std::string fileName, std::string containerName, technology::Id technology) :
   m_technology(technology),
   m_fileName(std::move(fileName)),
   m_containerName(std::move(containerName)),
-  m_id(id)
+  m_id(0),
+  m_hasId(false)
+{
+}
+
+/// Fully-specified constructor; id is set
+Token::Token(std::string fileName,
+             std::string containerName,
+             technology::Id technology,
+             std::uint64_t id) :
+  m_technology(technology),
+  m_fileName(std::move(fileName)),
+  m_containerName(std::move(containerName)),
+  m_id(id),
+  m_hasId(true)
 {
 }
 
@@ -21,6 +35,7 @@ std::string const& Token::fileName() const { return m_fileName; }
 std::string const& Token::containerName() const { return m_containerName; }
 /// Access technology type
 form::technology::Id Token::technology() const { return m_technology; }
-/// Set technology type
-/// Access identifier/entry number
-int Token::id() const { return m_id; }
+/// Access identifier/entry number (0-based row)
+std::uint64_t Token::id() const { return m_id; }
+/// Whether an id has been set on this token
+bool Token::hasId() const { return m_hasId; }

form/core/token.hpp

@@ -5,6 +5,7 @@
 
 #include "core/technology.hpp"
 
+#include <cstdint>
 #include <string>
 
 /* @class Token
@@ -13,11 +14,17 @@
 namespace form::detail::experimental {
   class Token {
   public:
-    /// Default constructor; delegates to the named constructor so the -1 sentinel for id is defined once
+    /// Default constructor; a token with no id set (delegates to the placement-only constructor)
     Token() : Token("", "", {}) {}
 
-    /// Named constructor; id defaults to -1 as a "not set" sentinel
-    Token(std::string fileName, std::string containerName, technology::Id technology, int id = -1);
+    /// Placement-only constructor; leaves the id unset (hasId() == false)
+    Token(std::string fileName, std::string containerName, technology::Id technology);
+
+    /// Fully-specified constructor; sets the 0-based row/entry id (hasId() == true)
+    Token(std::string fileName,
+          std::string containerName,
+          technology::Id technology,
+          std::uint64_t id);
 
     /// Access file name
     std::string const& fileName() const;
@@ -26,8 +33,10 @@ namespace form::detail::experimental {
     /// Access technology type
     technology::Id technology() const;
 
-    /// Access identifier/entry number
-    int id() const;
+    /// Access identifier/entry number (0-based row). Only meaningful when hasId() is true.
+    std::uint64_t id() const;
+    /// Whether an id has been set on this token
+    bool hasId() const;
 
   private:
     /// Technology identifier
@@ -36,8 +45,10 @@ namespace form::detail::experimental {
     std::string m_fileName;
     /// Container name
     std::string m_containerName;
-    /// Identifier/entry number
-    int m_id;
+    /// Identifier/entry number (0-based row)
+    std::uint64_t m_id;
+    /// Whether m_id holds a valid, set value
+    bool m_hasId;
   };
 } // namespace form::detail::experimental
 #endif // FORM_CORE_TOKEN_HPP

form/persistence/ipersistence_writer.hpp

@@ -3,6 +3,8 @@
 #ifndef FORM_PERSISTENCE_IPERSISTENCE_WRITER_HPP
 #define FORM_PERSISTENCE_IPERSISTENCE_WRITER_HPP
 
+#include "core/token.hpp"
+
 #include <map>
 #include <memory>
 #include <string>
@@ -27,10 +29,12 @@ namespace form::detail::experimental {
 
     virtual void createContainers(std::string const& creator,
                                   std::map<std::string, std::type_info const*> const& products) = 0;
-    virtual void registerWrite(std::string const& creator,
-                               std::string const& label,
-                               void const* data,
-                               std::type_info const& type) = 0;
+    // Write one product and return a Token locating it: placement plus 0-based row (entry) number
+    // Throws if backend isn't row-addressed, causing Token read lookup to fail
+    virtual Token registerWrite(std::string const& creator,
+                                std::string const& label,
+                                void const* data,
+                                std::type_info const& type) = 0;
     virtual void commitOutput(std::string const& creator, std::string const& id) = 0;
   };
 

form/persistence/persistence_writer.cpp

@@ -48,13 +48,21 @@ void PersistenceWriter::createContainers(
   m_store_writer->createContainers(containers, m_tech_settings);
 }
 
-void PersistenceWriter::registerWrite(std::string const& creator,
-                                      std::string const& label,
-                                      void const* data,
-                                      std::type_info const& type)
+Token PersistenceWriter::registerWrite(std::string const& creator,
+                                       std::string const& label,
+                                       void const* data,
+                                       std::type_info const& type)
 {
   std::unique_ptr<Placement> plcmnt = getPlacement(creator, label);
-  m_store_writer->fillContainer(*plcmnt, data, type);
+  std::uint64_t const row = m_store_writer->fillContainer(*plcmnt, data, type);
+  // A returned Token must locate a readable product: its row is the read-side navigation key.
+  // kInvalidRowId means backend does not address rows,so a product routed there could not be located on read, so throw here for such an unusable Token
+  if (row == kInvalidRowId) {
+    throw std::runtime_error("PersistenceWriter::registerWrite backend for product '" + label +
+                             "' from creator '" + creator + "' does not address rows; " +
+                             "cannot produce a Token locating the written product");
+  }
+  return Token{plcmnt->fileName(), plcmnt->containerName(), plcmnt->technology(), row};
 }
 
 void PersistenceWriter::commitOutput(std::string const& creator, std::string const& id)

form/persistence/persistence_writer.hpp

@@ -31,10 +31,10 @@ namespace form::detail::experimental {
 
     void createContainers(std::string const& creator,
                           std::map<std::string, std::type_info const*> const& products) override;
-    void registerWrite(std::string const& creator,
-                       std::string const& label,
-                       void const* data,
-                       std::type_info const& type) override;
+    Token registerWrite(std::string const& creator,
+                        std::string const& label,
+                        void const* data,
+                        std::type_info const& type) override;
     void commitOutput(std::string const& creator, std::string const& id) override;
 
   private:

form/root_storage/root_rfield_write_container.cpp

@@ -61,7 +61,7 @@ namespace form::detail::experimental {
     m_rntuple_parent = parentDerived;
   }
 
-  void ROOT_RField_Write_ContainerImp::fill(void const* data)
+  std::uint64_t ROOT_RField_Write_ContainerImp::fill(void const* data)
   {
     if (!m_rntuple_parent) {
       throw std::runtime_error(
@@ -79,6 +79,11 @@ namespace form::detail::experimental {
       m_rntuple_parent->m_entry = m_rntuple_parent->m_writer->CreateRawPtrWriteEntry();
     }
     m_rntuple_parent->m_entry->BindRawPtr(col_name(), data);
+
+    // Unlike a TBranch, an RNTuple entry is only written on commit();
+    // every field bound before that commit shares one entry.
+    // Return the 0-based index that pending entry will occupy (the current entry count).
+    return static_cast<std::uint64_t>(m_rntuple_parent->m_writer->GetNEntries());
   }
 
   void ROOT_RField_Write_ContainerImp::commit()

form/root_storage/root_rfield_write_container.hpp

@@ -23,7 +23,7 @@ namespace form::detail::experimental {
     void setFile(std::shared_ptr<IStorage_File> file) override;
     void setupWrite(std::type_info const& type) override;
     void setParent(std::shared_ptr<IStorage_Write_Container> const parent) override;
-    void fill(void const* data) override;
+    std::uint64_t fill(void const* data) override;
     void commit() override;
 
   private:

form/root_storage/root_rntuple_write_container.cpp

@@ -29,7 +29,7 @@ namespace form::detail::experimental {
     return;
   }
 
-  void ROOT_RNTuple_Write_ContainerImp::fill(void const* /*data*/)
+  std::uint64_t ROOT_RNTuple_Write_ContainerImp::fill(void const* /*data*/)
   {
     throw std::runtime_error("ROOT_RNTuple_Write_ContainerImp::fill not implemented");
   }

form/root_storage/root_rntuple_write_container.hpp

@@ -52,7 +52,7 @@ namespace form::detail::experimental {
 
     void setFile(std::shared_ptr<IStorage_File> file) override;
     void setupWrite(std::type_info const& type) override;
-    void fill(void const* data) override;
+    std::uint64_t fill(void const* data) override;
     void commit() override;
 
     //State shared by ROOT_RField_ContainerImps

form/root_storage/root_tbranch_write_container.cpp

@@ -94,7 +94,7 @@ void ROOT_TBranch_Write_ContainerImp::setupWrite(std::type_info const& type)
   }
 }
 
-void ROOT_TBranch_Write_ContainerImp::fill(void const* data)
+std::uint64_t ROOT_TBranch_Write_ContainerImp::fill(void const* data)
 {
   // NOTE: incoming parameter `data` is `const` due to the constraints on how we
   // expect users to interact with the data; however, ROOT's SetBranchAddress
@@ -112,8 +112,18 @@ void ROOT_TBranch_Write_ContainerImp::fill(void const* data)
   } else {
     m_branch->SetAddress(reinterpret_cast<void*>(&data));
   }
-  m_branch->Fill();
+  // TBranch::Fill() returns the number of bytes committed, or a negative value on a write error.
+  // ROOT increments entry count before a basket write can fail, so check return value first
+  Int_t const nbytes = m_branch->Fill();
   m_branch->ResetAddress();
+  if (nbytes < 0) {
+    throw std::runtime_error("ROOT_TBranch_Write_ContainerImp::fill TBranch::Fill() failed for " +
+                             col_name());
+  }
+
+  // 0-based entries: GetEntries() is the total count after this Fill(); row = count - 1.
+  // GetEntries() >= 1 here (Fill() succeeded), so the row is non-negative.
+  return static_cast<std::uint64_t>(m_branch->GetEntries() - 1);
 }
 
 void ROOT_TBranch_Write_ContainerImp::commit()

form/root_storage/root_tbranch_write_container.hpp

@@ -25,7 +25,7 @@ namespace form::detail::experimental {
     void setParent(std::shared_ptr<IStorage_Write_Container> parent) override;
 
     void setupWrite(std::type_info const& type = typeid(void)) override;
-    void fill(void const* data) override;
+    std::uint64_t fill(void const* data) override;
     void commit() override;
 
   private:

form/root_storage/root_ttree_write_container.cpp

@@ -45,7 +45,7 @@ void ROOT_TTree_Write_ContainerImp::setupWrite(std::type_info const& /* type*/)
   }
 }
 
-void ROOT_TTree_Write_ContainerImp::fill(void const* /* data*/)
+std::uint64_t ROOT_TTree_Write_ContainerImp::fill(void const* /* data*/)
 {
   throw std::runtime_error("ROOT_TTree_Write_ContainerImp::fill not implemented");
 }

form/root_storage/root_ttree_write_container.hpp

@@ -25,7 +25,7 @@ namespace form::detail::experimental {
 
     void setFile(std::shared_ptr<IStorage_File> file) override;
     void setupWrite(std::type_info const& type) override;
-    void fill(void const* data) override;
+    std::uint64_t fill(void const* data) override;
     void commit() override;
 
     TTree* getTTree();

form/storage/istorage.hpp

@@ -7,13 +7,19 @@
 #include "core/token.hpp"
 #include "form/config.hpp"
 
+#include <cstdint>
+#include <limits>
 #include <map>
 #include <memory>
 #include <string>
 #include <vector>
 
 namespace form::detail::experimental {
 
+  // Sentinel returned by the write chain when no addressable row was written
+  // (e.g. the generic no-op container). A real row is always < this value.
+  inline constexpr std::uint64_t kInvalidRowId = std::numeric_limits<std::uint64_t>::max();
+
   class IStorageReader {
   public:
     IStorageReader() = default;
@@ -41,9 +47,10 @@ namespace form::detail::experimental {
     virtual void createContainers(
       std::map<std::unique_ptr<Placement>, std::type_info const*> const& containers,
       form::experimental::config::tech_setting_config const& settings) = 0;
-    virtual void fillContainer(Placement const& plcmnt,
-                               void const* data,
-                               std::type_info const& type) = 0;
+    // Returns the 0-based row (entry) number written, or kInvalidRowId if no rows
+    virtual std::uint64_t fillContainer(Placement const& plcmnt,
+                                        void const* data,
+                                        std::type_info const& type) = 0;
     virtual void commitContainers(Placement const& plcmnt) = 0;
   };
 
@@ -67,7 +74,8 @@ namespace form::detail::experimental {
 
     virtual void setFile(std::shared_ptr<IStorage_File> file) = 0;
     virtual void setupWrite(std::type_info const& type = typeid(void)) = 0;
-    virtual void fill(void const* data) = 0;
+    // Returns the 0-based row (entry) number written, or kInvalidRowId if no rows
+    virtual std::uint64_t fill(void const* data) = 0;
     virtual void commit() = 0;
 
     virtual void setAttribute(std::string const& name, std::string const& value) = 0;

form/storage/storage_reader.cpp

@@ -388,5 +388,6 @@ void StorageReader::readContainer(Token const& token,
       cont->second->setAttribute(key, value);
     }
   }
-  cont->second->read(token.id(), data, type);
+  // TODO: Token::id() is a 64-bit row; the read container interface still takes an int entry. Narrow explicitly here (exact for all realistic row counts). Widening the read path to 64-bit is a follow-up PR.
+  cont->second->read(static_cast<int>(token.id()), data, type);
 }

form/storage/storage_write_container.cpp

@@ -18,7 +18,7 @@ void Storage_Write_Container::setFile(std::shared_ptr<IStorage_File> file) { m_f
 
 void Storage_Write_Container::setupWrite(std::type_info const& /* type*/) {}
 
-void Storage_Write_Container::fill(void const* /* data*/) {}
+std::uint64_t Storage_Write_Container::fill(void const* /* data*/) { return kInvalidRowId; }
 
 void Storage_Write_Container::commit() {}
 

form/storage/storage_write_container.hpp

@@ -20,7 +20,7 @@ namespace form::detail::experimental {
     void setFile(std::shared_ptr<IStorage_File> file) override;
 
     void setupWrite(std::type_info const& type = typeid(void)) override;
-    void fill(void const* data) override;
+    std::uint64_t fill(void const* data) override;
     void commit() override;
 
     void setAttribute(std::string const& name, std::string const& value) override;

form/storage/storage_writer.cpp

@@ -103,9 +103,9 @@ void StorageWriter::createContainers(
   }
 }
 
-void StorageWriter::fillContainer(Placement const& plcmnt,
-                                  void const* data,
-                                  std::type_info const& /* type*/)
+std::uint64_t StorageWriter::fillContainer(Placement const& plcmnt,
+                                           void const* data,
+                                           std::type_info const& /* type*/)
 {
   // Use file+container as composite key
   auto contKey = std::make_pair(plcmnt.fileName(), plcmnt.containerName());
@@ -115,7 +115,7 @@ void StorageWriter::fillContainer(Placement const& plcmnt,
     throw std::runtime_error("StorageWriter::fillContainer Container doesn't exist: " +
                              plcmnt.containerName());
   }
-  cont->second->fill(data);
+  return cont->second->fill(data);
 }
 
 void StorageWriter::commitContainers(Placement const& plcmnt)

form/storage/storage_writer.hpp

@@ -23,9 +23,9 @@ namespace form::detail::experimental {
     void createContainers(
       std::map<std::unique_ptr<Placement>, std::type_info const*> const& containers,
       form::experimental::config::tech_setting_config const& settings) override;
-    void fillContainer(Placement const& plcmnt,
-                       void const* data,
-                       std::type_info const& type) override;
+    std::uint64_t fillContainer(Placement const& plcmnt,
+                                void const* data,
+                                std::type_info const& type) override;
     void commitContainers(Placement const& plcmnt) override;
 
   private:

test/form/form_basics_test.cpp

@@ -37,8 +37,8 @@ TEST_CASE("Token default constructor", "[form]")
   CHECK(t.fileName().empty());
   CHECK(t.containerName().empty());
   CHECK(t.technology() == form::technology::Id{});
-  // Default-constructed token must carry the -1 sentinel for id
-  CHECK(t.id() == -1);
+  // Default-constructed token has no id set
+  CHECK_FALSE(t.hasId());
 }
 
 TEST_CASE("Token basics", "[form]")
@@ -47,7 +47,8 @@ TEST_CASE("Token basics", "[form]")
   CHECK(t.fileName() == "file.root");
   CHECK(t.containerName() == "container");
   CHECK(t.technology() == form::technology::ROOT_TTREE);
-  CHECK(t.id() == 42);
+  CHECK(t.hasId());
+  CHECK(t.id() == 42u);
 }
 
 TEST_CASE("technology::Id string conversions", "[form]")