feat: Support FHIRPath union() and combine() combining functions#2587
Open
piotrszul wants to merge 4 commits intorelease/9.6.0from
Open
feat: Support FHIRPath union() and combine() combining functions#2587piotrszul wants to merge 4 commits intorelease/9.6.0from
piotrszul wants to merge 4 commits intorelease/9.6.0from
Conversation
Closes #2384. Implements the FHIRPath combining functions from the specification's §6.5 Combining section as parser-level desugarings into the existing `|` operator AST (for `union`) and a new `CombineOperator` (for `combine`). Desugaring is the only approach that preserves the spec's `name.select(use.union(given)) ≡ name.select(use | given)` equivalence, because routing the argument through the standard function invocation path would lose the surrounding iteration focus. A shared `CombiningLogic` helper holds the array-level merge primitives used by both `UnionOperator` and `CombineOperator`. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
Reuse stateless UnionOperator and CombineOperator instances via a static map instead of allocating them on every parse of `union()` / `combine()`. Remove javadoc paragraphs that re-explained the parser desugaring in each operator, since that rationale lives with the parser where the rewrite happens. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
piotrszul
added
new feature
Reference #2588 from the repeat()-after-combine() exclusion so the latent Quantity struct shape mismatch is tracked alongside the existing feature exclusions that follow the same id format. Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
|
Quantity literals built by QuantityEncoding.encodeLiteral used lit(null) for absent fields, producing Spark NullType (VOID) in the struct schema. to_variant_object() rejects VOID fields, breaking repeat($this) over any Quantity literal collection via variantTransformTree. Cast the struct to dataType() after construction and fix FlexiDecimalSupport.toLiteral to return a typed null. The (3 'min').combine(180 seconds).repeat($this) exclusion is removed (now passes); the (1 year).combine (12 months) case is re-classified as wontfix (equality semantics, not VOID). Closes #2588 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
union(other)andcombine(other)functions from §6.5 Combining.EvalOperatorASTs so thatx.union(y)is strictly equivalent tox | y(including inside iteration contexts such asname.select(use.union(given))), andx.combine(y)reaches a new peerCombineOperatorthat merges without deduplicating.CombiningLogichelper holds the array-level merge primitives (Decimal normalization, comparator-aware dedup/concat) used by bothUnionOperatorandCombineOperator.Why parser desugaring
The spec explicitly declares
x.union(y)to be synonymous withx | y, and the worked examplename.select(use.union(given)) ≡ name.select(use | given)requires thegivenargument to resolve against the current iteration focus. Routing the argument through Pathling's standard function invocation path would evaluategivenagainst%context(the outer root focus), losing the per-name iteration. Desugaringx.union(y)andx.combine(y)at parse time into operator-form ASTs gives this equivalence by construction, without refactoringComposite.apply/EvalFunction/FunctionParameterResolver.The full design rationale lives in
openspec/changes/archive/2026-04-11-add-combining-functions/design.md.Test plan
mvn test -pl fhirpath -Dtest=CombiningFunctionsDslTest— 78 new assertions covering the type matrix, duplicate-preservation,union() ≡ \|equivalence across 9 types (Boolean, Integer, Decimal, String, Date, DateTime, Time, Quantity, Coding), iteration-context pin-downs insideselect, incompatible-type errors, and arity errorsmvn test -pl fhirpath -Dtest=CombiningOperatorsDslTest— 188/188 pass (refactor behaviour-preserving for\|)mvn test -pl fhirpath -Dtest=YamlReferenceImplTest— 1821 run, 0 failures, 979 skipped; many previously-failing reference tests for.combine(...)now passmvn test -pl fhirpath— no new failures introducedfhirpath-js/config.yamlexclusions reviewed: extended#2398(polymorphic) and#437(primitive-with-extensions) to cover the new function forms; added an entry for a latent Quantity +repeat()canonicalization bug thatcombine()now surfaces (tracked as a separate follow-up)Documentation
site/docs/fhirpath/index.mdgains a new "Combining functions" section listingunion()andcombine(), and the|operator row cross-references theunion()function.🤖 Generated with Claude Code