From 810bd3ef4b6594c3e6f4f6da7f05a235753ef839 Mon Sep 17 00:00:00 2001 From: Jonathan Percival Date: Wed, 18 Dec 2024 14:53:37 -0700 Subject: [PATCH 1/2] Fixes for Union --- .../cqf/cql/engine/execution/EvaluationVisitor.java | 2 +- .../opencds/cqf/cql/engine/execution/TestUnion.java | 10 +++++++++- .../opencds/cqf/cql/engine/execution/TestUnion.cql | 13 ++++++++++++- 3 files changed, 22 insertions(+), 3 deletions(-) diff --git a/Src/java/engine/src/main/java/org/opencds/cqf/cql/engine/execution/EvaluationVisitor.java b/Src/java/engine/src/main/java/org/opencds/cqf/cql/engine/execution/EvaluationVisitor.java index acf25dc0d..62f79b269 100644 --- a/Src/java/engine/src/main/java/org/opencds/cqf/cql/engine/execution/EvaluationVisitor.java +++ b/Src/java/engine/src/main/java/org/opencds/cqf/cql/engine/execution/EvaluationVisitor.java @@ -187,7 +187,7 @@ public Object visitUnion(Union elm, State state) { (org.opencds.cqf.cql.engine.runtime.Interval) rightResult, state); } else { - return UnionEvaluator.union(left, right, state); + return UnionEvaluator.union(leftResult, rightResult, state); } } diff --git a/Src/java/engine/src/test/java/org/opencds/cqf/cql/engine/execution/TestUnion.java b/Src/java/engine/src/test/java/org/opencds/cqf/cql/engine/execution/TestUnion.java index c24361bdb..ba6694621 100644 --- a/Src/java/engine/src/test/java/org/opencds/cqf/cql/engine/execution/TestUnion.java +++ b/Src/java/engine/src/test/java/org/opencds/cqf/cql/engine/execution/TestUnion.java @@ -2,6 +2,7 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; +import static org.junit.jupiter.api.Assertions.assertNull; import static org.junit.jupiter.api.Assertions.assertTrue; import java.util.List; @@ -12,10 +13,17 @@ class TestUnion extends CqlTestBase { @Test void union() { var results = engine.evaluate(toElmIdentifier("TestUnion")); - var value = results.forExpression("NullAndNull").value(); + + var value = results.forExpression("NullAndNullList").value(); assertNotNull(value); assertTrue(((List) value).isEmpty()); + value = results.forExpression("NullAndNullInterval").value(); + assertNull(value); + + value = results.forExpression("NullAndNullUntyped").value(); + assertNull(value); + value = results.forExpression("NullAndEmpty").value(); assertNotNull(value); assertTrue(((List) value).isEmpty()); diff --git a/Src/java/engine/src/test/resources/org/opencds/cqf/cql/engine/execution/TestUnion.cql b/Src/java/engine/src/test/resources/org/opencds/cqf/cql/engine/execution/TestUnion.cql index b4126287d..a52afcab5 100644 --- a/Src/java/engine/src/test/resources/org/opencds/cqf/cql/engine/execution/TestUnion.cql +++ b/Src/java/engine/src/test/resources/org/opencds/cqf/cql/engine/execution/TestUnion.cql @@ -1,9 +1,20 @@ library TestUnion // expect {} -define "NullAndNull": +define "NullAndNullList": null as List union null as List +// expect null +define "NullAndNullInterval": + null as Interval union null as Interval + +// expect null +// Based on the CQL conversion precedence rules, +// the compiler _should have_ inferred this as Intervals +// and not Lists. +define "NullAndNullUntyped": + null union null + // expect {} define "NullAndEmpty": null union {} From 3b552705cc3ee1a8860109775f5ccd641bc737d7 Mon Sep 17 00:00:00 2001 From: Jonathan Percival Date: Wed, 18 Dec 2024 21:19:15 -0700 Subject: [PATCH 2/2] More tests --- .../fhir/data/FhirExecutionTestBase.java | 5 +- .../cqf/cql/engine/fhir/data/Issue1441.java | 64 +++++++++++++++++++ .../cql/engine/fhir/data/TestFHIRHelpers.java | 37 ++++++++++- .../cqf/cql/engine/fhir/data/Issue1441.cql | 16 +++++ 4 files changed, 116 insertions(+), 6 deletions(-) create mode 100644 Src/java/engine-fhir/src/test/java/org/opencds/cqf/cql/engine/fhir/data/Issue1441.java create mode 100644 Src/java/engine-fhir/src/test/resources/org/opencds/cqf/cql/engine/fhir/data/Issue1441.cql diff --git a/Src/java/engine-fhir/src/test/java/org/opencds/cqf/cql/engine/fhir/data/FhirExecutionTestBase.java b/Src/java/engine-fhir/src/test/java/org/opencds/cqf/cql/engine/fhir/data/FhirExecutionTestBase.java index 39401d3fd..d4395e9ea 100644 --- a/Src/java/engine-fhir/src/test/java/org/opencds/cqf/cql/engine/fhir/data/FhirExecutionTestBase.java +++ b/Src/java/engine-fhir/src/test/java/org/opencds/cqf/cql/engine/fhir/data/FhirExecutionTestBase.java @@ -83,10 +83,7 @@ public static void setup() { r4Provider = new CompositeDataProvider(r4ModelResolver, r4RetrieveProvider); modelManager = new ModelManager(); - var compilerOptions = new CqlCompilerOptions( - CqlCompilerException.ErrorSeverity.Info, - LibraryBuilder.SignatureLevel.All, - CqlCompilerOptions.Options.EnableDateRangeOptimization); + var compilerOptions = CqlCompilerOptions.defaultOptions(); libraryManager = new LibraryManager(modelManager, compilerOptions); libraryManager.getLibrarySourceLoader().clearProviders(); libraryManager.getLibrarySourceLoader().registerProvider(new FhirLibrarySourceProvider()); diff --git a/Src/java/engine-fhir/src/test/java/org/opencds/cqf/cql/engine/fhir/data/Issue1441.java b/Src/java/engine-fhir/src/test/java/org/opencds/cqf/cql/engine/fhir/data/Issue1441.java new file mode 100644 index 000000000..effd52a18 --- /dev/null +++ b/Src/java/engine-fhir/src/test/java/org/opencds/cqf/cql/engine/fhir/data/Issue1441.java @@ -0,0 +1,64 @@ +package org.opencds.cqf.cql.engine.fhir.data; + +import static org.junit.jupiter.api.Assertions.assertIterableEquals; + +import java.util.Collections; +import org.hl7.fhir.r4.model.Encounter; +import org.hl7.fhir.r4.model.Patient; +import org.hl7.fhir.r4.model.Procedure; +import org.junit.jupiter.api.Test; +import org.opencds.cqf.cql.engine.data.CompositeDataProvider; +import org.opencds.cqf.cql.engine.retrieve.RetrieveProvider; +import org.opencds.cqf.cql.engine.runtime.Code; +import org.opencds.cqf.cql.engine.runtime.Interval; + +// https://github.com/cqframework/clinical_quality_language/issues/1441 +// unions without aliases are not working +class Issue1441 extends FhirExecutionTestBase { + + @Test + void unionsWithoutAliasesAreTheSameAsUnionsWithAliases() { + var patient = new Patient().setId("123"); + var observation = new Encounter().setId("456"); + var procedure = new Procedure().setId("789"); + + var r = new RetrieveProvider() { + @Override + public Iterable retrieve( + String context, + String contextPath, + Object contextValue, + String dataType, + String templateId, + String codePath, + Iterable codes, + String valueSet, + String datePath, + String dateLowPath, + String dateHighPath, + Interval dateRange) { + + switch (dataType) { + case "Patient": + return Collections.singletonList(patient); + case "Observation": + return Collections.singletonList(observation); + case "Procedure": + return Collections.singletonList(procedure); + default: + return Collections.emptyList(); + } + } + }; + + var engine = getEngine(); + engine.getState() + .getEnvironment() + .registerDataProvider("http://hl7.org/fhir", new CompositeDataProvider(r4ModelResolver, r)); + var result = engine.evaluate("Issue1441"); + var x = (Iterable) result.forExpression("x").value(); + var y = (Iterable) result.forExpression("y").value(); + + assertIterableEquals(x, y); + } +} diff --git a/Src/java/engine-fhir/src/test/java/org/opencds/cqf/cql/engine/fhir/data/TestFHIRHelpers.java b/Src/java/engine-fhir/src/test/java/org/opencds/cqf/cql/engine/fhir/data/TestFHIRHelpers.java index e74b0dcec..ed88db13f 100644 --- a/Src/java/engine-fhir/src/test/java/org/opencds/cqf/cql/engine/fhir/data/TestFHIRHelpers.java +++ b/Src/java/engine-fhir/src/test/java/org/opencds/cqf/cql/engine/fhir/data/TestFHIRHelpers.java @@ -3,17 +3,50 @@ import static org.hamcrest.CoreMatchers.instanceOf; import static org.hamcrest.CoreMatchers.is; import static org.hamcrest.MatcherAssert.assertThat; +import static org.junit.jupiter.api.Assertions.assertThrows; +import org.cqframework.cql.cql2elm.CqlCompilerOptions; +import org.cqframework.cql.cql2elm.LibraryBuilder; +import org.cqframework.cql.cql2elm.LibraryManager; +import org.cqframework.cql.cql2elm.ModelManager; +import org.cqframework.cql.cql2elm.quick.FhirLibrarySourceProvider; import org.junit.jupiter.api.Test; +import org.opencds.cqf.cql.engine.exception.CqlException; import org.opencds.cqf.cql.engine.execution.CqlEngine; +import org.opencds.cqf.cql.engine.execution.Environment; class TestFHIRHelpers extends FhirExecutionTestBase { @Test - void test() { + void testWithUnambiguousCompilerOptions() { + // Test with non-ambiguous compiler options. Should pass. + var compilerOptions = CqlCompilerOptions.defaultOptions(); + compilerOptions.setSignatureLevel(LibraryBuilder.SignatureLevel.Overloads); + var modelManager = new ModelManager(); + var libraryManager = new LibraryManager(modelManager, compilerOptions); + libraryManager.getLibrarySourceLoader().clearProviders(); + libraryManager.getLibrarySourceLoader().registerProvider(new FhirLibrarySourceProvider()); + libraryManager.getLibrarySourceLoader().registerProvider(new TestLibrarySourceProvider()); + + var engine = new CqlEngine(new Environment(libraryManager)); + engine.getEnvironment().registerDataProvider("http://hl7.org/fhir", r4Provider); + + runTest(engine); + } - CqlEngine engine = getEngine(); + @Test + void standardCompilerOptions() { + // Test with standard compiler options. Should throw an Exception as of the + // time this test is written because the default compiler options produce + // ambiguous ELM output. This test is intended to fail, and if we change the + // compiler options to be non-ambiguous, this test should be updated to expect + // a different (presumably passing) result. + var engine = getEngine(); engine.getEnvironment().registerDataProvider("http://hl7.org/fhir", r4Provider); + assertThrows(CqlException.class, () -> runTest(engine)); + } + + void runTest(CqlEngine engine) { var results = engine.evaluate(library.getIdentifier()); // Primitives diff --git a/Src/java/engine-fhir/src/test/resources/org/opencds/cqf/cql/engine/fhir/data/Issue1441.cql b/Src/java/engine-fhir/src/test/resources/org/opencds/cqf/cql/engine/fhir/data/Issue1441.cql new file mode 100644 index 000000000..5909bb196 --- /dev/null +++ b/Src/java/engine-fhir/src/test/resources/org/opencds/cqf/cql/engine/fhir/data/Issue1441.cql @@ -0,0 +1,16 @@ +library "Issue1441" version '1' + +// https://github.com/cqframework/clinical_quality_language/issues/1441 +// Unions of unaliased retrieves not working + +using FHIR version '4.0.1' + +context Patient + +define x: + [Observation] a + union [Procedure] b + +define y: + [Observation] + union [Procedure] \ No newline at end of file