Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix Intersect on intervals with nulls #1381

Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -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(
Expand All @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.*;
Expand Down Expand Up @@ -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)));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
Loading