[ntuple] automatic evolution of nullable fields #19966
Conversation
Test Results 21 files 21 suites 3d 16h 56m 11s ⏱️ Results for commit edc8d6d. ♻️ This comment has been updated with latest results. |
| EnsureMatchingOnDiskField(fieldDesc, kDiffTypeName); | ||
| EnsureMatchingTypePrefix(fieldDesc, prefixes); | ||
| } catch (const RException &) { | ||
| fSubfields[0]->SetOnDiskId(GetOnDiskId()); |
There was a problem hiding this comment.
This looks a bit sketchy to me: since we're using RException for everything, how can we know that it's not a fatal exception, possibly coming from a deep call inside the Ensure methods?
Unrelated, for the Ensure methods I would adopt the same approach as e.g. VerifyXxHash3, where the call returns a RResult rather than throwing an exception directly (of course in a separate PR)
There was a problem hiding this comment.
Also similar question to #19937 (review): Is unconditionally delegating to the item field maybe confusing for the error message?
There was a problem hiding this comment.
Unrelated, for the Ensure methods I would adopt the same approach as e.g.
VerifyXxHash3, where the call returns aRResultrather than throwing an exception directly (of course in a separate PR)
Makes sense. See #19971
jblomer
force-pushed
the
ntuple-evolution-nullable
branch
from
4808ccf to
edc8d6d
Compare
Conversion between
std::unique_ptr<T>andstd::optional<T>as well asT-->std::unique_ptr|std::optional<T>.