Skip to content

Commit

Permalink
fix: Events query (org units): use IN instead of EQUALS operator in S…
Browse files Browse the repository at this point in the history
…QL [2.38-DHIS2-11085] (#9092)

* fix: Events query (org units): use IN instead of EQUALS operator in SQL [2.38-DHIS2-11085]

* clean coding
  • Loading branch information
d-bernat authored Oct 21, 2021
1 parent 7e05600 commit 46d8f30
Show file tree
Hide file tree
Showing 3 changed files with 44 additions and 21 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@
import static org.hisp.dhis.common.IdentifiableObjectUtils.getUids;
import static org.hisp.dhis.common.QueryOperator.IN;
import static org.hisp.dhis.commons.util.TextUtils.getQuotedCommaDelimitedString;
import static org.hisp.dhis.commons.util.TextUtils.removeLastOr;
import static org.hisp.dhis.feedback.ErrorCode.E7131;
import static org.hisp.dhis.feedback.ErrorCode.E7132;
import static org.hisp.dhis.feedback.ErrorCode.E7133;
Expand All @@ -49,6 +48,7 @@

import java.util.Date;
import java.util.List;
import java.util.Map;
import java.util.stream.Collectors;

import lombok.extern.slf4j.Slf4j;
Expand Down Expand Up @@ -109,6 +109,8 @@ public class JdbcEventAnalyticsManager
{
protected static final String OPEN_IN = " in (";

private static final String ORG_UNIT_UID_LEVEL_COLUMN_PREFIX = "uidlevel";

private final EventTimeFieldSqlRenderer timeFieldSqlRenderer;

public JdbcEventAnalyticsManager( JdbcTemplate jdbcTemplate, StatementBuilder statementBuilder,
Expand Down Expand Up @@ -421,18 +423,13 @@ else if ( params.isOrganisationUnitMode( OrganisationUnitSelectionMode.CHILDREN
{
String orgUnitAlias = getOrgUnitAlias( params );

sql += hlp.whereAnd() + " (";
String sqlSnippet = getOrgDescendantsSqlSnippet( orgUnitAlias,
params.getDimensionOrFilterItems( ORGUNIT_DIM_ID ) );

for ( DimensionalItemObject object : params.getDimensionOrFilterItems( ORGUNIT_DIM_ID ) )
if ( sqlSnippet != null && !sqlSnippet.trim().isEmpty() )
{
OrganisationUnit unit = (OrganisationUnit) object;

String orgUnitCol = quote( orgUnitAlias, "uidlevel" + unit.getLevel() );

sql += orgUnitCol + " = '" + unit.getUid() + "' or ";
sql += hlp.whereAnd() + " " + sqlSnippet;
}

sql = removeLastOr( sql ) + ") ";
}

// ---------------------------------------------------------------------
Expand Down Expand Up @@ -589,6 +586,32 @@ else if ( params.isOrganisationUnitMode( OrganisationUnitSelectionMode.CHILDREN
return sql;
}

/**
* Generates a sub query which provides a filter by organisation -
* descendant level
*/
private String getOrgDescendantsSqlSnippet( String orgUnitAlias,
List<DimensionalItemObject> dimensionOrFilterItems )
{
Map<String, List<OrganisationUnit>> collect = dimensionOrFilterItems.stream()
.map( object -> (OrganisationUnit) object )
.collect( Collectors.groupingBy(
unit -> quote( orgUnitAlias, ORG_UNIT_UID_LEVEL_COLUMN_PREFIX + unit.getLevel() ) ) );

return collect.keySet()
.stream()
.map( org -> toInCondition( org, collect.get( org ) ) )
.collect( Collectors.joining( " and " ) );
}

private String toInCondition( String org, List<OrganisationUnit> organisationUnits )
{
return organisationUnits.stream()
.filter( unit -> unit.getUid() != null && !unit.getUid().trim().isEmpty() )
.map( unit -> "'" + unit.getUid() + "'" )
.collect( Collectors.joining( ",", org + OPEN_IN, ") " ) );
}

/**
* Generates a sub query which provides a view of the data where each row is
* ranked by the execution date, latest first. The events are partitioned by
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -371,6 +371,6 @@ public void testGetWhereClauseWithMultipleOrgUnitDescendantsAtSameLevel()
// Then
assertThat( whereClause,
containsString(
"and (ax.\"uidlevel0\" = 'ouabcdefghA' or ax.\"uidlevel0\" = 'ouabcdefghB' or ax.\"uidlevel0\" = 'ouabcdefghC' )" ) );
"and ax.\"uidlevel0\" in ('ouabcdefghA','ouabcdefghB','ouabcdefghC')" ) );
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ public void verifyGetEventSqlWithProgramWithNoRegistration()

String expected = "select psi,ps,executiondate,storedby,lastupdated,ST_AsGeoJSON(psigeometry, 6) as geometry,longitude,latitude,ouname,oucode,ax.\"monthly\",ax.\"ou\" from "
+ getTable( programA.getUid() )
+ " as ax where ax.\"monthly\" in ('2000Q1') and (ax.\"uidlevel1\" = 'ouabcdefghA' ) limit 101";
+ " as ax where ax.\"monthly\" in ('2000Q1') and ax.\"uidlevel1\" in ('ouabcdefghA') limit 101";

assertThat( sql.getValue(), is( expected ) );
}
Expand All @@ -163,7 +163,7 @@ public void verifyGetEventSqlWithOrgUnitTypeDataElement()
"as geometry,longitude,latitude,ouname,oucode,ax.\"monthly\",ax.\"ou\",\"" + dataElement.getUid() + "_name"
+ "\" " +
"from " + getTable( programA.getUid() )
+ " as ax where ax.\"monthly\" in ('2000Q1') and (ax.\"uidlevel1\" = 'ouabcdefghA' ) limit 101";
+ " as ax where ax.\"monthly\" in ('2000Q1') and ax.\"uidlevel1\" in ('ouabcdefghA') limit 101";

assertThat( sql.getValue(), is( expected ) );
}
Expand All @@ -178,7 +178,7 @@ public void verifyGetEventSqlWithProgram()
verify( jdbcTemplate ).queryForRowSet( sql.capture() );

String expected = "ax.\"monthly\",ax.\"ou\" from " + getTable( programA.getUid() )
+ " as ax where ax.\"monthly\" in ('2000Q1') and (ax.\"uidlevel1\" = 'ouabcdefghA' ) limit 101";
+ " as ax where ax.\"monthly\" in ('2000Q1') and ax.\"uidlevel1\" in ('ouabcdefghA') limit 101";

assertSql( expected, sql.getValue() );
}
Expand All @@ -194,7 +194,7 @@ public void verifyGetEventsSqlWithProgramAndProgramStage()
verify( jdbcTemplate ).queryForRowSet( sql.capture() );

String expected = "ax.\"monthly\",ax.\"ou\" from " + getTable( programA.getUid() )
+ " as ax where ax.\"monthly\" in ('2000Q1') and (ax.\"uidlevel1\" = 'ouabcdefghA' ) and ax.\"ps\" = '"
+ " as ax where ax.\"monthly\" in ('2000Q1') and ax.\"uidlevel1\" in ('ouabcdefghA') and ax.\"ps\" = '"
+ programStage.getUid() + "' limit 101";

assertSql( expected, sql.getValue() );
Expand All @@ -211,7 +211,7 @@ public void verifyGetEventsWithProgramStageAndNumericDataElement()
verify( jdbcTemplate ).queryForRowSet( sql.capture() );

String expected = "ax.\"monthly\",ax.\"ou\",ax.\"fWIAEtYVEGk\" from " + getTable( programA.getUid() )
+ " as ax where ax.\"monthly\" in ('2000Q1') and (ax.\"uidlevel1\" = 'ouabcdefghA' ) and ax.\"ps\" = '"
+ " as ax where ax.\"monthly\" in ('2000Q1') and ax.\"uidlevel1\" in ('ouabcdefghA') and ax.\"ps\" = '"
+ programStage.getUid() + "' limit 101";

assertSql( expected, sql.getValue() );
Expand All @@ -228,7 +228,7 @@ public void verifyGetEventsWithProgramStageAndNumericDataElementAndFilter()
verify( jdbcTemplate ).queryForRowSet( sql.capture() );

String expected = "ax.\"monthly\",ax.\"ou\",ax.\"fWIAEtYVEGk\" from " + getTable( programA.getUid() )
+ " as ax where ax.\"monthly\" in ('2000Q1') and (ax.\"uidlevel1\" = 'ouabcdefghA' ) and ax.\"ps\" = '"
+ " as ax where ax.\"monthly\" in ('2000Q1') and ax.\"uidlevel1\" in ('ouabcdefghA') and ax.\"ps\" = '"
+ programStage.getUid() + "' and ax.\"fWIAEtYVEGk\" > '10' limit 101";

assertSql( expected, sql.getValue() );
Expand Down Expand Up @@ -307,7 +307,7 @@ public void verifyGetEventsWithProgramStageAndTextDataElement()
verify( jdbcTemplate ).queryForRowSet( sql.capture() );

String expected = "ax.\"monthly\",ax.\"ou\",ax.\"fWIAEtYVEGk\" from " + getTable( programA.getUid() )
+ " as ax where ax.\"monthly\" in ('2000Q1') and (ax.\"uidlevel1\" = 'ouabcdefghA' ) and ax.\"ps\" = '"
+ " as ax where ax.\"monthly\" in ('2000Q1') and ax.\"uidlevel1\" in ('ouabcdefghA') and ax.\"ps\" = '"
+ programStage.getUid() + "' limit 101";

assertSql( expected, sql.getValue() );
Expand All @@ -323,7 +323,7 @@ public void verifyGetEventsWithProgramStageAndTextDataElementAndFilter()
verify( jdbcTemplate ).queryForRowSet( sql.capture() );

String expected = "ax.\"monthly\",ax.\"ou\",ax.\"fWIAEtYVEGk\" from " + getTable( programA.getUid() )
+ " as ax where ax.\"monthly\" in ('2000Q1') and (ax.\"uidlevel1\" = 'ouabcdefghA' ) and ax.\"ps\" = '"
+ " as ax where ax.\"monthly\" in ('2000Q1') and ax.\"uidlevel1\" in ('ouabcdefghA') and ax.\"ps\" = '"
+ programStage.getUid() + "' and lower(ax.\"fWIAEtYVEGk\") > '10' limit 101";

assertSql( expected, sql.getValue() );
Expand Down Expand Up @@ -351,7 +351,7 @@ public void verifyGetAggregatedEventQuery()

String expected = "select count(ax.\"psi\") as value,ax.\"monthly\",ax.\"ou\",ax.\"fWIAEtYVEGk\" from "
+ getTable( programA.getUid() )
+ " as ax where ax.\"monthly\" in ('2000Q1') and (ax.\"uidlevel1\" = 'ouabcdefghA' ) and ax.\"ps\" = '"
+ " as ax where ax.\"monthly\" in ('2000Q1') and ax.\"uidlevel1\" in ('ouabcdefghA') and ax.\"ps\" = '"
+ programStage.getUid() + "' group by ax.\"monthly\",ax.\"ou\",ax.\"fWIAEtYVEGk\" limit 200001";

assertThat( sql.getValue(), is( expected ) );
Expand Down Expand Up @@ -379,7 +379,7 @@ public void verifyGetAggregatedEventQueryWithFilter()
verify( jdbcTemplate ).queryForRowSet( sql.capture() );
String expected = "select count(ax.\"psi\") as value,ax.\"monthly\",ax.\"ou\",ax.\"fWIAEtYVEGk\" from "
+ getTable( programA.getUid() )
+ " as ax where ax.\"monthly\" in ('2000Q1') and (ax.\"uidlevel1\" = 'ouabcdefghA' ) and ax.\"ps\" = '"
+ " as ax where ax.\"monthly\" in ('2000Q1') and ax.\"uidlevel1\" in ('ouabcdefghA') and ax.\"ps\" = '"
+ programStage.getUid()
+ "' and lower(ax.\"fWIAEtYVEGk\") > '10' group by ax.\"monthly\",ax.\"ou\",ax.\"fWIAEtYVEGk\" limit 200001";
assertThat( sql.getValue(), is( expected ) );
Expand Down

0 comments on commit 46d8f30

Please sign in to comment.