perf: extract AOC ACL check into MATERIALIZED CTE for data value export [DHIS2-21497]#23896
Merged
jason-p-pickering merged 13 commits intoMay 14, 2026
Merged
Conversation
…21497] Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…[DHIS2-21497] Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…21497-aoc-materialized-cte
…on for AOC CTE [DHIS2-21497] Instead of maintaining a dual-JOIN template and erasing the wrong line at runtime, determine the AOC path (CTE vs direct join) upfront and build the SQL string with String.replace() before handing it to QueryBuilder. Removes the eraseLineContaining / erasedFragments machinery from QueryBuilder that was added specifically to work around the alias-extraction limitation (both JOIN lines extract alias "aoc", making eraseJoinLine unable to distinguish them). The simpler approach: if useAocCte, inject the MATERIALIZED CTE block and JOIN aoc_ids; otherwise inject the direct categoryoptioncombo join. No erasure of AOC-related lines needed at all. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Extracts the multi-line AOC sharing CTE string into a `private static final String AOC_CTE_BLOCK` text block, and uses `CollectionUtils.isEmpty` for the orders null-or-empty check to reduce cognitive complexity from 16 to 15 (Sonar S1764 / S5663). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…arams Moves the SQL template and AOC CTE block construction into a private static `getSql(String aocAclSql)` helper, keeping `createExportQuery` focused on parameter binding and query builder configuration. The WHERE NOT EXISTS subquery is now spread across readable lines inside the text block (test expectation updated to match). Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Per reviewer feedback: the QueryBuilder DSL is designed for one SQL template and one parameter/condition block — splitting the SQL into a helper method works against this design. Inline everything back into createExportQuery. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
jbee
approved these changes
May 13, 2026
|
david-mackessy
approved these changes
May 14, 2026
jason-p-pickering
added a commit
that referenced
this pull request
May 14, 2026
* perf: Adds Gatling performance test for aggregate data export with descendants [DHIS2-21490] (#23895) * perf: add Gatling performance test for aggregate data export with descendants [DHIS2-21490] Adds AggregateDataExportPerformanceTest covering GET /api/dataValueSets/ with children=true against the Sierra Leone demo database — the exact scenario that exposed the O(n²) org unit descendants cross-join fixed in DHIS2-21490. Uses Sierra Leone root org unit and ART monthly summary dataset with a rolling two-year date window, 5 concurrent users over 10 seconds. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com> * Add aggregate data export with descendats performance test --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com> * perf: extract AOC ACL check into MATERIALIZED CTE for data value export [DHIS2-21497] (#23896) --------- Co-authored-by: Claude Sonnet 4.6 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
3 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
GET /api/dataValueSets/ with children=true was slow for non-superusers because PostgreSQL inlined the AOC sharing check (NOT EXISTS over JSONB functions) into the main query. Query analysis showed that the row estimate was 14 (actual: 2,981), so it placed the 225 AOC rows as the outer driver of a nested loop. The resulted in scanning the entire data value table 225 times. 737k buffer hits, ~1,150ms on the Sierra Leone demo DB.
The fix extracts the check into a MATERIALIZED CTE (aoc_access). This forces it to execute once before the main join is planned, giving the planner real cardinality (225 rows) and a hash join instead. 4k buffer hits, ~130ms.
Performance comparison below.
95th Percentile Response Time (p95) (ms)
⬇️ = faster, ⬆️ = slower
Percentiles use the inclusive definition (e.g. p95 = X means 95% of requests responded in X ms or less) and are computed over OK + KO responses.
🤖 AI Assisted