Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: do not ignore measure criteria in aggregated analytics query [DHIS2-18788] #19986

Merged
merged 9 commits into from
Feb 24, 2025

Conversation

luciano-fiandesio
Copy link
Contributor

@luciano-fiandesio luciano-fiandesio commented Feb 19, 2025

Summary

This PR fixes a bug with aggregated analytics query containing the measureCriteria parameter. The parameter is ignored and the filtering condition is not applied to the final SQL query.

Problem

Given this query params:

/analytics?dimension=dx:GSae40Fyppf,pe:LAST_10_YEARS;LAST_12_MONTHS
&filter=ou:USER_ORGUNIT
&measureCriteria=GT:15;LE:21 <- measure criteria

The resulting SQL aggregated query does not contain the measuring criteria

select
	avg((date_part('year', age(cast(occurreddate as date), cast("iESIqZ0R0R0" as date))))) as value,
	ax."monthly"
from
	analytics_event_uy2gu8kt1jf as ax
where
	(ax."monthly" in ('202402', '202403', '202404', '202405', '202406', '202407', '202408', '202409', '202410', '202411', '202412', '202501') )
	and ax."uidlevel1" in ('ImspTQPwCqd')
	and ("iESIqZ0R0R0" is not null)
	and ax."yearly" in ('2025', '2024')
group by
	ax."monthly"

Fix

  • Propagate the original measureFilter data from DataQueryParams to EventQueryParams
  • Add the having clause to the query whenever measureFilter is present in EventQueryParams.
  • Cast the expression to Numeric and round it, otherwise the comparison may fail due to floating point precision.
  • Added 3 e2e test cases to verify various scenario

Fixed SQL aggregated query

select
	avg((date_part('year', age(cast(occurreddate as date), cast("iESIqZ0R0R0" as date))))) as value,
	ax."monthly"
from
	analytics_event_uy2gu8kt1jf as ax
where
	(ax."monthly" in ('202402', '202403', '202404', '202405', '202406', '202407', '202408', '202409', '202410', '202411', '202412', '202501') )
	and ax."uidlevel1" in ('ImspTQPwCqd')
	and ("iESIqZ0R0R0" is not null)
	and ax."yearly" in ('2025', '2024')
group by
	ax."monthly"
having round(avg(date_part('year', age(cast(occurreddate as date), cast("iESIqZ0R0R0" as date))))::NUMERIC,10) > 15

@luciano-fiandesio luciano-fiandesio force-pushed the DHIS2-18788_MEASURE_CRITERIA_FIX branch from d9ee561 to afaaeb4 Compare February 19, 2025 13:52
@luciano-fiandesio luciano-fiandesio added the run-api-analytics-tests Enables analytics e2e tests label Feb 19, 2025
Copy link
Contributor

@maikelarabori maikelarabori left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We can also add an e2e, we can chat later on

@luciano-fiandesio luciano-fiandesio force-pushed the DHIS2-18788_MEASURE_CRITERIA_FIX branch from 1d8667f to 1d2ca53 Compare February 20, 2025 13:38
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
run-api-analytics-tests Enables analytics e2e tests
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants