SQ various fixes + Unit Tests

dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/common/InQueryCteFilter.java

@@ -30,7 +30,10 @@
 import static org.hisp.dhis.analytics.QueryKey.NV;
 
 import java.util.List;
+import java.util.Objects;
 import java.util.function.Predicate;
+import java.util.stream.Collectors;
+import java.util.stream.Stream;
 import org.hisp.dhis.common.QueryFilter;
 
 /** Mimics the logic of @{@link org.hisp.dhis.common.InQueryFilter} to be used in CTEs */
@@ -59,10 +62,15 @@ public String getSqlFilter(int offset) {
     StringBuilder condition = new StringBuilder();
     String alias = cteDefinition.getAlias(offset);
     if (hasNonMissingValue(filterItems)) {
-      // TODO GIUSEPPE!
-
+      condition
+          .append("%s.%s in".formatted(alias, field))
+          .append(
+              streamOfNonMissingValues(filterItems)
+                  .filter(Objects::nonNull)
+                  .map(this::quoteIfNecessary)
+                  .collect(Collectors.joining(",", " (", ")")));
       if (hasMissingValue(filterItems)) {
-
+        throw new UnsupportedOperationException("Not implemented yet");
         // TODO GIUSEPPE!
       }
     } else {
@@ -124,4 +132,16 @@ private boolean anyMatch(List<String> filterItems, Predicate<String> predi) {
   private boolean hasMissingValue(List<String> filterItems) {
     return anyMatch(filterItems, this::isMissingItem);
   }
+
+  private Stream<String> streamOfNonMissingValues(List<String> filterItems) {
+    return filterItems.stream().filter(this::isNotMissingItem);
+  }
+
+  private String quoteIfNecessary(String item) {
+    return isText ? quote(item) : item;
+  }
+
+  protected String quote(String filterItem) {
+    return "'" + filterItem + "'";
+  }
 }

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

@@ -1440,7 +1440,7 @@ protected List<String> getSelectColumnsWithCTE(EventQueryParams params, CteConte
         }
       } else if (queryItem.hasProgramStage()) {
         // Handle program stage items with CTE
-        columns.add(getColumnWithCte(queryItem, "", cteContext));
+        columns.add(getColumnWithCte(queryItem, cteContext));
       } else {
         // Handle other types as before
         ColumnAndAlias columnAndAlias = getColumnAndAlias(queryItem, false, "");
@@ -1462,7 +1462,8 @@ protected boolean useExperimentalAnalyticsQueryEngine() {
    */
   protected abstract String getSelectClause(EventQueryParams params);
 
-  protected abstract String getColumnWithCte(QueryItem item, String suffix, CteContext cteContext);
+  /** Returns the column name associated with the CTE */
+  protected abstract String getColumnWithCte(QueryItem item, CteContext cteContext);
 
   /**
    * Generates the SQL for the from-clause. Generally this means which analytics table to get data

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

@@ -160,13 +160,12 @@ public void getEnrollments(EventQueryParams params, Grid grid, int maxLimit) {
               ? buildAggregatedEnrollmentQueryWithCte(grid.getHeaders(), params)
               : getAggregatedEnrollmentsSql(grid.getHeaders(), params);
     } else {
-      // getAggregatedEnrollmentsSql
       sql =
           useExperimentalAnalyticsQueryEngine()
               ? buildEnrollmentQueryWithCte(params)
               : getAggregatedEnrollmentsSql(params, maxLimit);
     }
-
+    System.out.println(sql);
     if (params.analyzeOnly()) {
       withExceptionHandling(
           () -> executionPlanStore.addExecutionPlan(params.getExplainOrderId(), sql));
@@ -519,56 +518,127 @@ private String addFiltersToWhereClause(EventQueryParams params) {
     return getQueryItemsAndFiltersWhereClause(params, new SqlHelper());
   }
 
-  private String addCteFiltersToWhereClause(EventQueryParams params, CteContext cteContext) {
-    StringBuilder cteWhereClause = new StringBuilder();
-    Set<QueryItem> processedItems = new HashSet<>(); // Track processed items
+  /**
+   * Builds a WHERE clause by combining CTE filters and non-CTE filters for event queries.
+   *
+   * @param params The event query parameters containing items and filters
+   * @param cteContext The CTE context containing CTE definitions
+   * @return A Condition representing the combined WHERE clause
+   * @throws IllegalArgumentException if params or cteContext is null
+   */
+  private Condition addCteFiltersToWhereClause(EventQueryParams params, CteContext cteContext) {
+    if (params == null || cteContext == null) {
+      throw new IllegalArgumentException("Query parameters and CTE context cannot be null");
+    }
+
+    Set<QueryItem> processedItems = new HashSet<>();
+
+    // Build CTE conditions
+    Condition cteConditions = buildCteConditions(params, cteContext, processedItems);
+
+    // Get non-CTE conditions
+    String nonCteWhereClause =
+        getQueryItemsAndFiltersWhereClause(params, processedItems, new SqlHelper())
+            .replace("where", "");
+
+    // Combine conditions
+    if (!nonCteWhereClause.isEmpty()) {
+      return cteConditions != null
+          ? Condition.and(cteConditions, Condition.raw(nonCteWhereClause))
+          : Condition.raw(nonCteWhereClause);
+    }
+
+    return cteConditions;
+  }
+
+  /**
+   * Builds conditions for CTE filters.
+   *
+   * @param params The event query parameters
+   * @param cteContext The CTE context
+   * @param processedItems Set to track processed items
+   * @return Combined condition for CTE filters
+   */
+  private Condition buildCteConditions(
+      EventQueryParams params, CteContext cteContext, Set<QueryItem> processedItems) {
+
+    List<Condition> conditions = new ArrayList<>();
 
-    // Get all filters from the query items and item filters
     List<QueryItem> filters =
         Stream.concat(params.getItems().stream(), params.getItemFilters().stream())
             .filter(QueryItem::hasFilter)
             .toList();
-    // Iterate over each filter and apply the correct condition
+
     for (QueryItem item : filters) {
       String cteName = CteUtils.computeKey(item);
 
       if (cteContext.containsCte(cteName)) {
-        processedItems.add(item); // Mark item as processed
-        CteDefinition cteDef = cteContext.getDefinitionByItemUid(cteName);
-        for (QueryFilter filter : item.getFilters()) {
-          if (IN.equals(filter.getOperator())) {
-            InQueryCteFilter inQueryCteFilter =
-                new InQueryCteFilter("value", filter.getFilter(), item.isText(), cteDef);
-            cteWhereClause
-                .append(" and ")
-                .append(
-                    inQueryCteFilter.getSqlFilter(
-                        computeRowNumberOffset(item.getProgramStageOffset())));
-          } else {
-            String value = getSqlFilterValue(filter, item);
-
-            cteWhereClause
-                .append(" and ")
-                .append(cteDef.getAlias())
-                .append(".value ")
-                .append("NULL".equals(value) ? "is" : filter.getSqlOperator())
-                .append(" ")
-                .append(value);
-          }
-        }
+        processedItems.add(item);
+        conditions.addAll(buildItemConditions(item, cteContext.getDefinitionByItemUid(cteName)));
       }
     }
-    // Add filters for items that are not part of the CTE
-    String nonCteWhereClause =
-        getQueryItemsAndFiltersWhereClause(params, processedItems, new SqlHelper())
-            .replace("where", "");
-    if (nonCteWhereClause.isEmpty()) return cteWhereClause.toString();
 
-    String currentWhereClause = cteWhereClause.toString().toLowerCase().trim();
-    cteWhereClause.append(
-        currentWhereClause.endsWith("and") ? nonCteWhereClause : " and " + nonCteWhereClause);
+    return conditions.isEmpty() ? null : Condition.and(conditions.toArray(new Condition[0]));
+  }
+
+  /**
+   * Builds conditions for a single query item.
+   *
+   * @param item The query item
+   * @param cteDef The CTE definition
+   * @return List of conditions for the item
+   */
+  private List<Condition> buildItemConditions(QueryItem item, CteDefinition cteDef) {
+    return item.getFilters().stream()
+        .map(filter -> buildFilterCondition(filter, item, cteDef))
+        .collect(Collectors.toList());
+  }
+
+  /**
+   * Builds a condition for a single filter.
+   *
+   * @param filter The query filter
+   * @param item The query item
+   * @param cteDef The CTE definition
+   * @return Condition for the filter
+   */
+  private Condition buildFilterCondition(QueryFilter filter, QueryItem item, CteDefinition cteDef) {
+    return IN.equals(filter.getOperator())
+        ? buildInFilterCondition(filter, item, cteDef)
+        : buildStandardFilterCondition(filter, item, cteDef);
+  }
+
+  /**
+   * Builds a condition for an IN filter.
+   *
+   * @param filter The IN query filter
+   * @param item The query item
+   * @param cteDef The CTE definition
+   * @return Condition for the IN filter
+   */
+  private Condition buildInFilterCondition(
+      QueryFilter filter, QueryItem item, CteDefinition cteDef) {
+    InQueryCteFilter inQueryCteFilter =
+        new InQueryCteFilter("value", filter.getFilter(), item.isText(), cteDef);
+
+    return Condition.raw(
+        inQueryCteFilter.getSqlFilter(computeRowNumberOffset(item.getProgramStageOffset())));
+  }
+
+  /**
+   * Builds a condition for a standard (non-IN) filter.
+   *
+   * @param filter The query filter
+   * @param item The query item
+   * @param cteDef The CTE definition
+   * @return Condition for the standard filter
+   */
+  private Condition buildStandardFilterCondition(
+      QueryFilter filter, QueryItem item, CteDefinition cteDef) {
+    String value = getSqlFilterValue(filter, item);
+    String operator = "NULL".equals(value) ? "is" : filter.getSqlOperator();
 
-    return cteWhereClause.toString();
+    return Condition.raw(String.format("%s.value %s %s", cteDef.getAlias(), operator, value));
   }
 
   private String getSqlFilterValue(QueryFilter filter, QueryItem item) {
@@ -729,12 +799,19 @@ protected ColumnAndAlias getCoordinateColumn(QueryItem item, String suffix) {
   }
 
   @Override
-  protected String getColumnWithCte(QueryItem item, String suffix, CteContext cteContext) {
+  protected String getColumnWithCte(QueryItem item, CteContext cteContext) {
     List<String> columns = new ArrayList<>();
 
+    // Get the CTE definition for the item
     CteDefinition cteDef = cteContext.getDefinitionByItemUid(computeKey(item));
+    if (cteDef == null) {
+      throw new IllegalArgumentException("CTE definition not found for item: " + item);
+    }
     int programStageOffset = computeRowNumberOffset(item.getProgramStageOffset());
-    String alias = getAlias(item).orElse(null);
+    // calculate the alias for the column
+    // if the item is not a repeatable stage, the alias is the program stage + item name
+    String alias =
+        getAlias(item).orElse("%s.%s".formatted(item.getProgramStage().getUid(), item.getItemId()));
     columns.add("%s.value as %s".formatted(cteDef.getAlias(programStageOffset), quote(alias)));
     if (cteDef.isRowContext()) {
       // Add additional status and exists columns for row context
@@ -1164,8 +1241,6 @@ private String buildMainCteSql(
    *
    * @param cteContext the {@link CteContext} to which the new CTE definition(s) will be added
    * @param item the {@link QueryItem} containing program-stage details
-   * @param params the {@link EventQueryParams}, used for checking row-context eligibility, offsets,
-   *     etc.
    * @param eventTableName the event table name
    * @param colName the quoted column name for the item
    */
@@ -1503,7 +1578,7 @@ private void addFromClause(SelectBuilder sb, EventQueryParams params) {
    */
   private void addWhereClause(SelectBuilder sb, EventQueryParams params, CteContext cteContext) {
     Condition baseConditions = Condition.raw(getWhereClause(params));
-    Condition cteConditions = Condition.raw(addCteFiltersToWhereClause(params, cteContext));
+    Condition cteConditions = addCteFiltersToWhereClause(params, cteContext);
     sb.where(Condition.and(baseConditions, cteConditions));
   }
 

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

@@ -396,7 +396,7 @@ private String getCoordinateSelectExpression(EventQueryParams params) {
   }
 
   @Override
-  protected String getColumnWithCte(QueryItem item, String suffix, CteContext cteContext) {
+  protected String getColumnWithCte(QueryItem item, CteContext cteContext) {
     // TODO: Implement
     return "";
   }

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

@@ -116,7 +116,7 @@ public void contributeCte(
                   earliestStartDate,
                   latestDate)
               // FIXME this is a bit of an hack
-              .replace("subax\\.", "");
+              .replace("subax.", "");
       filter = "where " + piResolvedSqlFilter;
     }
 
@@ -128,7 +128,7 @@ public void contributeCte(
                 earliestStartDate,
                 latestDate)
             // FIXME this is a bit of an hack
-            .replace("subax\\.", "");
+            .replace("subax.", "");
 
     String cteSql =
         "select enrollment, %s(%s) as value from %s %s group by enrollment"

dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/util/sql/Condition.java

@@ -31,6 +31,7 @@
 import java.util.Arrays;
 import java.util.Collection;
 import java.util.List;
+import java.util.Objects;
 import java.util.stream.Collectors;
 
 /**
@@ -145,7 +146,7 @@ static Condition raw(String sql) {
    * @return a new And condition containing all provided conditions
    */
   static Condition and(Condition... conditions) {
-    return new And(Arrays.asList(conditions));
+    return new And(Arrays.asList(conditions).stream().filter(Objects::nonNull).toList());
   }
 
   /**