From f4f2aca67f194d6780242adbadd6305513f4ad10 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Lars=20Helge=20=C3=98verland?= Date: Wed, 11 Dec 2024 10:15:44 +0100 Subject: [PATCH] fix: Use dynamic org unit level analytics table columns [DHIS2-18417] (#19439) --- ...dbcTrackedEntityAnalyticsTableManager.java | 41 +------------------ ...ntityEnrollmentsAnalyticsTableManager.java | 25 +---------- ...ckedEntityEventsAnalyticsTableManager.java | 25 +---------- .../dhis/analytics/util/AnalyticsUtils.java | 30 -------------- ...EntityEventsAnalyticsTableManagerTest.java | 4 +- .../analytics/util/AnalyticsUtilsTest.java | 8 ---- .../dhis/db/sql/ClickHouseSqlBuilder.java | 5 +++ .../org/hisp/dhis/db/sql/DorisSqlBuilder.java | 5 +++ .../hisp/dhis/db/sql/PostgreSqlBuilder.java | 5 +++ .../java/org/hisp/dhis/db/sql/SqlBuilder.java | 5 +++ 10 files changed, 26 insertions(+), 127 deletions(-) diff --git a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/JdbcTrackedEntityAnalyticsTableManager.java b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/JdbcTrackedEntityAnalyticsTableManager.java index 4a483d156d37..5ef9f9a562d4 100644 --- a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/JdbcTrackedEntityAnalyticsTableManager.java +++ b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/JdbcTrackedEntityAnalyticsTableManager.java @@ -35,7 +35,6 @@ import static org.hisp.dhis.analytics.util.AnalyticsUtils.getColumnType; import static org.hisp.dhis.analytics.util.DisplayNameUtils.getDisplayName; import static org.hisp.dhis.commons.util.TextUtils.removeLastComma; -import static org.hisp.dhis.commons.util.TextUtils.replace; import static org.hisp.dhis.db.model.DataType.BOOLEAN; import static org.hisp.dhis.db.model.DataType.CHARACTER_11; import static org.hisp.dhis.db.model.DataType.DOUBLE; @@ -65,7 +64,6 @@ import org.hisp.dhis.analytics.table.setting.AnalyticsTableSettings; import org.hisp.dhis.category.CategoryService; import org.hisp.dhis.common.IdentifiableObjectManager; -import org.hisp.dhis.common.ValueType; import org.hisp.dhis.dataapproval.DataApprovalLevelService; import org.hisp.dhis.db.model.IndexType; import org.hisp.dhis.db.model.Logged; @@ -85,7 +83,7 @@ import org.springframework.transaction.annotation.Transactional; @Component("org.hisp.dhis.analytics.TrackedEntityAnalyticsTableManager") -public class JdbcTrackedEntityAnalyticsTableManager extends AbstractJdbcTableManager { +public class JdbcTrackedEntityAnalyticsTableManager extends AbstractEventJdbcTableManager { private static final String PROGRAMS_BY_TET_KEY = "programsByTetUid"; private static final String ALL_NON_CONFIDENTIAL_TET_ATTRIBUTES = @@ -215,7 +213,7 @@ private List getColumns( .name(tea.getUid()) .dataType(getColumnType(tea.getValueType(), isSpatialSupport())) .selectExpression( - castBasedOnType(tea.getValueType(), quote(tea.getUid()) + ".value")) + getColumnExpression(tea.getValueType(), quote(tea.getUid()) + ".value")) .build()) .toList()); @@ -242,41 +240,6 @@ private Stream getAllTrackedEntityAttributes( return getAllTrackedEntityAttributesByEntityType(trackedEntityType); } - /** - * Returns the select clause, potentially with a cast statement, based on the given value type. - * - * @param valueType the value type to represent as database column type. - */ - private String castBasedOnType(ValueType valueType, String columnName) { - if (valueType.isDecimal()) { - - return replace( - " cast(${columnName} as ${type})", - Map.of("columnName", columnName, "type", sqlBuilder.dataTypeDouble())); - } - if (valueType.isInteger()) { - return replace(" cast(${columnName} as bigint)", Map.of("columnName", columnName)); - } - if (valueType.isBoolean()) { - return replace( - " case when ${columnName} = 'true' then 1 when ${columnName} = 'false' then 0 end ", - Map.of("columnName", columnName)); - } - if (valueType.isDate()) { - return replace( - " cast(${columnName} as ${type})", - Map.of("columnName", columnName, "type", sqlBuilder.dataTypeTimestamp())); - } - if (valueType.isGeo() && isSpatialSupport() && sqlBuilder.supportsGeospatialData()) { - return replace( - """ - \s ST_GeomFromGeoJSON('{"type":"Point", "coordinates":' || (${columnName}) || ', - "crs":{"type":"name", "properties":{"name":"EPSG:4326"}}}')""", - Map.of("columnName", columnName)); - } - return columnName; - } - /** * Returns all {@link TrackedEntityAttribute} for the given {@link TrackedEntityType} and * programs. diff --git a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/JdbcTrackedEntityEnrollmentsAnalyticsTableManager.java b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/JdbcTrackedEntityEnrollmentsAnalyticsTableManager.java index 70428872148d..a00e34eb392b 100644 --- a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/JdbcTrackedEntityEnrollmentsAnalyticsTableManager.java +++ b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/JdbcTrackedEntityEnrollmentsAnalyticsTableManager.java @@ -115,30 +115,6 @@ public class JdbcTrackedEntityEnrollmentsAnalyticsTableManager extends AbstractJ .dataType(VARCHAR_50) .selectExpression("en.status") .build(), - AnalyticsTableColumn.builder() - .name("uidlevel1") - .dataType(CHARACTER_11) - .nullable(NULL) - .selectExpression("ous.uidlevel1") - .build(), - AnalyticsTableColumn.builder() - .name("uidlevel2") - .dataType(CHARACTER_11) - .nullable(NULL) - .selectExpression("ous.uidlevel2") - .build(), - AnalyticsTableColumn.builder() - .name("uidlevel3") - .dataType(CHARACTER_11) - .nullable(NULL) - .selectExpression("ous.uidlevel3") - .build(), - AnalyticsTableColumn.builder() - .name("uidlevel4") - .dataType(CHARACTER_11) - .nullable(NULL) - .selectExpression("ous.uidlevel4") - .build(), AnalyticsTableColumn.builder() .name("ou") .dataType(CHARACTER_11) @@ -225,6 +201,7 @@ private List getColumns() { List columns = new ArrayList<>(); columns.addAll(getFixedCols()); columns.add(getOrganisationUnitNameHierarchyColumn()); + columns.addAll(getOrganisationUnitLevelColumns()); if (sqlBuilder.supportsDeclarativePartitioning()) { columns.add(getPartitionColumn()); } diff --git a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/JdbcTrackedEntityEventsAnalyticsTableManager.java b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/JdbcTrackedEntityEventsAnalyticsTableManager.java index a89213becd23..b3968442378b 100644 --- a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/JdbcTrackedEntityEventsAnalyticsTableManager.java +++ b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/table/JdbcTrackedEntityEventsAnalyticsTableManager.java @@ -143,30 +143,6 @@ public class JdbcTrackedEntityEventsAnalyticsTableManager extends AbstractJdbcTa .dataType(VARCHAR_50) .selectExpression("ev.status") .build(), - AnalyticsTableColumn.builder() - .name("uidlevel1") - .dataType(CHARACTER_11) - .nullable(NULL) - .selectExpression("ous.uidlevel1") - .build(), - AnalyticsTableColumn.builder() - .name("uidlevel2") - .dataType(CHARACTER_11) - .nullable(NULL) - .selectExpression("ous.uidlevel2") - .build(), - AnalyticsTableColumn.builder() - .name("uidlevel3") - .dataType(CHARACTER_11) - .nullable(NULL) - .selectExpression("ous.uidlevel3") - .build(), - AnalyticsTableColumn.builder() - .name("uidlevel4") - .dataType(CHARACTER_11) - .nullable(NULL) - .selectExpression("ous.uidlevel4") - .build(), AnalyticsTableColumn.builder() .name("ou") .dataType(CHARACTER_11) @@ -367,6 +343,7 @@ private List getColumns() { } columns.add(getOrganisationUnitNameHierarchyColumn()); + columns.addAll(getOrganisationUnitLevelColumns()); return columns; } diff --git a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/util/AnalyticsUtils.java b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/util/AnalyticsUtils.java index cf73c25f349d..b61b7a82bbaa 100644 --- a/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/util/AnalyticsUtils.java +++ b/dhis-2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/analytics/util/AnalyticsUtils.java @@ -27,7 +27,6 @@ */ package org.hisp.dhis.analytics.util; -import static org.apache.commons.lang3.StringUtils.EMPTY; import static org.hisp.dhis.common.DataDimensionItem.DATA_DIM_TYPE_CLASS_MAP; import static org.hisp.dhis.common.DimensionalObject.ATTRIBUTEOPTIONCOMBO_DIM_ID; import static org.hisp.dhis.common.DimensionalObject.CATEGORYOPTIONCOMBO_DIM_ID; @@ -1202,33 +1201,4 @@ public static String replaceStringBetween( Matcher matcher = pattern.matcher(original); return matcher.replaceAll(startToken + replacement + endToken); } - - /** - * Returns a string containing closing parenthesis. The number of parenthesis is based on the - * number of missing closing parenthesis in the argument string. - * - *

Example: - * - *

{@code} input: "((( ))" -> output: ")" {@code} - * - * @param str a string. - * @return a String containing 0 or more "closing" parenthesis - */ - public static String getClosingParentheses(String str) { - if (StringUtils.isEmpty(str)) { - return EMPTY; - } - - int open = 0; - - for (int i = 0; i < str.length(); i++) { - if (str.charAt(i) == '(') { - open++; - } else if ((str.charAt(i) == ')') && open >= 1) { - open--; - } - } - - return StringUtils.repeat(")", open); - } } diff --git a/dhis-2/dhis-services/dhis-service-analytics/src/test/java/org/hisp/dhis/analytics/table/JdbcTrackedEntityEventsAnalyticsTableManagerTest.java b/dhis-2/dhis-services/dhis-service-analytics/src/test/java/org/hisp/dhis/analytics/table/JdbcTrackedEntityEventsAnalyticsTableManagerTest.java index 4cb4bdf9f195..57fce6c172ac 100644 --- a/dhis-2/dhis-services/dhis-service-analytics/src/test/java/org/hisp/dhis/analytics/table/JdbcTrackedEntityEventsAnalyticsTableManagerTest.java +++ b/dhis-2/dhis-services/dhis-service-analytics/src/test/java/org/hisp/dhis/analytics/table/JdbcTrackedEntityEventsAnalyticsTableManagerTest.java @@ -160,9 +160,9 @@ void testPopulateTable() { String expectedSql = """ insert into analytics_te_event_tetuid_temp ("trackedentity","program","enrollment","programstage","event","occurreddate","lastupdated","created", - "scheduleddate","status","uidlevel1","uidlevel2","uidlevel3","uidlevel4","ou","ouname","oucode","oulevel","eventdatavalues","eventgeometry", + "scheduleddate","status","ou","ouname","oucode","oulevel","eventdatavalues","eventgeometry", "evlongitude","evlatitude","ounamehierarchy") select distinct te.uid,p.uid,en.uid,ps.uid,ev.uid,ev.occurreddate,ev.lastupdated, - ev.created,ev.scheduleddate,ev.status,ous.uidlevel1,ous.uidlevel2,ous.uidlevel3,ous.uidlevel4,ous.organisationunituid,ous.name,ous.code,ous.level, + ev.created,ev.scheduleddate,ev.status,ous.organisationunituid,ous.name,ous.code,ous.level, %s, ev.geometry,case when 'POINT' = GeometryType(ev.geometry) then ST_X(ev.geometry) end,case when 'POINT' = GeometryType(ev.geometry) then ST_Y(ev.geometry) end,concat_ws(' / ',) as ounamehierarchy from "event" ev inner join "enrollment" en on en.enrollmentid=ev.enrollmentid diff --git a/dhis-2/dhis-services/dhis-service-analytics/src/test/java/org/hisp/dhis/analytics/util/AnalyticsUtilsTest.java b/dhis-2/dhis-services/dhis-service-analytics/src/test/java/org/hisp/dhis/analytics/util/AnalyticsUtilsTest.java index dbbb563aa2ac..d2b77d2fecbf 100644 --- a/dhis-2/dhis-services/dhis-service-analytics/src/test/java/org/hisp/dhis/analytics/util/AnalyticsUtilsTest.java +++ b/dhis-2/dhis-services/dhis-service-analytics/src/test/java/org/hisp/dhis/analytics/util/AnalyticsUtilsTest.java @@ -739,14 +739,6 @@ void testGetEnumCaseSensitivity() { assertNull(EnumUtils.getEnum(Database.class, "postgresql")); } - @Test - void testGetClosingParentheses() { - assertEquals("", AnalyticsUtils.getClosingParentheses(null)); - assertEquals("", AnalyticsUtils.getClosingParentheses("")); - assertEquals(")", AnalyticsUtils.getClosingParentheses("from(select(select (*))")); - assertEquals("))", AnalyticsUtils.getClosingParentheses("((")); - } - @Test void whenUncategorizedSQLException_withTableNotExisting_thenThrowException() { SQLException sqlException = new SQLException("relation does not exist", "42P01"); diff --git a/dhis-2/dhis-support/dhis-support-sql/src/main/java/org/hisp/dhis/db/sql/ClickHouseSqlBuilder.java b/dhis-2/dhis-support/dhis-support-sql/src/main/java/org/hisp/dhis/db/sql/ClickHouseSqlBuilder.java index f763eace89e7..6381be607075 100644 --- a/dhis-2/dhis-support/dhis-support-sql/src/main/java/org/hisp/dhis/db/sql/ClickHouseSqlBuilder.java +++ b/dhis-2/dhis-support/dhis-support-sql/src/main/java/org/hisp/dhis/db/sql/ClickHouseSqlBuilder.java @@ -164,6 +164,11 @@ public boolean supportsVacuum() { return false; } + @Override + public boolean supportsCorrelatedSubquery() { + return false; + } + @Override public boolean requiresIndexesForAnalytics() { return false; diff --git a/dhis-2/dhis-support/dhis-support-sql/src/main/java/org/hisp/dhis/db/sql/DorisSqlBuilder.java b/dhis-2/dhis-support/dhis-support-sql/src/main/java/org/hisp/dhis/db/sql/DorisSqlBuilder.java index e4414841175f..48bfe6bc31b8 100644 --- a/dhis-2/dhis-support/dhis-support-sql/src/main/java/org/hisp/dhis/db/sql/DorisSqlBuilder.java +++ b/dhis-2/dhis-support/dhis-support-sql/src/main/java/org/hisp/dhis/db/sql/DorisSqlBuilder.java @@ -166,6 +166,11 @@ public boolean supportsVacuum() { return false; } + @Override + public boolean supportsCorrelatedSubquery() { + return true; + } + @Override public boolean requiresIndexesForAnalytics() { return false; diff --git a/dhis-2/dhis-support/dhis-support-sql/src/main/java/org/hisp/dhis/db/sql/PostgreSqlBuilder.java b/dhis-2/dhis-support/dhis-support-sql/src/main/java/org/hisp/dhis/db/sql/PostgreSqlBuilder.java index f28259ce271b..92a14270f8cc 100644 --- a/dhis-2/dhis-support/dhis-support-sql/src/main/java/org/hisp/dhis/db/sql/PostgreSqlBuilder.java +++ b/dhis-2/dhis-support/dhis-support-sql/src/main/java/org/hisp/dhis/db/sql/PostgreSqlBuilder.java @@ -185,6 +185,11 @@ public boolean supportsVacuum() { return true; } + @Override + public boolean supportsCorrelatedSubquery() { + return true; + } + @Override public boolean requiresIndexesForAnalytics() { return true; diff --git a/dhis-2/dhis-support/dhis-support-sql/src/main/java/org/hisp/dhis/db/sql/SqlBuilder.java b/dhis-2/dhis-support/dhis-support-sql/src/main/java/org/hisp/dhis/db/sql/SqlBuilder.java index 78bbd2d866a4..07016f4c6e8d 100644 --- a/dhis-2/dhis-support/dhis-support-sql/src/main/java/org/hisp/dhis/db/sql/SqlBuilder.java +++ b/dhis-2/dhis-support/dhis-support-sql/src/main/java/org/hisp/dhis/db/sql/SqlBuilder.java @@ -174,6 +174,11 @@ public interface SqlBuilder { */ boolean supportsVacuum(); + /** + * @return true if the DBMS supports corrected subqueries. + */ + boolean supportsCorrelatedSubquery(); + /** * @return true if the DBMS requires indexes for analytics tables for performance. */