Common pattern validation for all regexp* functions (#18762)

Nicer user-facing error messages for invalid patterns used in the regexp* functions. Move existing compilePattern() utility to a shared place so the functions use them.

Author: abhishekrb19

processing/src/main/java/org/apache/druid/query/expression/RegexpExprUtils.java

@@ -0,0 +1,51 @@
+/*
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements.  See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership.  The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License.  You may obtain a copy of the License at
+ *
+ *   http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied.  See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ */
+
+package org.apache.druid.query.expression;
+
+import org.apache.druid.error.InvalidInput;
+import org.apache.druid.java.util.common.StringUtils;
+
+import javax.annotation.Nullable;
+import java.util.regex.Pattern;
+import java.util.regex.PatternSyntaxException;
+
+public class RegexpExprUtils
+{
+  /**
+   * Compile the provided pattern, or provide a nice error message if it cannot be compiled.
+   */
+  public static Pattern compilePattern(@Nullable String pattern, String functionName)
+  {
+    try {
+      return Pattern.compile(StringUtils.nullToEmptyNonDruidDataString(pattern));
+    }
+    catch (PatternSyntaxException e) {
+      throw InvalidInput.exception(
+          e,
+          StringUtils.format(
+              "An invalid pattern [%s] was provided for the [%s] function, error: [%s]",
+              e.getPattern(),
+              functionName,
+              e.getMessage()
+          )
+      );
+    }
+  }
+}

processing/src/main/java/org/apache/druid/query/expression/RegexpExtractExprMacro.java

@@ -19,8 +19,6 @@
 
 package org.apache.druid.query.expression;
 
-import org.apache.druid.error.InvalidInput;
-import org.apache.druid.java.util.common.StringUtils;
 import org.apache.druid.math.expr.Expr;
 import org.apache.druid.math.expr.ExprEval;
 import org.apache.druid.math.expr.ExprMacroTable;
@@ -31,7 +29,6 @@
 import java.util.List;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
-import java.util.regex.PatternSyntaxException;
 
 public class RegexpExtractExprMacro implements ExprMacroTable.ExprMacro
 {
@@ -61,7 +58,7 @@ public Expr apply(final List<Expr> args)
     }
 
     // Precompile the pattern.
-    final Pattern pattern = compilePattern((String) patternExpr.getLiteralValue());
+    final Pattern pattern = RegexpExprUtils.compilePattern((String) patternExpr.getLiteralValue(), FN_NAME);
 
     final int index = indexExpr == null ? 0 : ((Number) indexExpr.getLiteralValue()).intValue();
 
@@ -97,25 +94,4 @@ public ExpressionType getOutputType(InputBindingInspector inspector)
     }
     return new RegexpExtractExpr(args);
   }
-
-  /**
-   * Compile the provided pattern, or provide a nice error message if it cannot be compiled.
-   */
-  private static Pattern compilePattern(@Nullable String pattern)
-  {
-    try {
-      return Pattern.compile(StringUtils.nullToEmptyNonDruidDataString(pattern));
-    }
-    catch (PatternSyntaxException e) {
-      throw InvalidInput.exception(
-          e,
-          StringUtils.format(
-              "An invalid pattern [%s] was provided for the %s function, error: [%s]",
-              e.getPattern(),
-              FN_NAME,
-              e.getMessage()
-          )
-      );
-    }
-  }
 }

processing/src/main/java/org/apache/druid/query/expression/RegexpLikeExprMacro.java

@@ -86,7 +86,7 @@ private RegexpLikeExpr(List<Expr> args)
 
       final String patternString = (String) patternExpr.getLiteralValue();
       this.arg = args.get(0);
-      this.pattern = Pattern.compile(StringUtils.nullToEmptyNonDruidDataString(patternString));
+      this.pattern = RegexpExprUtils.compilePattern(patternString, FN_NAME);
     }
 
     @Nonnull

processing/src/main/java/org/apache/druid/query/expression/RegexpReplaceExprMacro.java

@@ -96,7 +96,7 @@ private RegexpReplaceExpr(List<Expr> args)
       final String patternString = (String) patternExpr.getLiteralValue();
 
       this.arg = args.get(0);
-      this.pattern = patternString != null ? Pattern.compile(patternString) : null;
+      this.pattern = patternString != null ? RegexpExprUtils.compilePattern(patternString, FN_NAME) : null;
       this.replacement = (String) replacementExpr.getLiteralValue();
     }
 

processing/src/test/java/org/apache/druid/query/expression/RegexpExtractExprMacroTest.java

@@ -19,9 +19,12 @@
 
 package org.apache.druid.query.expression;
 
+import org.apache.druid.error.DruidException;
+import org.apache.druid.error.DruidExceptionMatcher;
 import org.apache.druid.math.expr.ExprEval;
 import org.apache.druid.math.expr.ExpressionType;
 import org.apache.druid.math.expr.InputBindings;
+import org.hamcrest.MatcherAssert;
 import org.junit.Assert;
 import org.junit.Test;
 
@@ -46,6 +49,23 @@ public void testErrorFourArguments()
     eval("regexp_extract('a', 'b', 'c', 'd')", InputBindings.nilBindings());
   }
 
+  @Test
+  public void testInvalidRegexpExtractPattern()
+  {
+    MatcherAssert.assertThat(
+        Assert.assertThrows(DruidException.class, () ->
+            eval(
+                "regexp_extract('pod-1234-node', '[ab-0-9]')",
+                InputBindings.forInputSupplier("a", ExpressionType.STRING, () -> "foo")
+            )
+        ),
+        DruidExceptionMatcher.invalidInput().expectMessageContains(
+            "An invalid pattern [[ab-0-9]] was provided for the [regexp_extract] function,"
+            + " error: [Illegal character range near index 4"
+        )
+    );
+  }
+
   @Test
   public void testMatch()
   {

processing/src/test/java/org/apache/druid/query/expression/RegexpLikeExprMacroTest.java

@@ -19,9 +19,12 @@
 
 package org.apache.druid.query.expression;
 
+import org.apache.druid.error.DruidException;
+import org.apache.druid.error.DruidExceptionMatcher;
 import org.apache.druid.math.expr.ExprEval;
 import org.apache.druid.math.expr.ExpressionType;
 import org.apache.druid.math.expr.InputBindings;
+import org.hamcrest.MatcherAssert;
 import org.junit.Assert;
 import org.junit.Test;
 
@@ -46,6 +49,20 @@ public void testErrorThreeArguments()
     eval("regexp_like('a', 'b', 'c')", InputBindings.nilBindings());
   }
 
+  @Test
+  public void testInvalidRegexpLikePattern()
+  {
+    MatcherAssert.assertThat(
+        Assert.assertThrows(
+            DruidException.class,
+            () -> eval("regexp_like('a', '[Ab-C]')", InputBindings.nilBindings())),
+        DruidExceptionMatcher.invalidInput().expectMessageContains(
+            "An invalid pattern [[Ab-C]] was provided for the [regexp_like] function,"
+            + " error: [Illegal character range near index 4"
+        )
+    );
+  }
+
   @Test
   public void testMatch()
   {

processing/src/test/java/org/apache/druid/query/expression/RegexpReplaceExprMacroTest.java

@@ -20,9 +20,12 @@
 package org.apache.druid.query.expression;
 
 import com.google.common.collect.ImmutableMap;
+import org.apache.druid.error.DruidException;
+import org.apache.druid.error.DruidExceptionMatcher;
 import org.apache.druid.math.expr.ExprEval;
 import org.apache.druid.math.expr.ExpressionType;
 import org.apache.druid.math.expr.InputBindings;
+import org.hamcrest.MatcherAssert;
 import org.junit.Assert;
 import org.junit.Test;
 
@@ -40,6 +43,20 @@ public void testErrorZeroArguments()
     eval("regexp_replace()", InputBindings.nilBindings());
   }
 
+  @Test
+  public void testInvalidRegexpReplacePattern()
+  {
+    MatcherAssert.assertThat(
+        Assert.assertThrows(
+            DruidException.class,
+            () -> eval("regexp_replace(a, '[Ab-cd-0]', 'xyz')", InputBindings.nilBindings())),
+        DruidExceptionMatcher.invalidInput().expectMessageContains(
+            "An invalid pattern [[Ab-cd-0]] was provided for the [regexp_replace] function,"
+            + " error: [Illegal character range near index 7"
+        )
+    );
+  }
+
   @Test
   public void testErrorFourArguments()
   {

sql/src/test/java/org/apache/druid/sql/calcite/CalciteQueryTest.java

@@ -8413,7 +8413,7 @@ public void testRegexpExtractWithBadRegexPattern()
         + "  REGEXP_EXTRACT(dim1, '^(.))', 1)\n"
         + "FROM foo",
         DruidExceptionMatcher.invalidInput().expectMessageContains(
-            "An invalid pattern [^(.))] was provided for the regexp_extract function, " +
+            "An invalid pattern [^(.))] was provided for the [regexp_extract] function, " +
             "error: [Unmatched closing ')' near index 3\n^(.))\n   ^]"
         )
     );