[7.11] QL: Improve exact match detection in StringPattern (#68754) (#…

…68777) Small refactor to internal class to handle discovery of the exact match across all implementations without the need of manual escaping.
elastic · Feb 9, 2021 · 01ce714 · 01ce714
1 parent cab961a
commit 01ce714
Show file tree

Hide file tree

Showing 7 changed files with 28 additions and 37 deletions.
diff --git a/x-pack/plugin/eql/src/test/java/org/elasticsearch/xpack/eql/StringUtilsTests.java b/x-pack/plugin/eql/src/test/java/org/elasticsearch/xpack/eql/StringUtilsTests.java
@@ -19,14 +19,14 @@ public class StringUtilsTests extends ESTestCase {
 
     public void testLikePatternNoPattern() throws Exception {
         String string = "abc123";
-        assertEquals(string, toLikePattern(string).asString());
+        assertEquals(string, toLikePattern(string).exactMatch());
     }
 
     public void testLikePatternLikeChars() throws Exception {
         String string = "a%bc%%12_3__";
         String escape = Character.toString((char) 1);
         LikePattern pattern = toLikePattern(string);
-        assertEquals(string, pattern.asString());
+        assertEquals(string, pattern.exactMatch());
         assertEquals("a" + escape + "%bc" + escape + "%" + escape + "%" +
             "12" + escape + "_" + "3" + escape + "_" + escape + "_", pattern.pattern());
         assertEquals(string, pattern.asLuceneWildcard());
@@ -35,7 +35,7 @@ public void testLikePatternLikeChars() throws Exception {
     public void testLikePatternEqlChars() throws Exception {
         String string = "abc*123?";
         LikePattern pattern = toLikePattern(string);
-        assertEquals("abc%123_", pattern.asString());
+        assertNull(pattern.exactMatch());
         assertEquals("abc%123_", pattern.pattern());
         assertEquals(string, pattern.asLuceneWildcard());
     }
@@ -44,24 +44,25 @@ public void testLikePatternMixEqlAndLikeChars() throws Exception {
         String string = "abc*%123?_";
         String escape = Character.toString((char) 1);
         LikePattern pattern = toLikePattern(string);
-        assertEquals("abc%%123__", pattern.asString());
+        assertNull(pattern.exactMatch());
         assertEquals("abc%" + escape + "%123_" + escape + "_", pattern.pattern());
         assertEquals(string, pattern.asLuceneWildcard());
     }
 
     public void testIsExactMatch() throws Exception {
         List<String> list = asList("abc%123", "abc_123", "abc%%123__");
-        for (String string : list) {
+        for (int i = 0; i < list.size(); i++) {
+            String string = list.get(i);
             LikePattern pattern = toLikePattern(string);
-            assertTrue(pattern.isExactMatch());
+            assertEquals(string, pattern.exactMatch());
         }
     }
 
     public void testIsNonExactMatch() throws Exception {
         List<String> list = asList("abc*123", "abc?123", "abc**123??");
         for (String string : list) {
             LikePattern pattern = toLikePattern(string);
-            assertFalse(pattern.isExactMatch());
+            assertNull(pattern.exactMatch());
         }
     }
 }
diff --git a/...ain/java/org/elasticsearch/xpack/ql/expression/predicate/regex/AbstractStringPattern.java b/...ain/java/org/elasticsearch/xpack/ql/expression/predicate/regex/AbstractStringPattern.java
@@ -7,6 +7,8 @@
 
 package org.elasticsearch.xpack.ql.expression.predicate.regex;
 
+import org.apache.lucene.util.IntsRef;
+import org.apache.lucene.util.UnicodeUtil;
 import org.apache.lucene.util.automaton.Automaton;
 import org.apache.lucene.util.automaton.Operations;
 
@@ -29,7 +31,8 @@ public boolean matchesAll() {
     }
 
     @Override
-    public boolean isExactMatch() {
-        return Operations.getCommonPrefix(automaton()).equals(asString());
+    public String exactMatch() {
+        IntsRef singleton = Operations.getSingleton(automaton());
+        return singleton != null ? UnicodeUtil.newString(singleton.ints, singleton.offset, singleton.length) : null;
     }
 }
diff --git a/...n/ql/src/main/java/org/elasticsearch/xpack/ql/expression/predicate/regex/LikePattern.java b/...n/ql/src/main/java/org/elasticsearch/xpack/ql/expression/predicate/regex/LikePattern.java
@@ -31,7 +31,6 @@ public class LikePattern extends AbstractStringPattern {
     private final String regex;
     private final String wildcard;
     private final String indexNameWildcard;
-    private final String string;
 
     public LikePattern(String pattern, char escape) {
         this.pattern = pattern;
@@ -40,7 +39,6 @@ public LikePattern(String pattern, char escape) {
         this.regex = StringUtils.likeToJavaPattern(pattern, escape);
         this.wildcard = StringUtils.likeToLuceneWildcard(pattern, escape);
         this.indexNameWildcard = StringUtils.likeToIndexWildcard(pattern, escape);
-        this.string = pattern.replace(Character.toString(escape), StringUtils.EMPTY);
     }
 
     public String pattern() {
@@ -62,11 +60,6 @@ public String asJavaRegex() {
         return regex;
     }
 
-    @Override
-    public String asString() {
-        return string;
-    }
-
     /**
      * Returns the pattern in (Lucene) wildcard format.
      */

diff --git a/.../ql/src/main/java/org/elasticsearch/xpack/ql/expression/predicate/regex/RLikePattern.java b/.../ql/src/main/java/org/elasticsearch/xpack/ql/expression/predicate/regex/RLikePattern.java
@@ -22,11 +22,6 @@ Automaton createAutomaton() {
         return new RegExp(regexpPattern).toAutomaton();
     }
 
-    @Override
-    public String asString() {
-        return regexpPattern;
-    }
-
     @Override
     public String asJavaRegex() {
         return regexpPattern;

diff --git a/...ql/src/main/java/org/elasticsearch/xpack/ql/expression/predicate/regex/StringPattern.java b/...ql/src/main/java/org/elasticsearch/xpack/ql/expression/predicate/regex/StringPattern.java
@@ -12,11 +12,6 @@ public interface StringPattern {
      */
     String asJavaRegex();
 
-    /**
-     * Returns the pattern as a string. Should handle escaping.
-     */
-    String asString();
-
     /**
      * Hint method on whether this pattern matches everything or not.
      */
@@ -25,10 +20,9 @@ default boolean matchesAll() {
     }
 
     /**
-     * Hint method on whether this pattern is exact, that is has no wildcard
+     * Returns the match if this pattern is exact, that is has no wildcard
      * or other patterns inside.
+     * If the pattern is not exact, null is returned.
      */
-    default boolean isExactMatch() {
-        return false;
-    }
+    String exactMatch();
 }
diff --git a/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/optimizer/OptimizerRules.java b/x-pack/plugin/ql/src/main/java/org/elasticsearch/xpack/ql/optimizer/OptimizerRules.java
@@ -1233,16 +1233,19 @@ public ReplaceRegexMatch() {
             super(TransformDirection.DOWN);
         }
 
+        @Override
         protected Expression rule(Expression e) {
             if (e instanceof RegexMatch) {
                 RegexMatch<?> regexMatch = (RegexMatch<?>) e;
                 StringPattern pattern = regexMatch.pattern();
                 if (pattern.matchesAll()) {
                     e = new IsNotNull(e.source(), regexMatch.field());
-                }
-                else if (pattern.isExactMatch()) {
-                    Literal literal = new Literal(regexMatch.source(), regexMatch.pattern().asString(), DataTypes.KEYWORD);
-                    e = new Equals(e.source(), regexMatch.field(), literal);
+                } else {
+                    String match = pattern.exactMatch();
+                    if (match != null) {
+                        Literal literal = new Literal(regexMatch.source(), match, DataTypes.KEYWORD);
+                        e = new Equals(e.source(), regexMatch.field(), literal);
+                    }
                 }
             }
             return e;
@@ -1259,7 +1262,7 @@ public LogicalPlan apply(LogicalPlan plan) {
 
         @Override
         protected LogicalPlan rule(LogicalPlan plan) {
-            if (!plan.optimized()) {
+            if (plan.optimized() == false) {
                 plan.setOptimized();
             }
             return plan;

diff --git a/...c/test/java/org/elasticsearch/xpack/ql/expression/predicate/regex/StringPatternTests.java b/...c/test/java/org/elasticsearch/xpack/ql/expression/predicate/regex/StringPatternTests.java
@@ -8,6 +8,7 @@
 package org.elasticsearch.xpack.ql.expression.predicate.regex;
 
 import org.elasticsearch.test.ESTestCase;
+import org.elasticsearch.xpack.ql.util.StringUtils;
 
 public class StringPatternTests extends ESTestCase {
 
@@ -24,15 +25,16 @@ private boolean matchesAll(String pattern, char escape) {
     }
 
     private boolean exactMatch(String pattern, char escape) {
-        return like(pattern, escape).isExactMatch();
+        String escaped = pattern.replace(Character.toString(escape), StringUtils.EMPTY);
+        return escaped.equals(like(pattern, escape).exactMatch());
     }
 
     private boolean matchesAll(String pattern) {
         return rlike(pattern).matchesAll();
     }
 
     private boolean exactMatch(String pattern) {
-        return rlike(pattern).isExactMatch();
+        return pattern.equals(rlike(pattern).exactMatch());
     }
 
     public void testWildcardMatchAll() throws Exception {