Skip to content

Commit

Permalink
refactor: Join attribute value table for attributes [DHIS2-18417] (#1…
Browse files Browse the repository at this point in the history
  • Loading branch information
larshelge authored Dec 4, 2024
1 parent 3784ef6 commit 8fcf3bf
Show file tree
Hide file tree
Showing 6 changed files with 73 additions and 57 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,7 @@
import java.util.ArrayList;
import java.util.List;
import java.util.Map;
import java.util.stream.Collectors;
import org.hisp.dhis.analytics.AnalyticsTableHookService;
import org.hisp.dhis.analytics.partition.PartitionManager;
import org.hisp.dhis.analytics.table.model.AnalyticsDimensionType;
Expand All @@ -46,6 +47,7 @@
import org.hisp.dhis.analytics.table.model.Skip;
import org.hisp.dhis.analytics.table.setting.AnalyticsTableSettings;
import org.hisp.dhis.category.CategoryService;
import org.hisp.dhis.common.IdentifiableObject;
import org.hisp.dhis.common.IdentifiableObjectManager;
import org.hisp.dhis.common.ValueType;
import org.hisp.dhis.commons.util.TextUtils;
Expand All @@ -55,6 +57,7 @@
import org.hisp.dhis.db.sql.SqlBuilder;
import org.hisp.dhis.organisationunit.OrganisationUnitService;
import org.hisp.dhis.period.PeriodDataProvider;
import org.hisp.dhis.program.Program;
import org.hisp.dhis.resourcetable.ResourceTableService;
import org.hisp.dhis.setting.SystemSettingsProvider;
import org.hisp.dhis.trackedentity.TrackedEntityAttribute;
Expand Down Expand Up @@ -117,14 +120,13 @@ protected Skip skipIndex(ValueType valueType, boolean hasOptionSet) {
}

/**
* Returns a select expression, potentially with a cast statement, based on the given value type.
* Handles data element and tracked entity attribute select expressions.
* Returns a column expression, potentially with a cast statement, based on the given value type.
*
* @param valueType the {@link ValueType} to represent as database column type.
* @param columnExpression the expression or name of the column to be selected.
* @return a select expression appropriate for the given value type and context.
*/
protected String getSelectExpression(ValueType valueType, String columnExpression) {
protected String getColumnExpression(ValueType valueType, String columnExpression) {
if (valueType.isDecimal()) {
return getCastExpression(columnExpression, NUMERIC_REGEXP, sqlBuilder.dataTypeDouble());
} else if (valueType.isInteger()) {
Expand All @@ -138,7 +140,8 @@ protected String getSelectExpression(ValueType valueType, String columnExpressio
} else if (valueType.isGeo() && isSpatialSupport()) {
return String.format(
"""
ST_GeomFromGeoJSON('{"type":"Point", "coordinates":' || (%s) || ', "crs":{"type":"name", "properties":{"name":"EPSG:4326"}}}')""",
ST_GeomFromGeoJSON('{"type":"Point", "coordinates":' || (%s) || \
', "crs":{"type":"name", "properties":{"name":"EPSG:4326"}}}')""",
columnExpression);
} else {
return columnExpression;
Expand Down Expand Up @@ -219,22 +222,22 @@ protected void populateTableInternal(AnalyticsTablePartition partition, String f
protected List<AnalyticsTableColumn> getColumnForAttribute(TrackedEntityAttribute attribute) {
List<AnalyticsTableColumn> columns = new ArrayList<>();

String valueColumn = String.format("%s.%s", quote(attribute.getUid()), "value");
DataType dataType = getColumnType(attribute.getValueType(), isSpatialSupport());
String selectExpression = getSelectExpression(attribute.getValueType(), "value");
String selectExpression = getColumnExpression(attribute.getValueType(), valueColumn);
String dataFilterClause = getDataFilterClause(attribute);
String sql = getSelectSubquery(attribute, selectExpression, dataFilterClause);
Skip skipIndex = skipIndex(attribute.getValueType(), attribute.hasOptionSet());

if (attribute.getValueType().isOrganisationUnit()) {
columns.addAll(getColumnForOrgUnitTrackedEntityAttribute(attribute, dataFilterClause));
columns.addAll(getColumnForOrgUnitAttribute(attribute, dataFilterClause));
}

columns.add(
AnalyticsTableColumn.builder()
.name(attribute.getUid())
.dimensionType(AnalyticsDimensionType.DYNAMIC)
.dataType(dataType)
.selectExpression(sql)
.selectExpression(selectExpression)
.skipIndex(skipIndex)
.build());

Expand All @@ -248,7 +251,7 @@ protected List<AnalyticsTableColumn> getColumnForAttribute(TrackedEntityAttribut
* @param dataFilterClause the data filter clause.
* @return a list of {@link AnalyticsTableColumn}.
*/
private List<AnalyticsTableColumn> getColumnForOrgUnitTrackedEntityAttribute(
private List<AnalyticsTableColumn> getColumnForOrgUnitAttribute(
TrackedEntityAttribute attribute, String dataFilterClause) {
List<AnalyticsTableColumn> columns = new ArrayList<>();

Expand Down Expand Up @@ -306,4 +309,34 @@ private String getSelectSubquery(
"closingParentheses", getClosingParentheses(selectExpression),
"attributeUid", quote(attribute.getUid())));
}

/**
* Returns a join clause for attribute value for every attribute of the given program.
*
* @param program the {@link Program}.
* @return a join clause.
*/
protected String getAttributeValueJoinClause(Program program) {
String template =
"""
left join ${trackedentityattributevalue} as ${uid} \
on en.trackedentityid=${uid}.trackedentityid \
and ${uid}.trackedentityattributeid = ${id}\s""";

return program.getNonConfidentialTrackedEntityAttributes().stream()
.map(attribute -> replaceQualify(template, toVariableMap(attribute)))
.collect(Collectors.joining());
}

/**
* Returns a map of identifiable properties and values.
*
* @param object the {@link IdentifiableObject}.
* @return a {@link Map}.
*/
protected Map<String, String> toVariableMap(IdentifiableObject object) {
return Map.of(
"id", String.valueOf(object.getId()),
"uid", quote(object.getUid()));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -136,6 +136,7 @@ protected List<String> getPartitionChecks(Integer year, Date endDate) {
@Override
public void populateTable(AnalyticsTableUpdateParams params, AnalyticsTablePartition partition) {
Program program = partition.getMasterTable().getProgram();
String attributeJoinClause = getAttributeValueJoinClause(program);

String fromClause =
replaceQualify(
Expand All @@ -148,13 +149,15 @@ public void populateTable(AnalyticsTableUpdateParams params, AnalyticsTableParti
left join analytics_rs_dateperiodstructure dps on cast(en.enrollmentdate as date)=dps.dateperiod \
left join analytics_rs_orgunitstructure ous on en.organisationunitid=ous.organisationunitid \
left join analytics_rs_organisationunitgroupsetstructure ougs on en.organisationunitid=ougs.organisationunitid \
${attributeJoinClause}\
where pr.programid=${programId} \
and en.organisationunitid is not null \
and (ougs.startdate is null or dps.monthstartdate=ougs.startdate) \
and en.lastupdated <= '${startTime}' \
and en.occurreddate is not null \
and en.deleted = false\s""",
Map.of(
"attributeJoinClause", attributeJoinClause,
"programId", String.valueOf(program.getId()),
"startTime", toLongDate(params.getStartTime())));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -328,6 +328,7 @@ public void populateTable(AnalyticsTableUpdateParams params, AnalyticsTableParti
Integer latestDataYear = availableDataYears.get(availableDataYears.size() - 1);
Program program = partition.getMasterTable().getProgram();
String partitionClause = getPartitionClause(partition);
String attributeJoinClause = getAttributeValueJoinClause(program);

String fromClause =
replaceQualify(
Expand All @@ -344,6 +345,7 @@ left join analytics_rs_dateperiodstructure dps on cast(${eventDateExpression} as
left join analytics_rs_organisationunitgroupsetstructure ougs on ev.organisationunitid=ougs.organisationunitid \
left join ${organisationunit} enrollmentou on en.organisationunitid=enrollmentou.organisationunitid \
inner join analytics_rs_categorystructure acs on ev.attributeoptioncomboid=acs.categoryoptioncomboid \
${attributeJoinClause}\
where ev.lastupdated < '${startTime}' ${partitionClause} \
and pr.programid=${programId} \
and ev.organisationunitid is not null \
Expand All @@ -356,6 +358,7 @@ and ev.status in (${exportableEventStatues}) \
Map.of(
"eventDateExpression", eventDateExpression,
"partitionClause", partitionClause,
"attributeJoinClause", attributeJoinClause,
"startTime", toLongDate(params.getStartTime()),
"programId", String.valueOf(program.getId()),
"firstDataYear", String.valueOf(firstDataYear),
Expand Down Expand Up @@ -486,15 +489,16 @@ private List<AnalyticsTableColumn> getColumnForDataElement(
List<AnalyticsTableColumn> columns = new ArrayList<>();

DataType dataType = getColumnType(dataElement.getValueType(), isSpatialSupport());
String columnExpression =
String jsonExpression =
sqlBuilder.jsonExtractNested("eventdatavalues", dataElement.getUid(), "value");
String selectExpression = getSelectExpression(dataElement.getValueType(), columnExpression);
String columnExpression = getColumnExpression(dataElement.getValueType(), jsonExpression);
String dataFilterClause = getDataFilterClause(dataElement);
String sql = String.format("%s as %s", selectExpression, quote(dataElement.getUid()));
String selectExpression =
String.format("%s as %s", columnExpression, quote(dataElement.getUid()));
Skip skipIndex = skipIndex(dataElement.getValueType(), dataElement.hasOptionSet());

if (withLegendSet) {
return getColumnFromDataElementWithLegendSet(dataElement, selectExpression, dataFilterClause);
return getColumnFromDataElementWithLegendSet(dataElement, columnExpression, dataFilterClause);
}

if (dataElement.getValueType().isOrganisationUnit()) {
Expand All @@ -506,7 +510,7 @@ private List<AnalyticsTableColumn> getColumnForDataElement(
.name(dataElement.getUid())
.dimensionType(AnalyticsDimensionType.DYNAMIC)
.dataType(dataType)
.selectExpression(sql)
.selectExpression(selectExpression)
.skipIndex(skipIndex)
.build());

Expand Down Expand Up @@ -587,7 +591,7 @@ private List<AnalyticsTableColumn> getAttributeColumns(Program program) {
*/
private List<AnalyticsTableColumn> getColumnForAttributeWithLegendSet(
TrackedEntityAttribute attribute) {
String selectClause = getSelectExpression(attribute.getValueType(), "value");
String columnExpression = getColumnExpression(attribute.getValueType(), "value");
String numericClause = getNumericClause();
String query =
"""
Expand All @@ -602,11 +606,11 @@ private List<AnalyticsTableColumn> getColumnForAttributeWithLegendSet(
.map(
ls -> {
String column = attribute.getUid() + PartitionUtils.SEP + ls.getUid();
String sql =
String selectExpression =
replaceQualify(
query,
Map.of(
"selectClause", selectClause,
"selectClause", columnExpression,
"legendSetId", String.valueOf(ls.getId()),
"column", column,
"attributeId", String.valueOf(attribute.getId()),
Expand All @@ -615,14 +619,14 @@ private List<AnalyticsTableColumn> getColumnForAttributeWithLegendSet(
return AnalyticsTableColumn.builder()
.name(column)
.dataType(CHARACTER_11)
.selectExpression(sql)
.selectExpression(selectExpression)
.build();
})
.toList();
}

/**
* Returns a select statement for the given data element with value type org unit.
* Returns a select statement for the given select expression.
*
* @param dataElement the data element to create the select statement for.
* @param selectExpression the select expression.
Expand Down Expand Up @@ -717,15 +721,14 @@ private List<Integer> getDataYears(
Program program,
Integer firstDataYear,
Integer lastDataYear) {
String fromDate = toMediumDate(params.getFromDate());
String fromDateClause =
params.getFromDate() != null
? replace(
"and (${eventDateExpression}) >= '${fromDate}'",
Map.of(
"eventDateExpression",
eventDateExpression,
"fromDate",
toMediumDate(params.getFromDate())))
"eventDateExpression", eventDateExpression,
"fromDate", fromDate))
: EMPTY;

String sql =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ then cast(eventdatavalues #>> '{GieVkTxp4HH, value}' as double precision) \
else null end""";

String actual =
manager.getSelectExpression(ValueType.NUMBER, "eventdatavalues #>> '{GieVkTxp4HH, value}'");
manager.getColumnExpression(ValueType.NUMBER, "eventdatavalues #>> '{GieVkTxp4HH, value}'");

assertEquals(expected, actual);
}
Expand All @@ -88,7 +88,7 @@ void testGetSelectExpressionBoolean() {
case when eventdatavalues #>> '{Xl3voRRcmpo, value}' = 'true' then 1 when eventdatavalues #>> '{Xl3voRRcmpo, value}' = 'false' then 0 else null end""";

String actual =
manager.getSelectExpression(
manager.getColumnExpression(
ValueType.BOOLEAN, "eventdatavalues #>> '{Xl3voRRcmpo, value}'");

assertEquals(expected, actual);
Expand All @@ -103,7 +103,7 @@ then cast(eventdatavalues #>> '{AL04Wbutskk, value}' as timestamp) \
else null end""";

String actual =
manager.getSelectExpression(ValueType.DATE, "eventdatavalues #>> '{AL04Wbutskk, value}'");
manager.getColumnExpression(ValueType.DATE, "eventdatavalues #>> '{AL04Wbutskk, value}'");

assertEquals(expected, actual);
}
Expand All @@ -115,7 +115,7 @@ void testGetSelectExpressionText() {
eventdatavalues #>> '{FwUzmc49Pcr, value}'""";

String actual =
manager.getSelectExpression(ValueType.TEXT, "eventdatavalues #>> '{FwUzmc49Pcr, value}'");
manager.getColumnExpression(ValueType.TEXT, "eventdatavalues #>> '{FwUzmc49Pcr, value}'");

assertEquals(expected, actual);
}
Expand All @@ -129,7 +129,7 @@ void testGetSelectExpressionGeometry() {
ST_GeomFromGeoJSON('{"type":"Point", "coordinates":' || (eventdatavalues #>> '{C6bh7GevJfH, value}') || ', "crs":{"type":"name", "properties":{"name":"EPSG:4326"}}}')""";

String actual =
manager.getSelectExpression(
manager.getColumnExpression(
ValueType.GEOJSON, "eventdatavalues #>> '{C6bh7GevJfH, value}'");

assertEquals(expected, actual);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -137,13 +137,7 @@ void verifyTeiTypeOrgUnitFetchesOuUidWhenPopulatingEventAnalyticsTable() {
subject.populateTable(params, partition);
verify(jdbcTemplate).execute(sql.capture());

String ouQuery =
format(
"""
(select value from "trackedentityattributevalue" \
where trackedentityid=en.trackedentityid and \
trackedentityattributeid=9999) as %s""",
quote(tea.getUid()));
String ouQuery = format("%s.value", quote(tea.getUid()));

assertThat(sql.getValue(), containsString(ouQuery));
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -501,10 +501,7 @@ void verifyGetTableWithTrackedEntityAttribute() {
String aliasD1 =
"""
eventdatavalues #>> '{deabcdefghZ, value}' as "deabcdefghZ\"""";
String aliasTeaUid =
"""
(select value from "trackedentityattributevalue" where trackedentityid=en.trackedentityid \
and trackedentityattributeid=%d) as "%s\"""";
String aliasTeaUid = "%s.value";

String aliasTea1 =
"""
Expand Down Expand Up @@ -537,12 +534,8 @@ void verifyGetTableWithTrackedEntityAttribute() {
.withTableType(AnalyticsTableType.EVENT)
.withColumnSize(59 + OU_NAME_HIERARCHY_COUNT)
.addColumns(periodColumns)
.addColumn(
d1.getUid(),
TEXT,
toSelectExpression(aliasD1, d1.getUid()),
Skip.SKIP) // ValueType.TEXT
.addColumn(tea1.getUid(), TEXT, String.format(aliasTeaUid, tea1.getId(), tea1.getUid()))
.addColumn(d1.getUid(), TEXT, toSelectExpression(aliasD1, d1.getUid()), Skip.SKIP)
.addColumn(tea1.getUid(), TEXT, String.format(aliasTeaUid, quote(tea1.getUid())))
// Org unit geometry column
.addColumn(
tea1.getUid() + "_geom",
Expand Down Expand Up @@ -650,12 +643,7 @@ void verifyTeiTypeOrgUnitFetchesOuNameWhenPopulatingEventAnalyticsTable() {
subject.populateTable(params, partition);
verify(jdbcTemplate).execute(sql.capture());

String ouUidQuery =
String.format(
"""
(select value from "trackedentityattributevalue" where trackedentityid=en.trackedentityid and \
trackedentityattributeid=9999) as %s""",
quote(tea.getUid()));
String ouUidQuery = String.format("%s.value", quote(tea.getUid()));

String ouNameQuery =
String.format(
Expand Down Expand Up @@ -944,12 +932,7 @@ void verifyTeaTypeOrgUnitFetchesOuNameWhenPopulatingEventAnalyticsTable() {

verify(jdbcTemplate).execute(sql.capture());

String ouUidQuery =
String.format(
"""
select value from "trackedentityattributevalue" where trackedentityid=en.trackedentityid \
and trackedentityattributeid=9999) as %s""",
quote(tea.getUid()));
String ouUidQuery = String.format("%s.value", quote(tea.getUid()));

String ouNameQuery =
String.format(
Expand Down

0 comments on commit 8fcf3bf

Please sign in to comment.