perf: extract AOC ACL check into MATERIALIZED CTE for data value export [DHIS2-21497] (2.43)

dhis-2/dhis-api/src/main/java/org/hisp/dhis/sql/QueryBuilder.java

@@ -87,7 +87,8 @@ public final class QueryBuilder {
   private static final Pattern WHERE_AND =
       Pattern.compile("([\n\t ]+)WHERE[\n\t ]+(?:1=1)?[\n\t ]+AND[\n\t ]+");
 
-  private static final Pattern WITH_START = Pattern.compile("^\\s*[a-z_]{1,30}\\s+AS\\s*\\(\\s*$");
+  private static final Pattern WITH_START =
+      Pattern.compile("^\\s*[a-z_]{1,30}\\s+AS(?:\\s+MATERIALIZED)?\\s*\\(\\s*$");
   private static final Pattern WITH_END_COMMA = Pattern.compile("^\\s*\\)\\s*,\\s*$");
   private static final Pattern WITH_END_SELECT = Pattern.compile("^\\s*\\)\\s*$");
   private static final Pattern WITH_END_COMMA_SELECT =

dhis-2/dhis-services/dhis-service-core/src/main/java/org/hisp/dhis/datavalue/hibernate/HibernateDataExportStore.java

@@ -31,6 +31,7 @@
 
 import static java.lang.System.currentTimeMillis;
 import static java.util.function.Function.identity;
+import static org.apache.commons.collections4.CollectionUtils.isEmpty;
 import static org.hisp.dhis.query.JpaQueryUtils.generateSQlQueryForSharingCheck;
 import static org.hisp.dhis.security.acl.AclService.LIKE_READ_DATA;
 import static org.hisp.dhis.user.CurrentUserUtil.getCurrentUserDetails;
@@ -131,6 +132,12 @@ public Stream<DataExportValue> exportValues(@Nonnull DataExportParams params) {
 
   static QueryBuilder createExportQuery(
       DataExportParams params, SQL.QueryAPI api, UserDetails currentUser) {
+    String aocAclSql = null;
+    boolean isSuper = currentUser.isSuper();
+    // explicit AOCs mean they are already sharing checked
+    if ((params.getAttributeOptionCombos() == null || params.getAttributeOptionCombos().isEmpty())
+        && !isSuper)
+      aocAclSql = generateSQlQueryForSharingCheck("co.sharing", currentUser, LIKE_READ_DATA);
     String sql =
         """
       WITH
@@ -177,7 +184,14 @@ ou_with_descendants_ids AS (
         FROM ou_ids
         JOIN organisationunit root USING (organisationunitid)
         JOIN organisationunit ou ON ou.path LIKE root.path || '%'
-      )
+      ),
+      aoc_access AS MATERIALIZED (
+        SELECT aoc.categoryoptioncomboid, aoc.uid
+        FROM categoryoptioncombo aoc
+        WHERE NOT EXISTS (SELECT 1 FROM categoryoptioncombos_categoryoptions coc_co
+        JOIN categoryoption co ON coc_co.categoryoptionid = co.categoryoptionid
+        WHERE coc_co.categoryoptioncomboid = aoc.categoryoptioncomboid AND NOT (:aocAccess))
+      ),
       SELECT
         de.uid AS deid,
         pe.iso,
@@ -201,31 +215,22 @@ JOIN organisationunit root USING (organisationunitid)
       JOIN period pe ON dv.periodid = pe.periodid
       JOIN organisationunit ou ON dv.sourceid = ou.organisationunitid
       JOIN categoryoptioncombo coc ON dv.categoryoptioncomboid = coc.categoryoptioncomboid
+      JOIN aoc_access ON dv.attributeoptioncomboid = aoc_access.categoryoptioncomboid
       JOIN categoryoptioncombo aoc ON dv.attributeoptioncomboid = aoc.categoryoptioncomboid
       WHERE 1=1
         AND coc.uid = ANY(:coc)
         AND aoc.uid = ANY(:aoc)
         AND dv.lastupdated >= :lastUpdated
         AND dv.deleted = :deleted
-        AND ou.hierarchylevel = :level
-        -- access check below must be 1 line for erasure
-        AND NOT EXISTS (SELECT 1 FROM categoryoptioncombos_categoryoptions coc_co \
-          JOIN categoryoption co ON coc_co.categoryoptionid = co.categoryoptionid \
-          WHERE coc_co.categoryoptioncomboid = aoc.categoryoptioncomboid AND NOT (:aocAccess))""";
+        AND ou.hierarchylevel = :level""";
+
     Date lastUpdated = params.getLastUpdated();
     if (lastUpdated == null && params.getLastUpdatedDuration() != null)
       lastUpdated = new Date(currentTimeMillis() - params.getLastUpdatedDuration().toMillis());
 
-    String aocAclSql = null;
-    boolean isSuper = currentUser.isSuper();
-    // explicit AOCs mean they are already sharing checked
-    if ((params.getAttributeOptionCombos() == null || params.getAttributeOptionCombos().isEmpty())
-        && !isSuper)
-      aocAclSql = generateSQlQueryForSharingCheck("co.sharing", currentUser, LIKE_READ_DATA);
-
     boolean descendants = params.isIncludeDescendants();
     List<Order> orders = params.getOrders();
-    if (orders == null || orders.isEmpty()) orders = List.of(Order.PE, Order.CREATED, Order.DE);
+    if (isEmpty(orders)) orders = List.of(Order.PE, Order.CREATED, Order.DE);
 
     List<UID> oug = params.getOrganisationUnitGroups();
     Set<String> ouCapture = currentUser.getUserOrgUnitIds();
@@ -251,6 +256,7 @@ AND NOT EXISTS (SELECT 1 FROM categoryoptioncombos_categoryoptions coc_co \
         .setDynamicClause("aocAccess", aocAclSql)
         .eraseNullParameterLines()
         // keep params below even when null
+        .eraseJoinLine("aoc_access", aocAclSql == null)
         .eraseJoinLine("de_ids", !params.hasDataElementFilters())
         .eraseJoinLine("pe_ids", !params.hasPeriodFilters())
         .eraseJoinLine("ou_with_descendants_ids", !descendants || !params.hasOrgUnitFilters())

dhis-2/dhis-services/dhis-service-core/src/test/java/org/hisp/dhis/datavalue/hibernate/DataExportQueryBuilderTest.java

@@ -32,10 +32,13 @@
 import static org.hisp.dhis.datavalue.DataExportParams.Order.*;
 import static org.hisp.dhis.datavalue.hibernate.HibernateDataExportStore.createExportQuery;
 import static org.hisp.dhis.period.Period.of;
+import static org.junit.jupiter.api.Assertions.assertFalse;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 
 import java.util.Date;
 import java.util.List;
 import java.util.Set;
+import java.util.concurrent.atomic.AtomicReference;
 import org.hisp.dhis.common.UID;
 import org.hisp.dhis.datavalue.DataExportParams;
 import org.hisp.dhis.sql.AbstractQueryBuilderTest;
@@ -436,6 +439,13 @@ ou_ids AS (
             UNION (SELECT ougm.organisationunitid FROM orgunitgroupmembers ougm            JOIN orgunitgroup oug ON ougm.orgunitgroupid = oug.orgunitgroupid            JOIN organisationunit ou ON ougm.organisationunitid = ou.organisationunitid            WHERE oug.uid = ANY(:oug) AND ou.uid = ANY(:capture))
           ) ou_all
           WHERE organisationunitid IS NOT NULL
+        ),
+        aoc_access AS MATERIALIZED (
+          SELECT aoc.categoryoptioncomboid, aoc.uid
+          FROM categoryoptioncombo aoc
+          WHERE NOT EXISTS (SELECT 1 FROM categoryoptioncombos_categoryoptions coc_co
+          JOIN categoryoption co ON coc_co.categoryoptionid = co.categoryoptionid
+          WHERE coc_co.categoryoptioncomboid = aoc.categoryoptioncomboid AND NOT ( ( co.sharing->>'owner' is null or co.sharing->>'owner' = 'null')  or co.sharing->>'public' like '__r_____' or co.sharing->>'public' is null  or (jsonb_has_user_id( co.sharing, 'null') = true  and jsonb_check_user_access( co.sharing, 'null', '__r_____' ) = true )  ))
         )
         SELECT
           de.uid AS deid,
@@ -457,9 +467,9 @@ ou_ids AS (
         JOIN period pe ON dv.periodid = pe.periodid
         JOIN organisationunit ou ON dv.sourceid = ou.organisationunitid
         JOIN categoryoptioncombo coc ON dv.categoryoptioncomboid = coc.categoryoptioncomboid
+        JOIN aoc_access ON dv.attributeoptioncomboid = aoc_access.categoryoptioncomboid
         JOIN categoryoptioncombo aoc ON dv.attributeoptioncomboid = aoc.categoryoptioncomboid
         WHERE dv.deleted = :deleted
-          AND NOT EXISTS (SELECT 1 FROM categoryoptioncombos_categoryoptions coc_co     JOIN categoryoption co ON coc_co.categoryoptionid = co.categoryoptionid     WHERE coc_co.categoryoptioncomboid = aoc.categoryoptioncomboid AND NOT ( ( co.sharing->>'owner' is null or co.sharing->>'owner' = 'null')  or co.sharing->>'public' like '__r_____' or co.sharing->>'public' is null  or (jsonb_has_user_id( co.sharing, 'null') = true  and jsonb_check_user_access( co.sharing, 'null', '__r_____' ) = true )  ))
         ORDER BY pe.startdate, pe.enddate, dv.created, deid""",
         Set.of("oug", "capture", "deleted"),
         createExportQuery(params, createSpyQuery(), currentUser));
@@ -500,4 +510,36 @@ void testFilter_LastUpdated() {
         Set.of("lastUpdated"),
         createExportQuery(params, createSpyQuery(), new SystemUser()));
   }
+
+  @Test
+  void testAocAccess_nonSuperuser_usesMaterializedCte() {
+    UserDetails user = UserDetails.empty().build();
+    DataExportParams params = DataExportParams.builder().includeDeleted(true).build();
+    AtomicReference<String> captured = new AtomicReference<>();
+    SQL.QueryAPI spy = SQL.spy(captured::set, (name, param) -> {});
+    createExportQuery(params, spy, user).stream();
+    String sql = captured.get();
+    assertTrue(sql.contains("aoc_access AS MATERIALIZED"), "expected MATERIALIZED CTE: " + sql);
+    assertTrue(sql.contains("JOIN aoc_access ON"), "expected join to aoc_access CTE: " + sql);
+    assertTrue(
+        sql.contains("JOIN categoryoptioncombo aoc ON"),
+        "expected direct categoryoptioncombo join for aoc alias: " + sql);
+    assertFalse(
+        sql.contains("AND NOT EXISTS"),
+        "NOT EXISTS should not appear inline in WHERE clause: " + sql);
+  }
+
+  @Test
+  void testAocAccess_superuser_usesDirectJoin() {
+    DataExportParams params = DataExportParams.builder().includeDeleted(true).build();
+    AtomicReference<String> captured = new AtomicReference<>();
+    SQL.QueryAPI spy = SQL.spy(captured::set, (name, param) -> {});
+    createExportQuery(params, spy, new SystemUser()).stream();
+    String sql = captured.get();
+    assertFalse(sql.contains("aoc_access"), "expected no aoc_access CTE for superuser: " + sql);
+    assertTrue(
+        sql.contains("JOIN categoryoptioncombo aoc ON"),
+        "expected direct categoryoptioncombo join: " + sql);
+    assertFalse(sql.contains("NOT EXISTS"), "expected no NOT EXISTS correlated subquery: " + sql);
+  }
 }

dhis-2/dhis-test-performance/src/test/java/org/hisp/dhis/test/platform/AggregateDataExportPerformanceTest.java

@@ -0,0 +1,104 @@
+/*
+ * Copyright (c) 2004-2025, University of Oslo
+ * All rights reserved.
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions are met:
+ *
+ * 1. Redistributions of source code must retain the above copyright notice, this
+ * list of conditions and the following disclaimer.
+ *
+ * 2. Redistributions in binary form must reproduce the above copyright notice,
+ * this list of conditions and the following disclaimer in the documentation
+ * and/or other materials provided with the distribution.
+ *
+ * 3. Neither the name of the copyright holder nor the names of its contributors 
+ * may be used to endorse or promote products derived from this software without
+ * specific prior written permission.
+ *
+ * THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" AND
+ * ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
+ * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+ * DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT OWNER OR CONTRIBUTORS BE LIABLE FOR
+ * ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
+ * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND ON
+ * ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF THIS
+ * SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+package org.hisp.dhis.test.platform;
+
+import static io.gatling.javaapi.core.CoreDsl.*;
+import static io.gatling.javaapi.http.HttpDsl.*;
+
+import io.gatling.javaapi.core.ClosedInjectionStep;
+import io.gatling.javaapi.core.ScenarioBuilder;
+import io.gatling.javaapi.core.Simulation;
+import io.gatling.javaapi.http.HttpProtocolBuilder;
+import java.time.LocalDate;
+import java.time.format.DateTimeFormatter;
+
+/**
+ * Performance test for data value set export with org unit descendants (DHIS2-21490).
+ *
+ * <p>Exercises {@code GET /api/dataValueSets/} with {@code children=true}, which activates the
+ * {@code ou_with_descendants_ids} CTE in {@code HibernateDataExportStore}. The original query used
+ * an O(n²) self-join across the entire org unit table; the fix drives from the selected roots and
+ * uses the {@code varchar_pattern_ops} path index to find descendants in O(roots × log n).
+ *
+ * <p>Assumes a Sierra Leone demo database at {@code localhost:8080}. UIDs are stable across demo
+ * database versions:
+ *
+ * <ul>
+ *   <li>Sierra Leone root org unit: {@code ImspTQPwCqd}
+ *   <li>ART monthly summary dataset: {@code lyLU2wR22tC}
+ * </ul>
+ */
+public class AggregateDataExportPerformanceTest extends Simulation {
+
+  private static final String BASE_URL = "http://localhost:8080";
+  private static final String EXPORT_REQUEST = "GET dataValueSets - root OU with descendants";
+
+  @Override
+  public void before() {
+    MetadataImporter.importJsonFile("platform/superuser-data-sl-db.json", "admin", "district");
+  }
+
+  public AggregateDataExportPerformanceTest() {
+    DateTimeFormatter fmt = DateTimeFormatter.ISO_LOCAL_DATE;
+    String endDate = LocalDate.now().format(fmt);
+    String startDate = LocalDate.now().minusYears(2).format(fmt);
+
+    HttpProtocolBuilder httpProtocol =
+        http.baseUrl(BASE_URL)
+            .acceptHeader("application/json")
+            .warmUp(BASE_URL + "/api/ping")
+            .disableCaching()
+            .basicAuth("admin", "district");
+
+    ScenarioBuilder exportScenario =
+        scenario(EXPORT_REQUEST)
+            .group(EXPORT_REQUEST)
+            .on(
+                repeat(1)
+                    .on(
+                        exec(http(EXPORT_REQUEST)
+                                .get("/api/dataValueSets/")
+                                .queryParam("orgUnit", "ImspTQPwCqd")
+                                .queryParam("children", "true")
+                                .queryParam("dataSet", "lyLU2wR22tC")
+                                .queryParam("startDate", startDate)
+                                .queryParam("endDate", endDate))
+                            .pause(1)));
+
+    ClosedInjectionStep closedInjection = rampConcurrentUsers(0).to(5).during(10);
+
+    setUp(exportScenario.injectClosed(closedInjection))
+        .protocols(httpProtocol)
+        .assertions(
+            details(EXPORT_REQUEST).responseTime().percentile(95).lt(300),
+            details(EXPORT_REQUEST).responseTime().max().lt(500),
+            details(EXPORT_REQUEST).successfulRequests().percent().is(100D));
+  }
+}