fix: Regexp match for ClickHouse SQL builder [DHIS2-18417] (#19269)

dhis2 · Nov 23, 2024 · 36d913c · 36d913c
1 parent 5a18ad8
commit 36d913c
Show file tree

Hide file tree

Showing 13 changed files with 87 additions and 40 deletions.
diff --git a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/dashboard/embedded/EmbeddedDashboard.java b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/dashboard/embedded/EmbeddedDashboard.java
@@ -34,7 +34,11 @@
 import lombok.NoArgsConstructor;
 import lombok.Setter;
 
-/** Encapsulates metadata for an embedded and externally provided dashboard. */
+/**
+ * Encapsulates metadata for an embedded and externally provided dashboard.
+ *
+ * @author Lars Helge Overland
+ */
 @Getter
 @Setter
 @NoArgsConstructor

diff --git a/dhis-2/dhis-api/src/main/java/org/hisp/dhis/dashboard/embedded/FilterOptions.java b/dhis-2/dhis-api/src/main/java/org/hisp/dhis/dashboard/embedded/FilterOptions.java
@@ -34,6 +34,11 @@
 import lombok.NoArgsConstructor;
 import lombok.Setter;
 
+/**
+ * Encapsulates embedded dashboard filter options.
+ *
+ * @author Lars Helge Overland
+ */
 @Getter
 @Setter
 @NoArgsConstructor

diff --git a/...-analytics/src/main/java/org/hisp/dhis/analytics/table/AbstractEventJdbcTableManager.java b/...-analytics/src/main/java/org/hisp/dhis/analytics/table/AbstractEventJdbcTableManager.java
@@ -96,11 +96,11 @@ public AbstractEventJdbcTableManager(
   public static final String OU_NAME_COL_SUFFIX = "_name";
 
   protected final String getNumericClause() {
-    return " and value " + sqlBuilder.regexpMatch("'" + NUMERIC_LENIENT_REGEXP + "'");
+    return " and " + sqlBuilder.regexpMatch("value", "'" + NUMERIC_LENIENT_REGEXP + "'");
   }
 
   protected final String getDateClause() {
-    return " and value " + sqlBuilder.regexpMatch("'" + DATE_REGEXP + "'");
+    return " and " + sqlBuilder.regexpMatch("value", "'" + DATE_REGEXP + "'");
   }
 
   protected Skip skipIndex(ValueType valueType, boolean hasOptionSet) {

diff --git a/...vice-analytics/src/main/java/org/hisp/dhis/analytics/table/JdbcAnalyticsTableManager.java b/...vice-analytics/src/main/java/org/hisp/dhis/analytics/table/JdbcAnalyticsTableManager.java
@@ -40,6 +40,7 @@
 import static org.hisp.dhis.db.model.DataType.VARCHAR_255;
 import static org.hisp.dhis.db.model.constraint.Nullable.NOT_NULL;
 import static org.hisp.dhis.db.model.constraint.Nullable.NULL;
+import static org.hisp.dhis.system.util.MathUtils.NUMERIC_LENIENT_REGEXP;
 import static org.hisp.dhis.util.DateUtils.toLongDate;
 
 import com.google.common.collect.Sets;
@@ -78,7 +79,6 @@
 import org.hisp.dhis.setting.SystemSettings;
 import org.hisp.dhis.setting.SystemSettingsProvider;
 import org.hisp.dhis.system.database.DatabaseInfoProvider;
-import org.hisp.dhis.system.util.MathUtils;
 import org.hisp.dhis.util.DateUtils;
 import org.hisp.dhis.util.ObjectUtils;
 import org.springframework.beans.factory.annotation.Qualifier;
@@ -280,9 +280,7 @@ public void populateTable(AnalyticsTableUpdateParams params, AnalyticsTableParti
     String numericClause =
         skipDataTypeValidation
             ? ""
-            : replace(
-                "and dv.value ~* '${expression}'",
-                Map.of("expression", MathUtils.NUMERIC_LENIENT_REGEXP));
+            : "and " + sqlBuilder.regexpMatch("dv.value", "'" + NUMERIC_LENIENT_REGEXP + "'");
     String zeroValueCondition = includeZeroValues ? " or des.zeroissignificant = true" : "";
     String zeroValueClause =
         replace(

diff --git a/...analytics/src/main/java/org/hisp/dhis/analytics/table/JdbcEventAnalyticsTableManager.java b/...analytics/src/main/java/org/hisp/dhis/analytics/table/JdbcEventAnalyticsTableManager.java
@@ -714,7 +714,7 @@ private String getDataClause(String uid, ValueType valueType) {
 
       String jsonValue = sqlBuilder.jsonExtractNested("eventdatavalues", uid, "value");
 
-      return " and " + jsonValue + " " + sqlBuilder.regexpMatch("'" + regex + "'");
+      return " and " + sqlBuilder.regexpMatch(jsonValue, "'" + regex + "'");
     }
 
     return "";

diff --git a/...vices/dhis-service-analytics/src/main/java/org/hisp/dhis/db/sql/ClickHouseSqlBuilder.java b/...vices/dhis-service-analytics/src/main/java/org/hisp/dhis/db/sql/ClickHouseSqlBuilder.java
@@ -198,8 +198,8 @@ public String differenceInSeconds(String columnA, String columnB) {
   }
 
   @Override
-  public String regexpMatch(String pattern) {
-    return String.format("match %s", pattern); // TO DO
+  public String regexpMatch(String value, String pattern) {
+    return String.format("match(%s, %s)", value, pattern);
   }
 
   @Override
@@ -219,18 +219,18 @@ public String coalesce(String expression, String defaultValue) {
 
   @Override
   public String jsonExtract(String column, String property) {
-    return "JSONExtractRaw(" + column + ", '" + property + "')";
+    return "JSONExtractString(" + column + ", '" + property + "')";
   }
 
   @Override
   public String jsonExtract(String tablePrefix, String column, String jsonPath) {
-    return String.format("JSONExtractRaw(%s.%s, '%s')", tablePrefix, column, jsonPath);
+    return String.format("JSONExtractString(%s.%s, '%s')", tablePrefix, column, jsonPath);
   }
 
   @Override
   public String jsonExtractNested(String column, String... jsonPath) {
     String path = String.join(".", jsonPath);
-    return String.format("JSONExtract(%s, '%s')", column, path);
+    return String.format("JSONExtractString(%s, '%s')", column, path);
   }
 
   // Statements

diff --git a/...s-services/dhis-service-analytics/src/main/java/org/hisp/dhis/db/sql/DorisSqlBuilder.java b/...s-services/dhis-service-analytics/src/main/java/org/hisp/dhis/db/sql/DorisSqlBuilder.java
@@ -201,8 +201,8 @@ public String differenceInSeconds(String columnA, String columnB) {
   }
 
   @Override
-  public String regexpMatch(String pattern) {
-    return String.format("regexp %s", pattern);
+  public String regexpMatch(String value, String pattern) {
+    return String.format("%s regexp %s", value, pattern);
   }
 
   @Override
@@ -234,7 +234,7 @@ public String jsonExtract(String tablePrefix, String column, String jsonPath) {
   @Override
   public String jsonExtractNested(String column, String... jsonPath) {
     String path = "$." + String.join(".", jsonPath);
-    return String.format("JSON_UNQUOTE(JSON_EXTRACT(%s, '%s'))", column, path);
+    return String.format("json_unquote(json_extract(%s, '%s'))", column, path);
   }
 
   // Statements

diff --git a/...services/dhis-service-analytics/src/main/java/org/hisp/dhis/db/sql/PostgreSqlBuilder.java b/...services/dhis-service-analytics/src/main/java/org/hisp/dhis/db/sql/PostgreSqlBuilder.java
@@ -220,8 +220,8 @@ public String differenceInSeconds(String columnA, String columnB) {
   }
 
   @Override
-  public String regexpMatch(String pattern) {
-    return String.format("~* %s", pattern);
+  public String regexpMatch(String value, String pattern) {
+    return String.format("%s ~* %s", value, pattern);
   }
 
   @Override

diff --git a/...2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/db/sql/SqlBuilder.java b/...2/dhis-services/dhis-service-analytics/src/main/java/org/hisp/dhis/db/sql/SqlBuilder.java
@@ -237,10 +237,11 @@ public interface SqlBuilder {
   String differenceInSeconds(String columnA, String columnB);
 
   /**
-   * @param pattern the regular expression pattern to match against
-   * @return a regular expression matching clause.
+   * @param value the string value such as a column or expression.
+   * @param pattern the regular expression pattern to match against.
+   * @return a regular expression string matching clause.
    */
-  String regexpMatch(String pattern);
+  String regexpMatch(String value, String pattern);
 
   /**
    * Creates a SQL concatenation function that combines multiple columns or expressions.

diff --git a/.../src/test/java/org/hisp/dhis/analytics/table/JdbcEventAnalyticsTableManagerDorisTest.java b/.../src/test/java/org/hisp/dhis/analytics/table/JdbcEventAnalyticsTableManagerDorisTest.java
@@ -204,31 +204,31 @@ void verifyGetTableWithDataElements() {
     when(idObjectManager.getAllNoAcl(Program.class)).thenReturn(List.of(program));
 
     String aliasD1 =
-        "(select JSON_UNQUOTE(JSON_EXTRACT(eventdatavalues, '$.%s.value')) "
+        "(select json_unquote(json_extract(eventdatavalues, '$.%s.value')) "
             + FROM_CLAUSE
             + " ) as `%s`";
     String aliasD2 =
-        "(select cast(JSON_UNQUOTE(JSON_EXTRACT(eventdatavalues, '$.%s.value')) as double) "
+        "(select cast(json_unquote(json_extract(eventdatavalues, '$.%s.value')) as double) "
             + FROM_CLAUSE
-            + "  and JSON_UNQUOTE(JSON_EXTRACT(eventdatavalues, '$.%s.value')) regexp '^(-?[0-9]+)(\\.[0-9]+)?$') as `%s`";
+            + "  and json_unquote(json_extract(eventdatavalues, '$.%s.value')) regexp '^(-?[0-9]+)(\\.[0-9]+)?$') as `%s`";
     String aliasD3 =
-        "(select case when JSON_UNQUOTE(JSON_EXTRACT(eventdatavalues, '$.%s.value')) = 'true' then 1 when JSON_UNQUOTE(JSON_EXTRACT(eventdatavalues, '$.%s.value')) = 'false' then 0 else null end "
+        "(select case when json_unquote(json_extract(eventdatavalues, '$.%s.value')) = 'true' then 1 when json_unquote(json_extract(eventdatavalues, '$.%s.value')) = 'false' then 0 else null end "
             + FROM_CLAUSE
             + " ) as `%s`";
     String aliasD4 =
-        "(select cast(JSON_UNQUOTE(JSON_EXTRACT(eventdatavalues, '$.%s.value')) as datetime) "
+        "(select cast(json_unquote(json_extract(eventdatavalues, '$.%s.value')) as datetime) "
             + FROM_CLAUSE
-            + "  and JSON_UNQUOTE(JSON_EXTRACT(eventdatavalues, '$.%s.value')) regexp '^\\d{4}-\\d{2}-\\d{2}(\\s|T)?((\\d{2}:)(\\d{2}:)?(\\d{2}))?(|.(\\d{3})|.(\\d{3})Z)?$') as `%s`";
+            + "  and json_unquote(json_extract(eventdatavalues, '$.%s.value')) regexp '^\\d{4}-\\d{2}-\\d{2}(\\s|T)?((\\d{2}:)(\\d{2}:)?(\\d{2}))?(|.(\\d{3})|.(\\d{3})Z)?$') as `%s`";
     String aliasD5 =
-        "(select ou.uid from dhis2.public.`organisationunit` ou where ou.uid = JSON_UNQUOTE(JSON_EXTRACT(eventdatavalues, '$.deabcdefghG.value')) ) as `deabcdefghG`";
+        "(select ou.uid from dhis2.public.`organisationunit` ou where ou.uid = json_unquote(json_extract(eventdatavalues, '$.deabcdefghG.value')) ) as `deabcdefghG`";
     String aliasD5Name =
-        "(select ou.name from dhis2.public.`organisationunit` ou where ou.uid = JSON_UNQUOTE(JSON_EXTRACT(eventdatavalues, "
+        "(select ou.name from dhis2.public.`organisationunit` ou where ou.uid = json_unquote(json_extract(eventdatavalues, "
             + "'$.%s.value')) "
             + ") as `%s`";
     String aliasD6 =
-        "(select cast(JSON_UNQUOTE(JSON_EXTRACT(eventdatavalues, '$.%s.value')) as bigint) "
+        "(select cast(json_unquote(json_extract(eventdatavalues, '$.%s.value')) as bigint) "
             + FROM_CLAUSE
-            + "  and JSON_UNQUOTE(JSON_EXTRACT(eventdatavalues, '$.%s.value')) regexp '^(-?[0-9]+)(\\.[0-9]+)?$') as `%s`";
+            + "  and json_unquote(json_extract(eventdatavalues, '$.%s.value')) regexp '^(-?[0-9]+)(\\.[0-9]+)?$') as `%s`";
 
     AnalyticsTableUpdateParams params =
         AnalyticsTableUpdateParams.newBuilder()

diff --git a/...s/dhis-service-analytics/src/test/java/org/hisp/dhis/db/sql/ClickHouseSqlBuilderTest.java b/...s/dhis-service-analytics/src/test/java/org/hisp/dhis/db/sql/ClickHouseSqlBuilderTest.java
@@ -195,6 +195,32 @@ void testDateTrunc() {
         "date_trunc('month', pe.startdate)", sqlBuilder.dateTrunc("month", "pe.startdate"));
   }
 
+  @Test
+  void testDifferenceInSeconds() {
+    assertEquals(
+        "(toUnixTimestamp(a.startdate) - toUnixTimestamp(b.enddate))",
+        sqlBuilder.differenceInSeconds("a.startdate", "b.enddate"));
+    assertEquals(
+        "(toUnixTimestamp(a.\"startdate\") - toUnixTimestamp(b.\"enddate\"))",
+        sqlBuilder.differenceInSeconds(
+            sqlBuilder.quote("a", "startdate"), sqlBuilder.quote("b", "enddate")));
+  }
+
+  @Test
+  void testRegexpMatch() {
+    assertEquals("match(value, 'test')", sqlBuilder.regexpMatch("value", "'test'"));
+    assertEquals("match(number, '\\d')", sqlBuilder.regexpMatch("number", "'\\d'"));
+    assertEquals("match(color, '^Blue$')", sqlBuilder.regexpMatch("color", "'^Blue$'"));
+    assertEquals("match(id, '[a-z]\\w+\\d{3}')", sqlBuilder.regexpMatch("id", "'[a-z]\\w+\\d{3}'"));
+  }
+
+  @Test
+  void testJsonExtractNested() {
+    assertEquals(
+        "JSONExtractString(eventdatavalues, 'D7m8vpzxHDJ.value')",
+        sqlBuilder.jsonExtractNested("eventdatavalues", "D7m8vpzxHDJ", "value"));
+  }
+
   // Statements
 
   @Test

diff --git a/...rvices/dhis-service-analytics/src/test/java/org/hisp/dhis/db/sql/DorisSqlBuilderTest.java b/...rvices/dhis-service-analytics/src/test/java/org/hisp/dhis/db/sql/DorisSqlBuilderTest.java
@@ -191,10 +191,17 @@ void testDifferenceInSeconds() {
 
   @Test
   void testRegexpMatch() {
-    assertEquals("regexp test", sqlBuilder.regexpMatch("test"));
-    assertEquals("regexp \\d", sqlBuilder.regexpMatch("\\d"));
-    assertEquals("regexp ", sqlBuilder.regexpMatch(""));
-    assertEquals("regexp [a-z]\\w+\\d{3}", sqlBuilder.regexpMatch("[a-z]\\w+\\d{3}"));
+    assertEquals("value regexp 'test'", sqlBuilder.regexpMatch("value", "'test'"));
+    assertEquals("number regexp '\\d'", sqlBuilder.regexpMatch("number", "'\\d'"));
+    assertEquals("color regexp '^Blue$'", sqlBuilder.regexpMatch("color", "'^Blue$'"));
+    assertEquals("id regexp '[a-z]\\w+\\d{3}'", sqlBuilder.regexpMatch("id", "'[a-z]\\w+\\d{3}'"));
+  }
+
+  @Test
+  void testJsonExtractNested() {
+    assertEquals(
+        "json_unquote(json_extract(eventdatavalues, '$.D7m8vpzxHDJ.value'))",
+        sqlBuilder.jsonExtractNested("eventdatavalues", "D7m8vpzxHDJ", "value"));
   }
 
   // Statements

diff --git a/...ices/dhis-service-analytics/src/test/java/org/hisp/dhis/db/sql/PostgreSqlBuilderTest.java b/...ices/dhis-service-analytics/src/test/java/org/hisp/dhis/db/sql/PostgreSqlBuilderTest.java
@@ -219,11 +219,17 @@ void testDifferenceInSeconds() {
 
   @Test
   void testRegexpMatch() {
-    assertEquals("~* test", sqlBuilder.regexpMatch("test"));
-    assertEquals("~* ", sqlBuilder.regexpMatch(""));
-    assertEquals("~* null", sqlBuilder.regexpMatch(null));
-    assertEquals("~* .*[a-z]\\d+", sqlBuilder.regexpMatch(".*[a-z]\\d+"));
-    assertEquals("~*  ", sqlBuilder.regexpMatch(" "));
+    assertEquals("value ~* 'test'", sqlBuilder.regexpMatch("value", "'test'"));
+    assertEquals("number ~* '\\d'", sqlBuilder.regexpMatch("number", "'\\d'"));
+    assertEquals("color ~* '^Blue$'", sqlBuilder.regexpMatch("color", "'^Blue$'"));
+    assertEquals("id ~* '[a-z]\\w+\\d{3}'", sqlBuilder.regexpMatch("id", "'[a-z]\\w+\\d{3}'"));
+  }
+
+  @Test
+  void testJsonExtractNested() {
+    assertEquals(
+        "eventdatavalues #>> '{D7m8vpzxHDJ, value}'",
+        sqlBuilder.jsonExtractNested("eventdatavalues", "D7m8vpzxHDJ", "value"));
   }
 
   // Statements