[ntuple] Fix attribute writing with RMiniFile - #23075
Conversation
WriteTFileNTupleKey was doing 2 mistakes: - incorrectly writing the TFile's epilogue even for Hidden clones (which led to files containing 1 extra StreamerInfo/FreeList/KeysList per attribute set); - writing the wrong anchor location for attribute RNTuples. This was not caught by tests because we never actually added tests for attribute writing through RMiniFile.
Test Results 23 files 23 suites 3d 15h 26m 18s ⏱️ For more details on these failures, see this check. Results for commit a055390. ♻️ This comment has been updated with latest results. |
jblomer
left a comment
jblomer
left a comment
There was a problem hiding this comment.
Thanks! Perhaps check or add a comment to prevent the use of the minifile with only hidden ntuples (I think we need a main one to produce a correct file).
| // The first 3 RBlobs we're gonna find are, in order: the main RNTuple header, MyAttrSet1's header | ||
| // and MyAttrSet2's header. | ||
| const auto headerSeek = key.fSeekKey + key.fKeyLen; |
There was a problem hiding this comment.
That looks a bit brittle to me in the long term. Can we identify the header blobs through their offsets in the corresponding anchors?
This should be already mandated by the current public API (you cannot Recreate a RMiniFileWriter which is hidden through public methods: only Append to one or Clone one); though it doesn't hurt to point that out (where is it best to do it? In the private ctor perhaps?) |
8aafc73 to
a055390
Compare
Private constructor sounds good. |
2 participants
WriteTFileNTupleKey was making 2 mistakes:
(which led to files containing 1 extra StreamerInfo/FreeList/KeysList
per attribute set);
This was not caught by tests because we never actually added tests for
attribute writing through RMiniFile.