compute_logical_data_version joins components with no delimiter, so a compensating upstream change leaves the downstream asset reported FRESH #34184

Summary

compute_logical_data_version concatenates the code version and the upstream data versions with no delimiter and no length prefix before hashing. The map from those components to the hashed byte string is therefore not injective: two genuinely different upstream states hash to the same downstream data version whenever a character migrates across a component boundary.

The downstream asset is then reported FRESH with an empty stale-cause list while its inputs have changed, so a staleness-driven rebuild does not happen.

Where

python_modules/dagster/dagster/_core/definitions/data_version.py:

all_inputs = (code_version, *(v.value for v in ordered_input_versions))

hash_sig = sha256()
hash_sig.update(bytearray("".join(all_inputs), "utf8"))
return DataVersion(hash_sig.hexdigest())

Called from _get_output_asset_events in _core/execution/plan/execute_step.py, with the result stamped on the materialization as dagster/data_version and read back by the staleness resolver.

This is only reachable because upstream data versions are not always framework-computed fixed-width digests. When a version is user-supplied — Output(value, data_version=DataVersion(...)), or the documented @observable_source_asset idiom of returning an mtime, an etag, a row count or a cursor — the components are variable-width and the boundary is free to move. code_version is likewise an arbitrary user string.

Reproduction

Pure function level:

('x','yz') -> 99c37d36309112cb021cab58df5ac74a62945f3306a0cd72671e1087881d7a45
('xy','z') -> 99c37d36309112cb021cab58df5ac74a62945f3306a0cd72671e1087881d7a45
COLLISION  : True
code_version can absorb an input too: True   ('c1x' + ('y','z')  vs  'c1' + ('xy','z'))

End to end, A and B upstream of C, C upstream of D:

--- POSITIVE: compensating upstream change
  upstream (A,B) run1 = ('x', 'yz')
  upstream (A,B) run2 = ('xy', 'z')
  C data_version run1 = 99c37d36…
  C data_version run2 = 99c37d36…
  identical            = True
  D stale status       = (StaleStatus.FRESH, [])

C really did recompute over different upstream data. D, which consumes C, is reported FRESH with an empty cause list.

Through the ordinary @observable_source_asset path with plausible values — a counter "1" and a zero-padded id "005" becoming "10" and "05":

observable src versions run1 = ('1','005'), run2 = ('10','05')
C data_version run1 = 3d6a2326…   run2 = 3d6a2326…   identical = True
D status = StaleStatus.FRESH []

Negative control

Same harness, same assets, one upstream changed in a way that does not shift the boundary (('x','yz') → ('x2','yz')):

  C data_version run1 = 99c37d36…
  C data_version run2 = 0c8ff164…
  identical            = False
  D stale status       = (StaleStatus.STALE,
      [StaleCause(reason='has a new dependency data version', dependency=AssetKey(['C']),
         children=[StaleCause(key=AssetKey(['C']), reason='has a new data version')])])

The mechanism detects the change correctly and produces a precise stale cause. The defect is the collision, not the use of the API.

The silence

Both runs return success = True. No exception — the collision is never compared against anything. warnings.catch_warnings(record=True) around the colliding run captured nothing mentioning collision or data version; the only warnings are the generic beta notices for data_version and observable_source_asset. The downstream stale-cause list is empty, so the UI and scheduler see a positive assertion of freshness rather than an absence of information.

Scope, honestly

This is a conditional injectivity break, not an unconditional one.

If every upstream data version is framework-computed (a 64-character sha256 hexdigest) and code_version is stable across the comparison, the components are fixed-width and no collision is reachable. That is the common case.

The collision needs variable-length user-supplied versions and a compensating change across an adjacent boundary. That is unlikely on any given run, though trivially reachable by construction and increasingly plausible with short structured versions — counters, zero-padded ids, concatenated cursors, f"{table}:{n}" strings — across many assets and many runs over time.

So: a real correctness defect with a one-character fix ("\0".join, or hashing each component with a separator or length prefix), characterised as low-probability-per-run rather than as something users hit daily. Nothing in the API constrains user-supplied DataVersion strings, and no comment or documented width assumption accompanies the concatenation.

Two adjacent things that are clean

• Re-execution pairing. I attacked the claim that "re-execute-from-failure pairs each stored step to the step that produced it" by failing a job, swapping what two identically-named ops return, and re-executing from failure. Pairing is by step key, never by position, and a rename or removal makes the key absent rather than sliding into a neighbour's slot. Deliberate and correctly implemented.
• Dynamic mapping keys. A duplicate mapping_key from the same op raises loudly. This is the framework getting the same class of problem right.

Not tested

Partitioned assets and the partitioned staleness path; a full auto-materialize sensor tick (I demonstrated the wrong answer at the stale-status resolver, which the UI and sensor consume, and infer the skipped rebuild from FRESH plus an empty cause list); code_version-driven asset checks and freshness policies; multi-process or cross-code-location re-execution; asset key collisions across code locations.

I have not checked whether this was raised before; a pointer to an existing issue is welcome and I will close this in favour of it.

Version

Source at 76eed340c6b84517d91a86163c461c022ebef8d8; end-to-end reproduction against released dagster 1.12.1, Python 3.12, DagsterInstance.local_temp. I diffed the function between the two: identical apart from cosmetic refactoring.