Support null-safe variable references in expressions#181
Conversation
Replace the abort-on-null behavior of {x} with SQL-style NULL propagation:
None flows through arithmetic and function calls (returning None) while
comparisons evaluate naturally, enabling case() branching on null values.
{x} and bare x are now equivalent.
Closes #180
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Updates the LinkML expression evaluator to use SQL-style NULL propagation so that None values flow through arithmetic and function calls instead of aborting evaluation, enabling case() fallback branches to execute as expected.
Changes:
- Removed the
{x}“abort-on-null” behavior and made{x}equivalent to barexvariable resolution. - Added NULL-propagating wrappers for arithmetic and unary operators, and for scalar functions.
- Updated evaluator and transformer tests to reflect the new semantics and to use a reliably failing expression for continue-on-error coverage.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
src/linkml_map/utils/eval_utils.py |
Implements NULL propagation in operators/functions and removes the UnsetValueError abort path. |
tests/test_utils/test_eval_utils.py |
Updates/extends tests to cover NULL propagation in arithmetic, function calls, and case() branching. |
tests/test_transformer/test_continue_on_error.py |
Adjusts error-handling tests to use division-by-zero now that undefined-variable arithmetic no longer fails. |
Wrap list functions (len, min, max) with null-safe guards, fix None handling in distributed function calls over lists, and fix CLI continue-on-error tests to use genuinely failing expressions. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Review: Approve — with one gap to address
What this changes (and why it matters)
The old {x} behavior was raise-on-None: if any {var} resolved to None, the entire expression threw UnsetValueError and eval_expr_with_mapping caught it and returned None. That sounds safe, but it silently swallowed case() fallback branches — even (True, FALLBACK) never fired when a preceding {var} was null. This PR replaces that with SQL-style NULL propagation: None flows through arithmetic, functions return None when given None, and comparisons work naturally (None == "1" is False), so case() fallback branches execute as intended.
Impact on our production patterns
Three distinct patterns in our YAML files:
-
== "value"comparisons with(True, None)fallback — no behavioral change.{phv} == "1"with phv=None → False → case falls through to(True, None)→ None. Same result as the old abort path. -
== "value"comparisons with(True, MEANINGFUL_VALUE)fallback (e.g.,cig_smok.yaml(True, "OMOP:45885135")) — behavior changes. Old: phv=None → abort → None. New: phv=None → None == "Yes" → False → falls through → "Unknown if ever smoked". This is the correct intended behavior — the(True, ...)fallback now actually fires. This is a win. -
Numeric guard comparisons (
bdy_hgt.yaml:case(({phv} <= 0, None), ...)) — this is the gap. Arithmetic operators (+,*, etc.) are now null-propagating, but<,<=,>,>=are not._coercingwraps them for numeric string coercion but doesn't guard againstNone. So{phv} <= 0with phv=None → None is returned by{phv}, thenoperator.le(None, 0)raisesTypeError. Old behavior: graceful None. New behavior: uncaught TypeError.
The <= 0 guard pattern (case(({phv} <= 0, None), (True, {phv} * ...))) is a common defensive idiom across height, weight, and similar files. With this PR, a null input that previously returned None cleanly will now throw. Depending on how dm-bip handles the TypeError (catch vs. propagate), this is either a logged null or a failed row.
Recommendation
_null_propagating should also be applied to ast.Lt, ast.LtE, ast.Gt, ast.GtE — not just arithmetic. Eq/NotEq are fine since Python handles None == x natively. That would close the gap and make the null propagation complete and consistent.
Approving because the core change is correct, the test suite is solid (348 tests, 7/7 CI), and the arithmetic + case() semantics are exactly what we need. The <= gap needs a follow-up fix before the numeric guard pattern is fully safe. If the dm-bip error handler treats TypeError the same as prior None (nullifying the slot), this is low-risk in practice — but the inconsistency should be addressed.
Summary
Closes #180
Replaces the abort-on-null behavior of
{x}with SQL-style NULL propagation. Both{x}and barexare now equivalent — None flows through arithmetic and function calls (returning None) while comparisons evaluate naturally, enablingcase()branching on null values.Before
After
What changed
_eval_set:{x}returns None instead of raisingUnsetValueErrorNone == "1"→FalsenativelyUnsetValueErrorclass and its try/except ineval_expr_with_mappingBehavioral change
Expressions with
{x}wherexis None now propagate None through operations instead of aborting the entire expression. The end result for simple arithmetic ({x} * 0.453) is the same (field gets None). The difference is thatcase()fallback branches now fire instead of being silently skipped.Test plan
🤖 Generated with Claude Code