Fix Intersect on intervals with nulls (#1381)

* Handle null ends of intersecting intervals * Restore skipped tests * Run spotless * Remove files with only formatting changes * Improve coverage --------- Co-authored-by: JP <[email protected]>
cqframework · Jul 11, 2024 · 179c762 · 179c762
1 parent db3fe10
commit 179c762
Show file tree

Hide file tree

Showing 3 changed files with 46 additions and 11 deletions.
diff --git a/...ava/engine/src/main/java/org/opencds/cqf/cql/engine/elm/executing/IntersectEvaluator.java b/...ava/engine/src/main/java/org/opencds/cqf/cql/engine/elm/executing/IntersectEvaluator.java
@@ -42,10 +42,6 @@ public static Object intersect(Object left, Object right, State state) {
             Object rightStart = rightInterval.getStart();
             Object rightEnd = rightInterval.getEnd();
 
-            if (leftStart == null || leftEnd == null || rightStart == null || rightEnd == null) {
-                return null;
-            }
-
             String precision = null;
             if (leftStart instanceof BaseTemporal && rightStart instanceof BaseTemporal) {
                 precision = BaseTemporal.getHighestPrecision(
@@ -54,22 +50,31 @@ public static Object intersect(Object left, Object right, State state) {
             }
 
             Boolean overlaps = OverlapsEvaluator.overlaps(leftInterval, rightInterval, precision, state);
-            if (overlaps == null || !overlaps) {
+            if (overlaps != null && !overlaps) {
                 return null;
             }
 
             Boolean leftStartGtRightStart = GreaterEvaluator.greater(leftStart, rightStart, state);
             Boolean leftEndLtRightEnd = LessEvaluator.less(leftEnd, rightEnd, state);
 
             Object max;
-            if (leftStartGtRightStart == null && precision != null) {
+            if (leftStart == null || rightStart == null) {
+                // If either of the start points is null, the start point of the intersection is null because the
+                // boundary is unknown.
+                max = null;
+            } else if (leftStartGtRightStart == null && precision != null) {
+                // It is possible for leftStartGtRightStart to be null without either leftStart or rightStart being null
+                // if one has a value for the precision and the other does not, see:
+                // https://cql.hl7.org/09-b-cqlreference.html#greater
                 max = ((BaseTemporal) leftStart).getPrecision().toString().equals(precision) ? leftStart : rightStart;
             } else {
                 max = leftStartGtRightStart == null ? null : leftStartGtRightStart ? leftStart : rightStart;
             }
 
             Object min;
-            if (leftEndLtRightEnd == null && precision != null) {
+            if (leftEnd == null || rightEnd == null) {
+                min = null;
+            } else if (leftEndLtRightEnd == null && precision != null) {
                 min = ((BaseTemporal) leftEnd).getPrecision().toString().equals(precision) ? leftEnd : rightEnd;
             } else {
                 min = leftEndLtRightEnd == null ? null : leftEndLtRightEnd ? leftEnd : rightEnd;

diff --git a/...java/engine/src/test/java/org/opencds/cqf/cql/engine/execution/IntervalOperatorsTest.java b/...java/engine/src/test/java/org/opencds/cqf/cql/engine/execution/IntervalOperatorsTest.java
@@ -2,9 +2,7 @@
 
 import static org.hamcrest.MatcherAssert.assertThat;
 import static org.hamcrest.Matchers.*;
-import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertNull;
-import static org.junit.jupiter.api.Assertions.assertTrue;
+import static org.junit.jupiter.api.Assertions.*;
 
 import java.math.BigDecimal;
 import java.util.*;
@@ -598,6 +596,31 @@ void all_interval_operators() {
         value = results.forExpression("DateTimeIncludedInPrecisionNull").value();
         assertThat(value, is(nullValue()));
 
+        value = results.forExpression("TestIntersectNullRightStart").value();
+        // Because of how nulls work, equivalence, not equality, is the relevant test here (equality just gives null).
+        assertTrue(((Interval) value).equivalent(new Interval(null, false, 5, true)));
+
+        value = results.forExpression("TestIntersectNullRightEnd").value();
+        assertTrue(((Interval) value).equivalent(new Interval(5, true, null, false)));
+
+        value = results.forExpression("TestIntersectNullLeftStart").value();
+        assertTrue(((Interval) value).equivalent(new Interval(null, false, 5, true)));
+
+        value = results.forExpression("TestIntersectNullLeftEnd").value();
+        assertTrue(((Interval) value).equivalent(new Interval(5, true, null, false)));
+
+        value = results.forExpression("TestIntersectNull1").value();
+        assertTrue((Boolean) value);
+
+        value = results.forExpression("TestIntersectNull2").value();
+        assertTrue((Boolean) value);
+
+        value = results.forExpression("TestIntersectNull3").value();
+        assertFalse((Boolean) value);
+
+        value = results.forExpression("TestIntersectNull4").value();
+        assertFalse((Boolean) value);
+
         value = results.forExpression("IntegerIntervalIntersectTest4to10").value();
         assertTrue(((Interval) value).equal(new Interval(4, true, 10, true)));
 

diff --git a/...gine/src/test/resources/org/opencds/cqf/cql/engine/execution/CqlIntervalOperatorsTest.cql b/...gine/src/test/resources/org/opencds/cqf/cql/engine/execution/CqlIntervalOperatorsTest.cql
@@ -191,7 +191,14 @@ define DateTimeIncludedInPrecisionNull:
     Interval [@2017-09-01T00:00:00, @2017-09-01T00:00:00] included in millisecond of Interval [@2017-09-01T00:00:00.000, @2017-12-30T23:59:59.999]
 
 //Intersect
-define TestIntersectNull: IntegerIntervalTest intersect Interval[5, null)
+define TestIntersectNullRightStart: IntegerIntervalTest intersect Interval(null, 5]
+define TestIntersectNullRightEnd: IntegerIntervalTest intersect Interval[5, null)
+define TestIntersectNullLeftStart: Interval(null, 5] intersect IntegerIntervalTest
+define TestIntersectNullLeftEnd: Interval[5, null) intersect IntegerIntervalTest
+define TestIntersectNull1: start of (IntegerIntervalTest intersect Interval[5, null)) <= 10
+define TestIntersectNull2: start of (Interval[1, 10] intersect Interval[5, null)) >= 5
+define TestIntersectNull3: start of (Interval[1, 10] intersect Interval[5, null)) > 10
+define TestIntersectNull4: start of (Interval[1, 10] intersect Interval[5, null)) < 5
 define IntegerIntervalIntersectTest4to10: IntegerIntervalTest intersect IntegerIntervalTest4
 define IntegerIntervalIntersectTestNull: IntegerIntervalTest intersect IntegerIntervalTest2
 define DecimalIntervalIntersectTest4to10: DecimalIntervalTest intersect DecimalIntervalTest3