[GitHub] [drill] vvysotskyi commented on a diff in pull request #2762: DRILL-8402: Add REGEXP_EXTRACT Function

["vvysotskyi (via GitHub)" <gi...@apache.org>]

vvysotskyi commented on code in PR #2762:
URL: https://github.com/apache/drill/pull/2762#discussion_r1114857738


##########
exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/StringFunctions.java:
##########
@@ -293,6 +293,115 @@ public void eval() {
     }
   }
 
+  /*
+   * This function returns the capturing groups from a regex.
+   */
+  @FunctionTemplate(name = "regexp_extract", scope = FunctionScope.SIMPLE,
+      outputWidthCalculatorType = OutputWidthCalculatorType.CUSTOM_FIXED_WIDTH_DEFAULT)
+  public static class RegexpExtract implements DrillSimpleFunc {
+
+    @Param VarCharHolder input;
+    @Param(constant=true) VarCharHolder pattern;
+    @Inject
+    DrillBuf buffer;
+    @Workspace
+    java.util.regex.Matcher matcher;
+    @Workspace
+    org.apache.drill.exec.expr.fn.impl.CharSequenceWrapper charSequenceWrapper;
+    @Output
+    ComplexWriter out;
+
+    @Override
+    public void setup() {
+      matcher = java.util.regex.Pattern.compile(org.apache.drill.exec.expr.fn.impl.StringFunctionHelpers.toStringFromUTF8(pattern.start,  pattern.end,  pattern.buffer)).matcher("");
+      charSequenceWrapper = new org.apache.drill.exec.expr.fn.impl.CharSequenceWrapper();
+      matcher.reset(charSequenceWrapper);
+    }
+
+    @Override
+    public void eval() {
+      charSequenceWrapper.setBuffer(input.start, input.end, input.buffer);
+
+      // Reusing same charSequenceWrapper, no need to pass it in.
+      matcher.reset();
+      boolean result = matcher.find();
+
+      // Start the list here.  If there are no matches, we return an empty list.
+      org.apache.drill.exec.vector.complex.writer.BaseWriter.ListWriter listWriter = out.rootAsList();
+      listWriter.startList();
+
+      if (result) {
+        org.apache.drill.exec.vector.complex.writer.VarCharWriter varCharWriter = listWriter.varChar();
+        String extractedResult;
+        for(int i = 1; i <= matcher.groupCount(); i++) {
+          extractedResult = matcher.group(i);

Review Comment:
   It is better to avoid creating extra objects in UDFs to reduce the load on the garbage collector. Matcher has `Matcher.start(int group)` and `Matcher.end(int group)`, so please use them to obtain bytes that correspond to marching subsequence.



##########
NOTICE:
##########
@@ -1,5 +1,5 @@
 Apache Drill
-Copyright 2013-2022 The Apache Software Foundation
+Copyright 2013-2023 The Apache Software Foundation

Review Comment:
   Looks like this PR should be rebased on the latest master. Probably these changes are present because of the force push to master.



##########
exec/java-exec/src/main/java/org/apache/drill/exec/expr/fn/impl/CharSequenceWrapper.java:
##########
@@ -90,7 +90,10 @@ public char charAt(int index) {
    */
   @Override
   public CharSequence subSequence(int start, int end) {
-    throw new UnsupportedOperationException();
+    // throw new UnsupportedOperationException();

Review Comment:
   Please remove commented code.



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: dev-unsubscribe@drill.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org