Skip to content

Conversation

jblomer
Copy link
Contributor

@jblomer jblomer commented Sep 22, 2025

In contrast to other automatic evolution mechanisms, this one cannot use ReconcileOnDiskField() because for proper support the type of the field itself must change. In principle, we could stuff all the logic into RRecField, but it would be quite messy. We also discussed adding a "virtual column" for the fixed-size indexes; this doesn't work, however, because the index column stores offsets and not vector lengths, and it is also used to translate between global and local indexing.

Thus, this PR adds the infrastructure to replace fields of just created field hierarchies (models). This is legit as long as the fields weren't exposed to the user yet, where the user, e.g., could have already created entries. As a result, the automatic evolution from fixed-size to RVec (and later std::vector) won't work with imposed models. But it does work for views and models reconstructed from disk, which are the most common use cases.

The infrastructure will also be used to switch between streamer field and class field.

Copy link

github-actions bot commented Sep 22, 2025

Test Results

    21 files      21 suites   3d 13h 50m 30s ⏱️
 3 678 tests  3 678 ✅ 0 💤 0 ❌
75 418 runs  75 418 ✅ 0 💤 0 ❌

Results for commit 90b5d4b.

♻️ This comment has been updated with latest results.

@jblomer jblomer force-pushed the ntuple-evolution-fixed-arrays branch 6 times, most recently from a555c89 to 7de82c0 Compare September 23, 2025 09:10
@jblomer jblomer force-pushed the ntuple-evolution-fixed-arrays branch 3 times, most recently from f0943cb to 0bccc9d Compare September 23, 2025 10:55
@jblomer jblomer requested a review from silverweed September 24, 2025 06:31
Copy link
Contributor

@silverweed silverweed left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM but I would wait for a second approval.
Also see the warnings about typeid in the test

@jblomer jblomer force-pushed the ntuple-evolution-fixed-arrays branch from 0bccc9d to 4947ca8 Compare September 24, 2025 09:04
@jblomer jblomer force-pushed the ntuple-evolution-fixed-arrays branch from 4947ca8 to d737905 Compare September 24, 2025 19:53
We can safely apply field substitutions in a field hierarchy if we
control the time between field creation and connecting the field to the
page source, i.e. if we can ensure no field values have been created
in-between. This is the case for

  - the RNTupleReader
  - views
  - the RNTupleProcessor
  - RDF
  - source member fields used in schema evolution

Only imposed models created by users cannot apply field substitutions.
This field used to expect a pre-contructed RVec of a certain size, which
we can ensure in RDF but not elsewhere. Factor out the logic to resize
RVecs in a type-erased way and use it in both this field and the
RRVecField.
@jblomer jblomer force-pushed the ntuple-evolution-fixed-arrays branch from d737905 to 90b5d4b Compare September 25, 2025 02:55
@jblomer jblomer merged commit 0ae05af into root-project:master Sep 25, 2025
26 checks passed
@jblomer jblomer deleted the ntuple-evolution-fixed-arrays branch September 25, 2025 13:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants