[2.41] fix: PI filter equality no longer matches NULL data elements [DHIS2-20929]

dhis-2/dhis-api/src/main/java/org/hisp/dhis/program/ProgramIndicatorService.java

@@ -174,6 +174,23 @@ String getAnalyticsSql(
       Date startDate,
       Date endDate);
 
+  /**
+   * Gets the analytics SQL clause of an expression. The SQL does not substitute null values.
+   *
+   * @param expression the expression.
+   * @param dataType the data type to return.
+   * @param programIndicator the program indicator to evaluate.
+   * @param startDate the start date.
+   * @param endDate the end date.
+   * @return the SQL string.
+   */
+  String getAnalyticsSqlAllowingNulls(
+      String expression,
+      DataType dataType,
+      ProgramIndicator programIndicator,
+      Date startDate,
+      Date endDate);
+
   /**
    * Gets the the analytics SQL clause of an expression. Does not ignore missing numeric values for
    * data elements and attributes.
@@ -194,6 +211,25 @@ String getAnalyticsSql(
       Date endDate,
       String tableAlias);
 
+  /**
+   * Gets the analytics SQL clause of an expression. The SQL does not substitute null values.
+   *
+   * @param expression the expression.
+   * @param dataType the data type to return.
+   * @param programIndicator the program indicator to evaluate.
+   * @param startDate the start date.
+   * @param endDate the end date.
+   * @param tableAlias use this table alias for expression returning a inner query
+   * @return the SQL string.
+   */
+  String getAnalyticsSqlAllowingNulls(
+      String expression,
+      DataType dataType,
+      ProgramIndicator programIndicator,
+      Date startDate,
+      Date endDate,
+      String tableAlias);
+
   /**
    * Returns a SQL clause which matches any value for the data elements and attributes in the given
    * expression.

dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/event/data/JdbcEnrollmentAnalyticsManager.java

@@ -433,7 +433,7 @@ protected String getWhereClause(EventQueryParams params) {
 
     if (params.hasProgramIndicatorDimension() && params.getProgramIndicator().hasFilter()) {
       String filter =
-          programIndicatorService.getAnalyticsSql(
+          programIndicatorService.getAnalyticsSqlAllowingNulls(
               params.getProgramIndicator().getFilter(),
               BOOLEAN,
               params.getProgramIndicator(),

dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/event/data/JdbcEventAnalyticsManager.java

@@ -516,7 +516,7 @@ protected String getWhereClause(EventQueryParams params) {
 
     if (params.hasProgramIndicatorDimension() && params.getProgramIndicator().hasFilter()) {
       String filter =
-          programIndicatorService.getAnalyticsSql(
+          programIndicatorService.getAnalyticsSqlAllowingNulls(
               params.getProgramIndicator().getFilter(),
               BOOLEAN,
               params.getProgramIndicator(),

dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/event/data/programindicator/DefaultProgramIndicatorSubqueryBuilder.java

@@ -125,7 +125,7 @@ private String getAggregateClauseForPIandRelationshipType(
       aggregateSql +=
           (where.isBlank() ? " WHERE " : " AND ")
               + "("
-              + getProgramIndicatorSql(
+              + getProgramIndicatorFilterSql(
                   programIndicator.getFilter(),
                   BOOLEAN,
                   programIndicator,
@@ -205,4 +205,19 @@ private String getProgramIndicatorSql(
         latestDate,
         SUBQUERY_TABLE_ALIAS);
   }
+
+  private String getProgramIndicatorFilterSql(
+      String expression,
+      DataType dataType,
+      ProgramIndicator programIndicator,
+      Date earliestStartDate,
+      Date latestDate) {
+    return this.programIndicatorService.getAnalyticsSqlAllowingNulls(
+        expression,
+        dataType,
+        programIndicator,
+        earliestStartDate,
+        latestDate,
+        SUBQUERY_TABLE_ALIAS);
+  }
 }

dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/tei/query/context/querybuilder/ProgramIndicatorQueryBuilder.java

@@ -230,7 +230,7 @@ private ProgramIndicatorQueryParts getProgramIndicatorQueryParts(
             null,
             SUBQUERY_TABLE_ALIAS),
         // filter
-        programIndicatorService.getAnalyticsSql(
+        programIndicatorService.getAnalyticsSqlAllowingNulls(
             programIndicator.getFilter(),
             DataType.BOOLEAN,
             programIndicator,

dhis-2/dhis-services/dhis-service-analytics/src/test/java/org/hisp/dhis/analytics/event/data/AbstractJdbcEventAnalyticsManagerTest.java

@@ -61,6 +61,8 @@
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 import static org.mockito.ArgumentMatchers.any;
+import static org.mockito.ArgumentMatchers.eq;
+import static org.mockito.Mockito.lenient;
 import static org.mockito.Mockito.mock;
 import static org.mockito.Mockito.verify;
 import static org.mockito.Mockito.when;
@@ -1004,6 +1006,43 @@ private QueryItem buildQueryItemWithGroupAndFilters(
     return queryItem;
   }
 
+  /**
+   * DHIS2-20929: PI filters such as {@code #{stage.de} == 0} must not be wrapped in {@code
+   * coalesce(..., 0)} — otherwise events where the DE is NULL (or where the row belongs to a
+   * different program stage) incorrectly match the equality to zero. The filter must therefore be
+   * compiled with NULL-allowing semantics.
+   */
+  @Test
+  void verifyProgramIndicatorFilterCompiledAllowingNulls() {
+    ProgramIndicator programIndicator =
+        createProgramIndicator('A', programA, "V{event_count}", "#{ProgrmStagA.DataElmentA} == 0");
+
+    EventQueryParams params =
+        new EventQueryParams.Builder(createRequestParams())
+            .withProgramIndicator(programIndicator)
+            .build();
+
+    lenient()
+        .when(
+            programIndicatorService.getAnalyticsSqlAllowingNulls(
+                eq(programIndicator.getFilter()),
+                eq(org.hisp.dhis.analytics.DataType.BOOLEAN),
+                eq(programIndicator),
+                any(Date.class),
+                any(Date.class)))
+        .thenReturn("ax.\"DataElmentA\" = 0");
+
+    eventSubject.getWhereClause(params);
+
+    verify(programIndicatorService)
+        .getAnalyticsSqlAllowingNulls(
+            eq(programIndicator.getFilter()),
+            eq(org.hisp.dhis.analytics.DataType.BOOLEAN),
+            eq(programIndicator),
+            any(Date.class),
+            any(Date.class));
+  }
+
   private EventQueryParams getEventQueryParamsForCoordinateFieldsTest(
       List<String> coordinateFields) {
     DataElement deA = createDataElement('A', TEXT, AggregationType.NONE);

dhis-2/dhis-services/dhis-service-analytics/src/test/java/org/hisp/dhis/analytics/event/data/programindicator/ProgramIndicatorSubqueryBuilderTest.java

@@ -185,7 +185,7 @@ void verifyProgramIndicatorWithFilter() {
     when(programIndicatorService.getAnalyticsSql(
             DUMMY_EXPRESSION, NUMERIC, pi, startDate, endDate, "subax"))
         .thenReturn("distinct psi");
-    when(programIndicatorService.getAnalyticsSql(
+    when(programIndicatorService.getAnalyticsSqlAllowingNulls(
             DUMMY_FILTER_EXPRESSION, BOOLEAN, pi, startDate, endDate, "subax"))
         .thenReturn("a = b");
 

dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/program/DefaultProgramIndicatorService.java

@@ -90,6 +90,7 @@
 import org.hisp.dhis.parser.expression.CommonExpressionVisitor;
 import org.hisp.dhis.parser.expression.ExpressionItem;
 import org.hisp.dhis.parser.expression.ExpressionItemMethod;
+import org.hisp.dhis.parser.expression.ExpressionState;
 import org.hisp.dhis.parser.expression.ProgramExpressionParams;
 import org.hisp.dhis.parser.expression.function.RepeatableProgramStageOffset;
 import org.hisp.dhis.parser.expression.function.VectorAvg;
@@ -333,7 +334,20 @@ public String getAnalyticsSql(
       ProgramIndicator programIndicator,
       Date startDate,
       Date endDate) {
-    return getAnalyticsSqlCached(expression, dataType, programIndicator, startDate, endDate, null);
+    return getAnalyticsSqlCached(
+        expression, dataType, programIndicator, startDate, endDate, null, true);
+  }
+
+  @Override
+  @Transactional(readOnly = true)
+  public String getAnalyticsSqlAllowingNulls(
+      String expression,
+      DataType dataType,
+      ProgramIndicator programIndicator,
+      Date startDate,
+      Date endDate) {
+    return getAnalyticsSqlCached(
+        expression, dataType, programIndicator, startDate, endDate, null, false);
   }
 
   @Override
@@ -346,29 +360,49 @@ public String getAnalyticsSql(
       Date endDate,
       String tableAlias) {
     return getAnalyticsSqlCached(
-        expression, dataType, programIndicator, startDate, endDate, tableAlias);
+        expression, dataType, programIndicator, startDate, endDate, tableAlias, true);
   }
 
-  private String getAnalyticsSqlCached(
+  @Override
+  @Transactional(readOnly = true)
+  public String getAnalyticsSqlAllowingNulls(
       String expression,
       DataType dataType,
       ProgramIndicator programIndicator,
       Date startDate,
       Date endDate,
       String tableAlias) {
+    return getAnalyticsSqlCached(
+        expression, dataType, programIndicator, startDate, endDate, tableAlias, false);
+  }
+
+  private String getAnalyticsSqlCached(
+      String expression,
+      DataType dataType,
+      ProgramIndicator programIndicator,
+      Date startDate,
+      Date endDate,
+      String tableAlias,
+      boolean replaceNulls) {
     if (expression == null) {
       return null;
     }
 
     String cacheKey =
         getAnalyticsSqlCacheKey(
-            expression, dataType, programIndicator, startDate, endDate, tableAlias);
+            expression, dataType, programIndicator, startDate, endDate, tableAlias, replaceNulls);
 
     return analyticsSqlCache.get(
         cacheKey,
         k ->
             getAnalyticsSqlInternal(
-                expression, dataType, programIndicator, startDate, endDate, tableAlias));
+                expression,
+                dataType,
+                programIndicator,
+                startDate,
+                endDate,
+                tableAlias,
+                replaceNulls));
   }
 
   private String getAnalyticsSqlCacheKey(
@@ -377,7 +411,8 @@ private String getAnalyticsSqlCacheKey(
       ProgramIndicator programIndicator,
       Date startDate,
       Date endDate,
-      String tableAlias) {
+      String tableAlias,
+      boolean replaceNulls) {
     return expression
         + "|"
         + dataType.name()
@@ -386,7 +421,8 @@ private String getAnalyticsSqlCacheKey(
         + dateIfPresent(startDate)
         + dateIfPresent(endDate)
         + "|"
-        + (tableAlias == null ? "" : tableAlias);
+        + (tableAlias == null ? "" : tableAlias)
+        + replaceNulls;
   }
 
   /**
@@ -408,7 +444,8 @@ private String getAnalyticsSqlInternal(
       ProgramIndicator programIndicator,
       Date startDate,
       Date endDate,
-      String tableAlias) {
+      String tableAlias,
+      boolean replaceNulls) {
     // Get the uids from the expression even if this is the filter
     Set<String> uids =
         getDataElementAndAttributeIdentifiers(
@@ -424,7 +461,7 @@ private String getAnalyticsSqlInternal(
             .dataElementAndAttributeIdentifiers(uids)
             .build();
 
-    CommonExpressionVisitor visitor = newVisitor(ITEM_GET_SQL, params, progParams);
+    CommonExpressionVisitor visitor = newVisitor(ITEM_GET_SQL, params, progParams, replaceNulls);
 
     visitor.setExpressionLiteral(new SqlLiteral());
 
@@ -511,6 +548,14 @@ private CommonExpressionVisitor newVisitor(
       ExpressionItemMethod itemMethod,
       ExpressionParams params,
       ProgramExpressionParams progParams) {
+    return newVisitor(itemMethod, params, progParams, true);
+  }
+
+  private CommonExpressionVisitor newVisitor(
+      ExpressionItemMethod itemMethod,
+      ExpressionParams params,
+      ProgramExpressionParams progParams,
+      boolean replaceNulls) {
     return CommonExpressionVisitor.builder()
         .idObjectManager(idObjectManager)
         .dimensionService(dimensionService)
@@ -522,6 +567,7 @@ private CommonExpressionVisitor newVisitor(
         .itemMethod(itemMethod)
         .params(params)
         .progParams(progParams)
+        .state(ExpressionState.builder().replaceNulls(replaceNulls).build())
         .build();
   }
 

dhis-2/dhis-test-e2e/src/test/java/org/hisp/dhis/analytics/aggregate/AnalyticsQueryDv8AutoTest.java

@@ -331,14 +331,12 @@ public void queryChildHealthAndInpatientIndicators() throws JSONException {
     validateRow(response, List.of("htr2mMY515K", "202101", "141.0", "", "", "", "", ""));
     validateRow(response, List.of("htr2mMY515K", "202102", "136.0", "", "", "", "", ""));
     validateRow(response, List.of("htr2mMY515K", "202103", "163.0", "", "", "", "", ""));
-    validateRow(response, List.of("htr2mMY515K", "202104", "163.0", "", "", "", "", ""));
     validateRow(response, List.of("htr2mMY515K", "202105", "126.0", "", "", "", "", ""));
     validateRow(response, List.of("htr2mMY515K", "202106", "162.0", "", "", "", "", ""));
     validateRow(response, List.of("htr2mMY515K", "202107", "158.0", "", "", "", "", ""));
     validateRow(response, List.of("htr2mMY515K", "202108", "154.0", "", "", "", "", ""));
     validateRow(response, List.of("htr2mMY515K", "202109", "149.0", "", "", "", "", ""));
     validateRow(response, List.of("htr2mMY515K", "202110", "156.0", "", "", "", "", ""));
-    validateRow(response, List.of("htr2mMY515K", "202111", "181.0", "", "", "", "", ""));
     validateRow(response, List.of("htr2mMY515K", "202112", "139.0", "", "", "", "", ""));
     validateRow(response, List.of("tUdBD1JDxpn", "202101", "42.82", "", "", "", "", ""));
     validateRow(response, List.of("tUdBD1JDxpn", "202102", "43.78", "", "", "", "", ""));
@@ -391,14 +389,11 @@ public void queryChildHealthAndInpatientIndicators() throws JSONException {
     validateRow(response, List.of("ToQVD4irW3Q", "202101", "49.19", "", "", "", "", ""));
     validateRow(response, List.of("ToQVD4irW3Q", "202102", "50.39", "", "", "", "", ""));
     validateRow(response, List.of("ToQVD4irW3Q", "202103", "48.95", "", "", "", "", ""));
-    validateRow(response, List.of("ToQVD4irW3Q", "202104", "48.24", "", "", "", "", ""));
     validateRow(response, List.of("ToQVD4irW3Q", "202105", "49.01", "", "", "", "", ""));
     validateRow(response, List.of("ToQVD4irW3Q", "202106", "49.26", "", "", "", "", ""));
     validateRow(response, List.of("ToQVD4irW3Q", "202107", "49.89", "", "", "", "", ""));
     validateRow(response, List.of("ToQVD4irW3Q", "202108", "48.66", "", "", "", "", ""));
     validateRow(response, List.of("ToQVD4irW3Q", "202109", "48.86", "", "", "", "", ""));
-    validateRow(response, List.of("ToQVD4irW3Q", "202110", "48.23", "", "", "", "", ""));
-    validateRow(response, List.of("ToQVD4irW3Q", "202111", "49.09", "", "", "", "", ""));
     validateRow(response, List.of("ToQVD4irW3Q", "202112", "51.3", "", "", "", "", ""));
     validateRow(response, List.of("ReQEl5V3z6p", "202101", "49.93", "", "", "", "", ""));
     validateRow(response, List.of("ReQEl5V3z6p", "202102", "49.49", "", "", "", "", ""));
@@ -427,14 +422,11 @@ public void queryChildHealthAndInpatientIndicators() throws JSONException {
     validateRow(response, List.of("vDdRoZYybP2", "202101", "295.0", "", "", "", "", ""));
     validateRow(response, List.of("vDdRoZYybP2", "202102", "288.0", "", "", "", "", ""));
     validateRow(response, List.of("vDdRoZYybP2", "202103", "320.0", "", "", "", "", ""));
-    validateRow(response, List.of("vDdRoZYybP2", "202104", "317.0", "", "", "", "", ""));
     validateRow(response, List.of("vDdRoZYybP2", "202105", "298.0", "", "", "", "", ""));
     validateRow(response, List.of("vDdRoZYybP2", "202106", "307.0", "", "", "", "", ""));
     validateRow(response, List.of("vDdRoZYybP2", "202107", "316.0", "", "", "", "", ""));
     validateRow(response, List.of("vDdRoZYybP2", "202108", "299.0", "", "", "", "", ""));
     validateRow(response, List.of("vDdRoZYybP2", "202109", "310.0", "", "", "", "", ""));
-    validateRow(response, List.of("vDdRoZYybP2", "202110", "303.0", "", "", "", "", ""));
-    validateRow(response, List.of("vDdRoZYybP2", "202111", "316.0", "", "", "", "", ""));
     validateRow(response, List.of("vDdRoZYybP2", "202112", "282.0", "", "", "", "", ""));
     validateRow(response, List.of("p2Zxg0wcPQ3", "202107", "482.0", "", "", "", "", ""));
     validateRow(response, List.of("p2Zxg0wcPQ3", "202103", "493.0", "", "", "", "", ""));
@@ -535,14 +527,12 @@ public void queryChildHealthAndInpatientIndicators() throws JSONException {
     validateRow(response, List.of("Y7hKDSuqEtH", "202101", "75.51", "", "", "", "", ""));
     validateRow(response, List.of("Y7hKDSuqEtH", "202102", "60.76", "", "", "", "", ""));
     validateRow(response, List.of("Y7hKDSuqEtH", "202103", "65.06", "", "", "", "", ""));
-    validateRow(response, List.of("Y7hKDSuqEtH", "202104", "61.91", "", "", "", "", ""));
     validateRow(response, List.of("Y7hKDSuqEtH", "202105", "63.63", "", "", "", "", ""));
     validateRow(response, List.of("Y7hKDSuqEtH", "202106", "69.18", "", "", "", "", ""));
     validateRow(response, List.of("Y7hKDSuqEtH", "202107", "68.54", "", "", "", "", ""));
     validateRow(response, List.of("Y7hKDSuqEtH", "202108", "65.73", "", "", "", "", ""));
     validateRow(response, List.of("Y7hKDSuqEtH", "202109", "69.79", "", "", "", "", ""));
     validateRow(response, List.of("Y7hKDSuqEtH", "202110", "69.22", "", "", "", "", ""));
-    validateRow(response, List.of("Y7hKDSuqEtH", "202111", "61.1", "", "", "", "", ""));
     validateRow(response, List.of("Y7hKDSuqEtH", "202112", "71.79", "", "", "", "", ""));
   }