Skip to content

Commit

Permalink
fix: Order by occurreddate and created[2.42-DHIS-16709] (#16809)
Browse files Browse the repository at this point in the history
  • Loading branch information
@d-bernat
d-bernat authored Mar 18, 2024
1 parent e79b3e1 commit a2c3124
Show file tree
Hide file tree
Showing 6 changed files with 73 additions and 66 deletions.
47 changes: 34 additions & 13 deletions ...tics/src/main/java/org/hisp/dhis/analytics/event/data/JdbcEnrollmentAnalyticsManager.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,10 @@ public class JdbcEnrollmentAnalyticsManager extends AbstractJdbcEventAnalyticsMa

private static final String ANALYTICS_EVENT = "analytics_event_";

private static final String ORDER_BY_EXECUTION_DATE = "order by occurreddate, created ";
private static final String DIRECTION_PLACEHOLDER = "#DIRECTION_PLACEHOLDER";

private static final String ORDER_BY_EXECUTION_DATE =
"order by occurreddate " + DIRECTION_PLACEHOLDER + ", created " + DIRECTION_PLACEHOLDER;

private static final String LIMIT_1 = "limit 1";

Expand Down Expand Up @@ -519,8 +522,10 @@ protected ColumnAndAlias getCoordinateColumn(QueryItem item, String suffix) {
+ colName
+ IS_NOT_NULL
+ psCondition
+ ORDER_BY_EXECUTION_DATE
+ createOrderTypeAndOffset(item.getProgramStageOffset())
+ " "
+ createOrderType(item.getProgramStageOffset())
+ " "
+ createOffset(item.getProgramStageOffset())
+ " "
+ LIMIT_1
+ " )",
Expand Down Expand Up @@ -571,8 +576,10 @@ protected String getColumn(QueryItem item, String suffix) {
+ getExecutionDateFilter(
item.getRepeatableStageParams().getStartDate(),
item.getRepeatableStageParams().getEndDate())
+ ORDER_BY_EXECUTION_DATE
+ createOrderTypeAndOffset(item.getProgramStageOffset())
+ createOrderType(item.getProgramStageOffset())
+ " "
+ createOffset(item.getProgramStageOffset())
+ " "
+ getLimit(item.getRepeatableStageParams().getCount())
+ " ) as t1)";
}
Expand All @@ -593,8 +600,9 @@ protected String getColumn(QueryItem item, String suffix) {
+ getExecutionDateFilter(
item.getRepeatableStageParams().getStartDate(),
item.getRepeatableStageParams().getEndDate())
+ ORDER_BY_EXECUTION_DATE
+ createOrderTypeAndOffset(item.getProgramStageOffset())
+ createOrderType(item.getProgramStageOffset())
+ " "
+ createOffset(item.getProgramStageOffset())
+ " "
+ LIMIT_1
+ " )";
Expand All @@ -620,8 +628,9 @@ protected String getColumn(QueryItem item, String suffix) {
+ "and ps = '"
+ item.getProgramStage().getUid()
+ "' "
+ ORDER_BY_EXECUTION_DATE
+ createOrderTypeAndOffset(item.getProgramStageOffset())
+ createOrderType(item.getProgramStageOffset())
+ " "
+ createOffset(item.getProgramStageOffset())
+ " "
+ LIMIT_1
+ " )";
Expand Down Expand Up @@ -677,14 +686,26 @@ protected AnalyticsType getAnalyticsType() {
return AnalyticsType.ENROLLMENT;
}

private String createOrderTypeAndOffset(int offset) {
private String createOffset(int offset) {
if (offset == 0) {
return EMPTY;
}

if (offset < 0) {
return "offset " + (-1 * offset);
} else {
return "offset " + (offset - 1);
}
}

private String createOrderType(int offset) {
if (offset == 0) {
return "desc";
return ORDER_BY_EXECUTION_DATE.replace(DIRECTION_PLACEHOLDER, "desc");
}
if (offset < 0) {
return "desc offset " + (-1 * offset);
return ORDER_BY_EXECUTION_DATE.replace(DIRECTION_PLACEHOLDER, "desc");
} else {
return "asc offset " + (offset - 1);
return ORDER_BY_EXECUTION_DATE.replace(DIRECTION_PLACEHOLDER, "asc");
}
}
}
7 changes: 5 additions & 2 deletions .../src/main/java/org/hisp/dhis/analytics/tei/query/context/querybuilder/SqlQueryHelper.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,11 +77,14 @@ static String eventSelect(
TrackedEntityType trackedEntityType,
SqlParameterManager sqlParameterManager) {
int offset = programStage.getOffsetWithDefault();
String orderByDirection = offset > 0 ? "asc" : "desc";

return "select innermost_evt.*"
+ " from (select *,"
+ " row_number() over (partition by programinstanceuid order by occurreddate, created "
+ (offset > 0 ? "asc" : "desc")
+ " row_number() over (partition by programinstanceuid order by occurreddate "
+ orderByDirection
+ ", created "
+ orderByDirection
+ " ) as rn"
+ " from "
+ ANALYTICS_TEI_EVT
Expand Down
36 changes: 18 additions & 18 deletions ...tics/src/test/java/org/hisp/dhis/analytics/event/data/EnrollmentAnalyticsManagerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -216,7 +216,7 @@ private void verifyWithProgramStageAndDataElement(ValueType valueType) {
+ programA.getUid()
+ ".pi = ax.pi and \"fWIAEtYVEGk\" is not null and ps = '"
+ programStage.getUid()
+ "' order by occurreddate, created desc limit 1 )";
+ "' order by occurreddate desc, created desc limit 1 )";

if (valueType == ValueType.NUMBER) {
subSelect = subSelect + " as \"fWIAEtYVEGk\"";
Expand Down Expand Up @@ -259,7 +259,7 @@ private void verifyWithRepeatableProgramStageAndDataElement(ValueType valueType)
+ programUid
+ ".pi = ax.pi and ps = '"
+ repeatableProgramStage.getUid()
+ "' order by occurreddate, created desc offset 1 limit 1 ) "
+ "' order by occurreddate desc, created desc offset 1 limit 1 ) "
+ "as \""
+ programStageUid
+ "[-1]."
Expand All @@ -274,7 +274,7 @@ private void verifyWithRepeatableProgramStageAndDataElement(ValueType valueType)
+ programUid
+ ".pi = ax.pi and ps = '"
+ programStageUid
+ "' order by occurreddate, created desc offset 1 limit 1 )) "
+ "' order by occurreddate desc, created desc offset 1 limit 1 )) "
+ "as \""
+ programStageUid
+ "[-1]."
Expand Down Expand Up @@ -305,7 +305,7 @@ void verifyWithProgramStageAndTextualDataElementAndFilter() {
+ programA.getUid()
+ ".pi = ax.pi and \"fWIAEtYVEGk\" is not null and ps = '"
+ programStage.getUid()
+ "' order by occurreddate, created desc limit 1 )";
+ "' order by occurreddate desc, created desc limit 1 )";

String expected =
"ax.\"quarterly\",ax.\"ou\","
Expand Down Expand Up @@ -379,7 +379,7 @@ void verifyWithProgramStageAndNumericDataElementAndFilter2() {
+ programA.getUid()
+ ".pi = ax.pi and \"fWIAEtYVEGk\" is not null and ps = '"
+ programStage.getUid()
+ "' order by occurreddate, created desc limit 1 )";
+ "' order by occurreddate desc, created desc limit 1 )";

String expected =
"ax.\"quarterly\",ax.\"ou\","
Expand All @@ -406,7 +406,7 @@ void verifyGetEnrollmentsWithMissingValueEqFilter() {
+ programA.getUid()
+ ".pi = ax.pi and \"fWIAEtYVEGk\" is not null and ps = '"
+ programStage.getUid()
+ "' order by occurreddate, created desc limit 1 )";
+ "' order by occurreddate desc, created desc limit 1 )";

String expected = subSelect + " is null";

Expand All @@ -425,7 +425,7 @@ void verifyGetEnrollmentsWithMissingValueNeqFilter() {
+ programA.getUid()
+ ".pi = ax.pi and \"fWIAEtYVEGk\" is not null and ps = '"
+ programStage.getUid()
+ "' order by occurreddate, created desc limit 1 )";
+ "' order by occurreddate desc, created desc limit 1 )";

String expected = subSelect + " is not null";
testIt(
Expand All @@ -443,7 +443,7 @@ void verifyGetEnrollmentsWithMissingValueAndNumericValuesInFilter() {
+ programA.getUid()
+ ".pi = ax.pi and \"fWIAEtYVEGk\" is not null and ps = '"
+ programStage.getUid()
+ "' order by occurreddate, created desc limit 1 )";
+ "' order by occurreddate desc, created desc limit 1 )";

String numericValues = String.join(OPTION_SEP, "10", "11", "12");
String expected =
Expand Down Expand Up @@ -471,7 +471,7 @@ void verifyGetEnrollmentsWithoutMissingValueAndNumericValuesInFilter() {
+ programA.getUid()
+ ".pi = ax.pi and \"fWIAEtYVEGk\" is not null and ps = '"
+ programStage.getUid()
+ "' order by occurreddate, created desc limit 1 )";
+ "' order by occurreddate desc, created desc limit 1 )";

String numericValues = String.join(OPTION_SEP, "10", "11", "12");
String expected = subSelect + " in (" + String.join(",", numericValues.split(OPTION_SEP)) + ")";
Expand All @@ -490,7 +490,7 @@ void verifyGetEnrollmentsWithOnlyMissingValueInFilter() {
+ programA.getUid()
+ ".pi = ax.pi and \"fWIAEtYVEGk\" is not null and ps = '"
+ programStage.getUid()
+ "' order by occurreddate, created desc limit 1 )";
+ "' order by occurreddate desc, created desc limit 1 )";

String expected = subSelect + " is null";
String unexpected = "(" + subSelect + " in (";
Expand Down Expand Up @@ -722,7 +722,7 @@ void verifyGetColumnOfTypeCoordinateAndWithProgramStages() {
+ dataElementA.getUid()
+ "\" is not null and ps = '"
+ programStage.getUid()
+ "' order by occurreddate, created desc limit 1 )"));
+ "' order by occurreddate desc, created desc limit 1 )"));
}

@Test
Expand Down Expand Up @@ -754,7 +754,7 @@ void verifyGetColumnOfTypeCoordinateAndWithProgramStagesAndParamsWithReferenceTy
+ programB.getUid()
+ ".pi = ax.pi and ps = '"
+ repeatableProgramStage.getUid()
+ "' and occurreddate >= '2022-01-01' and occurreddate <= '2022-01-31' order by occurreddate, created desc LIMIT 100 ) as t1)"));
+ "' and occurreddate >= '2022-01-01' and occurreddate <= '2022-01-31' order by occurreddate desc, created desc LIMIT 100 ) as t1)"));
}

@Test
Expand Down Expand Up @@ -784,7 +784,7 @@ void verifyGetColumnOfTypeCoordinateAndWithProgramStagesAndParamsWithNumberTypeV
+ programB.getUid()
+ ".pi = ax.pi and ps = '"
+ repeatableProgramStage.getUid()
+ "' order by occurreddate, created desc limit 1 )"));
+ "' order by occurreddate desc, created desc limit 1 )"));
}

@Test
Expand Down Expand Up @@ -819,9 +819,9 @@ void verifyGetCoordinateColumnAndNoProgramStage() {
+ ".pi "
+ "and "
+ colName
+ " is not null "
+ "order by occurreddate, created "
+ "desc limit 1 )"));
+ " is not null "
+ "order by occurreddate desc, created "
+ "desc limit 1 )"));
}

@Test
Expand Down Expand Up @@ -860,8 +860,8 @@ void verifyGetCoordinateColumnWithProgramStage() {
+ " is not null "
+ "and ps = '"
+ item.getProgramStage().getUid()
+ "' order by occurreddate, created "
+ "desc limit 1 )"));
+ "' order by occurreddate desc, created "
+ "desc limit 1 )"));
}

@Test
Expand Down
6 changes: 3 additions & 3 deletions .../test/java/org/hisp/dhis/analytics/tei/query/context/querybuilder/SqlQueryHelperTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ void testEventSelectZeroOffset() {

// Then
assertEquals(
"select innermost_evt.* from (select *, row_number() over (partition by programinstanceuid order by occurreddate, created desc ) as rn from analytics_tei_events_uid2 where programuid = :1 and programstageuid = :2) innermost_evt where innermost_evt.rn = 1",
"select innermost_evt.* from (select *, row_number() over (partition by programinstanceuid order by occurreddate desc, created desc ) as rn from analytics_tei_events_uid2 where programuid = :1 and programstageuid = :2) innermost_evt where innermost_evt.rn = 1",
statement);
}

Expand Down Expand Up @@ -172,7 +172,7 @@ void testEventSelectPositiveOffset() {

// Then
assertEquals(
"select innermost_evt.* from (select *, row_number() over (partition by programinstanceuid order by occurreddate, created asc ) as rn from analytics_tei_events_uid2 where programuid = :1 and programstageuid = :2) innermost_evt where innermost_evt.rn = 2",
"select innermost_evt.* from (select *, row_number() over (partition by programinstanceuid order by occurreddate asc, created asc ) as rn from analytics_tei_events_uid2 where programuid = :1 and programstageuid = :2) innermost_evt where innermost_evt.rn = 2",
statement);
}

Expand Down Expand Up @@ -203,7 +203,7 @@ void testEventSelectNegativeOffset() {

// Then
assertEquals(
"select innermost_evt.* from (select *, row_number() over (partition by programinstanceuid order by occurreddate, created desc ) as rn from analytics_tei_events_uid2 where programuid = :1 and programstageuid = :2) innermost_evt where innermost_evt.rn = 4",
"select innermost_evt.* from (select *, row_number() over (partition by programinstanceuid order by occurreddate desc, created desc ) as rn from analytics_tei_events_uid2 where programuid = :1 and programstageuid = :2) innermost_evt where innermost_evt.rn = 4",
statement);
}
}
8 changes: 6 additions & 2 deletions ...2/dhis-test-e2e/src/test/java/org/hisp/dhis/analytics/enrollment/EnrollmentQueryTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -291,10 +291,14 @@ public void queryWithProgramAndRepeatableProgramStage() {
validateRow(
response,
0,
List.of("XLiqwElsFHO", "Sierra Leone / Port Loko / Koya / Kissy Koya MCHP", "25.0", ""));
List.of("fmkqsEx6MRo", "Sierra Leone / Port Loko / Koya / Mabora MCHP", "25.0", ""));
validateRow(
response,
1,
List.of("O63vIA5MVn6", "Sierra Leone / Port Loko / Kaffu Bullom / Tagrin CHC", "25.0", ""));
List.of(
"GCbYmPqcOOP",
"Sierra Leone / Port Loko / Bureh Kasseh Maconteh / Romeni MCHP",
"25.0",
""));
}
}
Loading

0 comments on commit a2c3124

Please sign in to comment.