From 36ff443d84a3254aad1b5c3ead571aa35ffdbf0f Mon Sep 17 00:00:00 2001 From: holly-smile <166412459+holly-smile@users.noreply.github.com> Date: Wed, 3 Jul 2024 11:29:55 -0700 Subject: [PATCH] Fix aggregate function errors (#1375) * Test cases * Fix AllTrue and AnyTrue * Fix standard deviation aggregators * Tests --------- Co-authored-by: JP --- .../elm/executing/AllTrueEvaluator.java | 2 +- .../elm/executing/AnyTrueEvaluator.java | 3 +-- .../executing/PopulationStdDevEvaluator.java | 4 ++++ .../engine/elm/executing/StdDevEvaluator.java | 4 ++++ .../execution/CqlAggregateFunctionsTest.java | 19 +++++++++++++++++++ .../execution/CqlAggregateFunctionsTest.cql | 6 +++++- 6 files changed, 34 insertions(+), 4 deletions(-) diff --git a/Src/java/engine/src/main/java/org/opencds/cqf/cql/engine/elm/executing/AllTrueEvaluator.java b/Src/java/engine/src/main/java/org/opencds/cqf/cql/engine/elm/executing/AllTrueEvaluator.java index dd57b798b..aa521f8a9 100644 --- a/Src/java/engine/src/main/java/org/opencds/cqf/cql/engine/elm/executing/AllTrueEvaluator.java +++ b/Src/java/engine/src/main/java/org/opencds/cqf/cql/engine/elm/executing/AllTrueEvaluator.java @@ -15,7 +15,7 @@ public class AllTrueEvaluator { public static Boolean allTrue(Object src) { if (src == null) { - return null; + return true; } if (src instanceof Iterable) { diff --git a/Src/java/engine/src/main/java/org/opencds/cqf/cql/engine/elm/executing/AnyTrueEvaluator.java b/Src/java/engine/src/main/java/org/opencds/cqf/cql/engine/elm/executing/AnyTrueEvaluator.java index e6f727d81..631d21672 100644 --- a/Src/java/engine/src/main/java/org/opencds/cqf/cql/engine/elm/executing/AnyTrueEvaluator.java +++ b/Src/java/engine/src/main/java/org/opencds/cqf/cql/engine/elm/executing/AnyTrueEvaluator.java @@ -14,9 +14,8 @@ public class AnyTrueEvaluator { public static Boolean anyTrue(Object src) { - if (src == null) { - return null; + return false; } if (src instanceof Iterable) { diff --git a/Src/java/engine/src/main/java/org/opencds/cqf/cql/engine/elm/executing/PopulationStdDevEvaluator.java b/Src/java/engine/src/main/java/org/opencds/cqf/cql/engine/elm/executing/PopulationStdDevEvaluator.java index 48b55f91c..ff4d9b0e1 100644 --- a/Src/java/engine/src/main/java/org/opencds/cqf/cql/engine/elm/executing/PopulationStdDevEvaluator.java +++ b/Src/java/engine/src/main/java/org/opencds/cqf/cql/engine/elm/executing/PopulationStdDevEvaluator.java @@ -31,6 +31,10 @@ public static Object popStdDev(Object source, State state) { } Object variance = PopulationVarianceEvaluator.popVariance(source, state); + // The cases in which PopulationVariance returns null are the same as those where PopulationStdDev does. + if(variance == null) { + return null; + } return variance instanceof BigDecimal ? PowerEvaluator.power(variance, new BigDecimal("0.5")) diff --git a/Src/java/engine/src/main/java/org/opencds/cqf/cql/engine/elm/executing/StdDevEvaluator.java b/Src/java/engine/src/main/java/org/opencds/cqf/cql/engine/elm/executing/StdDevEvaluator.java index e95d99939..cc9e27caa 100644 --- a/Src/java/engine/src/main/java/org/opencds/cqf/cql/engine/elm/executing/StdDevEvaluator.java +++ b/Src/java/engine/src/main/java/org/opencds/cqf/cql/engine/elm/executing/StdDevEvaluator.java @@ -31,6 +31,10 @@ public static Object stdDev(Object source, State state) { } Object variance = VarianceEvaluator.variance(source, state); + // The cases in which Variance returns null are the same as those where StdDev does. + if(variance == null) { + return null; + } return variance instanceof BigDecimal ? PowerEvaluator.power(variance, new BigDecimal("0.5")) diff --git a/Src/java/engine/src/test/java/org/opencds/cqf/cql/engine/execution/CqlAggregateFunctionsTest.java b/Src/java/engine/src/test/java/org/opencds/cqf/cql/engine/execution/CqlAggregateFunctionsTest.java index a80380903..769e23470 100644 --- a/Src/java/engine/src/test/java/org/opencds/cqf/cql/engine/execution/CqlAggregateFunctionsTest.java +++ b/Src/java/engine/src/test/java/org/opencds/cqf/cql/engine/execution/CqlAggregateFunctionsTest.java @@ -2,10 +2,13 @@ import static org.hamcrest.MatcherAssert.assertThat; import static org.hamcrest.Matchers.is; +import static org.hamcrest.Matchers.nullValue; import static org.junit.jupiter.api.Assertions.*; import java.math.BigDecimal; import java.util.Arrays; + +import org.hamcrest.core.IsNull; import org.junit.jupiter.api.Test; import org.opencds.cqf.cql.engine.elm.executing.AnyTrueEvaluator; import org.opencds.cqf.cql.engine.elm.executing.AvgEvaluator; @@ -43,6 +46,9 @@ void all_aggregate_function_tests() { value = results.forExpression("AllTrueEmptyList").value(); assertThat(value, is(true)); + value = results.forExpression("AllTrueIsTrueWhenNull").value(); + assertThat(value, is(true)); + value = results.forExpression("AnyTrueAllTrue").value(); assertThat(value, is(true)); @@ -70,6 +76,10 @@ void all_aggregate_function_tests() { value = results.forExpression("AnyTrueEmptyList").value(); assertThat(value, is(false)); + value = results.forExpression("AnyTrueIsFalseWhenNull").value(); + assertThat(value, is(false)); + + try { value = AnyTrueEvaluator.anyTrue(Arrays.asList("this", "is", "error")); fail(); @@ -152,15 +162,24 @@ void all_aggregate_function_tests() { ((BigDecimal) value) .compareTo(new BigDecimal("1.41421356"))); // 23730951454746218587388284504413604736328125 + value = results.forExpression("PopulationStdDevIsNull").value(); + assertThat(value, is(nullValue())); + value = results.forExpression("PopVarianceTest1").value(); assertEquals(0, ((BigDecimal) value).compareTo(new BigDecimal("2.0"))); + value = results.forExpression("PopVarianceIsNull").value(); + assertThat(value, is(nullValue())); + value = results.forExpression("StdDevTest1").value(); assertEquals( 0, ((BigDecimal) value) .compareTo(new BigDecimal("1.58113883"))); // 00841897613935316257993690669536590576171875 + value = results.forExpression("StdDevIsNull").value(); + assertThat(value, is(nullValue())); + value = results.forExpression("SumTest1").value(); assertThat(value, is(new BigDecimal("20.0"))); diff --git a/Src/java/engine/src/test/resources/org/opencds/cqf/cql/engine/execution/CqlAggregateFunctionsTest.cql b/Src/java/engine/src/test/resources/org/opencds/cqf/cql/engine/execution/CqlAggregateFunctionsTest.cql index 4930fa1a4..acbed4f77 100644 --- a/Src/java/engine/src/test/resources/org/opencds/cqf/cql/engine/execution/CqlAggregateFunctionsTest.cql +++ b/Src/java/engine/src/test/resources/org/opencds/cqf/cql/engine/execution/CqlAggregateFunctionsTest.cql @@ -8,7 +8,7 @@ define AllTrueAllTrueFalseTrue : AllTrue({true,false,true}) define AllTrueAllFalseTrueFalse : AllTrue({false,true,false}) define AllTrueNullFirst : AllTrue({null,true,true}) define AllTrueEmptyList : AllTrue({}) - +define AllTrueIsTrueWhenNull : AllTrue(null) //AnyTrue define AnyTrueAllTrue : AnyTrue({true,true}) define AnyTrueAllFalse : AnyTrue({false,false}) @@ -19,6 +19,7 @@ define AnyTrueFalseFirst : AnyTrue({false,true}) define AnyTrueNullFirstThenTrue : AnyTrue({null,true}) define AnyTrueNullFirstThenFalse : AnyTrue({null,false}) define AnyTrueEmptyList : AnyTrue({}) +define AnyTrueIsFalseWhenNull : AnyTrue(null) //Avg define AvgTest1: Avg({ 1.0, 2.0, 3.0, 6.0 }) @@ -57,12 +58,15 @@ define ModeTestTime: Mode({ @T15:59:59.999, @T05:59:59.999, @T20:59:59.999, @T05 //PopulationStdDev define PopStdDevTest1: PopulationStdDev({ 1.0, 2.0, 3.0, 4.0, 5.0 }) +define PopulationStdDevIsNull: PopulationStdDev({ null as Quantity, null as Quantity, null as Quantity }) //PopulationVariance define PopVarianceTest1: PopulationVariance({ 1.0, 2.0, 3.0, 4.0, 5.0 }) +define PopVarianceIsNull: PopulationVariance({ null as Quantity, null as Quantity, null as Quantity }) //StdDev define StdDevTest1: StdDev({ 1.0, 2.0, 3.0, 4.0, 5.0 }) +define StdDevIsNull: StdDev({ null as Quantity, null as Quantity, null as Quantity }) //Sum define SumTest1: Sum({ 6.0, 2.0, 3.0, 4.0, 5.0 })