nopayloaddb

nopayloaddb is the HEP Software Foundation’s reference conditions database, maintained at Brookhaven and used in the sPHENIX and Belle II ecosystems. It answers one question at scale: which calibration payload was valid at this point in a run?

Project

HEP Software Foundation reference conditions database

My role

External contributor

Merged

Fix comb_iov calculation in PayloadIOVAttachAPIView

Shipped in

v5.0.0 and v5.1.0

Repository

BNLNPPS/nopayloaddb

BNLNPPS/nopayloaddbHEP Software Foundation reference conditions database. Django REST Framework over PostgreSQL.Python

The problem

An interval of validity is a two-level integer pair: a major component, typically the run, and a minor one, typically the sub-run. This schema represents their ordering with a denormalised key. comb_iov collapses the pair into a single DECIMAL(38,19): major plus minor divided by ten to the nineteenth, so the minor component occupies the fractional part, and a covering index on it turns the lookup into one range scan ordered descending, limit one.

That representation comes with an obligation: every code path that changes a start interval must recompute the derived key. Miss one, and the index is quietly wrong.

What I found

One was missed. In the branch of the attach endpoint that adjusts a neighbouring interval when a newly attached one overlaps it, the recomputation read the major component twice instead of reading major and then minor.

The error occurs when an existing interval is adjusted during an attach and its minor component differs from its major component. The arithmetic still produces a valid decimal, so it need not raise an exception. Its fractional part is wrong, and a validity lookup can select a different payload than it should.

I opened an issue first, with a table showing the formula was correct at four other call sites and wrong at one, naming the two views whose queries depend on that ordering. Then the patch: one file, one line added, one removed. The fix is included in v5.0.0 and v5.1.0.

Hard parts

Proving a one-line change is the right one. A single-character difference in an unfamiliar codebase is indistinguishable from a misunderstanding. What made it reviewable was the audit around it – every place the same expression appears, which of them are correct, and why this branch is the exception – so the maintainer could check the claim without reconstructing it.

Working with an external test boundary. In the version I inspected, correctness is checked by standing the API up in Docker and running an external C++ client’s test suite against it. That catches behavior a client exercises, but a passing client run does not establish that every ordering-key update is correct. Comparing the expressions at each call site exposed the inconsistency.