Term range query against runtime fields to accept open ranges (#68056)

Range query allows to specify only one end of the range, or even none. The implementation of range query against keyword runtime fields had both ends mandatory which was an oversight.

Closes #67994

x-pack/plugin/runtime-fields/src/main/java/org/elasticsearch/xpack/runtimefields/mapper/KeywordScriptFieldType.java

@@ -139,8 +139,8 @@ public final class KeywordScriptFieldType extends AbstractScriptFieldType<String
             script,
             leafFactory(context),
             name(),
-            BytesRefs.toString(Objects.requireNonNull(lowerTerm)),
-            BytesRefs.toString(Objects.requireNonNull(upperTerm)),
+            lowerTerm == null ? null : BytesRefs.toString(lowerTerm),
+            upperTerm == null ? null : BytesRefs.toString(upperTerm),
             includeLower,
             includeUpper
         );

x-pack/plugin/runtime-fields/src/main/java/org/elasticsearch/xpack/runtimefields/query/StringScriptFieldRangeQuery.java

@@ -32,21 +32,32 @@ public class StringScriptFieldRangeQuery extends AbstractStringScriptFieldQuery
         boolean includeUpper
     ) {
         super(script, leafFactory, fieldName);
-        this.lowerValue = Objects.requireNonNull(lowerValue);
-        this.upperValue = Objects.requireNonNull(upperValue);
+        if (lowerValue == null && includeLower == false) {
+            throw new IllegalArgumentException("includeLower must be true when lowerValue is null (open ended)");
+        }
+        if (upperValue == null && includeUpper == false) {
+            throw new IllegalArgumentException("includeUpper must be true when upperValue is null (open ended)");
+        }
+        this.lowerValue = lowerValue;
+        this.upperValue = upperValue;
         this.includeLower = includeLower;
         this.includeUpper = includeUpper;
-        assert lowerValue.compareTo(upperValue) <= 0;
     }
 
     @Override
     protected boolean matches(List<String> values) {
         for (String value : values) {
-            int lct = lowerValue.compareTo(value);
-            boolean lowerOk = includeLower ? lct <= 0 : lct < 0;
+            boolean lowerOk = true;
+            if (lowerValue != null) {
+                int lct = lowerValue.compareTo(value);
+                lowerOk = includeLower ? lct <= 0 : lct < 0;
+            }
             if (lowerOk) {
-                int uct = upperValue.compareTo(value);
-                boolean upperOk = includeUpper ? uct >= 0 : uct > 0;
+                boolean upperOk = true;
+                if (upperValue != null) {
+                    int uct = upperValue.compareTo(value);
+                    upperOk = includeUpper ? uct >= 0 : uct > 0;
+                }
                 if (upperOk) {
                     return true;
                 }
@@ -62,7 +73,12 @@ public class StringScriptFieldRangeQuery extends AbstractStringScriptFieldQuery
                 this,
                 fieldName(),
                 () -> new ByteRunAutomaton(
-                    Automata.makeBinaryInterval(new BytesRef(lowerValue), includeLower, new BytesRef(upperValue), includeUpper)
+                    Automata.makeBinaryInterval(
+                        lowerValue == null ? null : new BytesRef(lowerValue),
+                        includeLower,
+                        upperValue == null ? null : new BytesRef(upperValue),
+                        includeUpper
+                    )
                 )
             );
         }
@@ -75,7 +91,9 @@ public class StringScriptFieldRangeQuery extends AbstractStringScriptFieldQuery
             b.append(fieldName()).append(':');
         }
         b.append(includeLower ? '[' : '{');
-        b.append(lowerValue).append(" TO ").append(upperValue);
+        b.append(lowerValue == null ? "*" : lowerValue);
+        b.append(" TO ");
+        b.append(upperValue == null ? "*" : upperValue);
         b.append(includeUpper ? ']' : '}');
         return b.toString();
     }
@@ -91,8 +109,8 @@ public class StringScriptFieldRangeQuery extends AbstractStringScriptFieldQuery
             return false;
         }
         StringScriptFieldRangeQuery other = (StringScriptFieldRangeQuery) obj;
-        return lowerValue.equals(other.lowerValue)
-            && upperValue.equals(other.upperValue)
+        return Objects.equals(lowerValue, other.lowerValue)
+            && Objects.equals(upperValue, other.upperValue)
             && includeLower == other.includeLower
             && includeUpper == other.includeUpper;
     }

x-pack/plugin/runtime-fields/src/test/java/org/elasticsearch/xpack/runtimefields/mapper/KeywordScriptFieldTypeTests.java

@@ -173,11 +173,11 @@ public class KeywordScriptFieldTypeTests extends AbstractScriptFieldTypeTestCase
         }
     }
 
-    public void testFuzzyQueryIsExpensive() throws IOException {
+    public void testFuzzyQueryIsExpensive() {
         checkExpensiveQuery(this::randomFuzzyQuery);
     }
 
-    public void testFuzzyQueryInLoop() throws IOException {
+    public void testFuzzyQueryInLoop() {
         checkLoop(this::randomFuzzyQuery);
     }
 
@@ -204,11 +204,11 @@ public class KeywordScriptFieldTypeTests extends AbstractScriptFieldTypeTestCase
         }
     }
 
-    public void testPrefixQueryIsExpensive() throws IOException {
+    public void testPrefixQueryIsExpensive() {
         checkExpensiveQuery(this::randomPrefixQuery);
     }
 
-    public void testPrefixQueryInLoop() throws IOException {
+    public void testPrefixQueryInLoop() {
         checkLoop(this::randomPrefixQuery);
     }
 
@@ -228,17 +228,31 @@ public class KeywordScriptFieldTypeTests extends AbstractScriptFieldTypeTestCase
                     searcher.count(simpleMappedFieldType().rangeQuery("cat", "d", false, false, null, null, null, mockContext())),
                     equalTo(1)
                 );
+                assertThat(
+                    searcher.count(simpleMappedFieldType().rangeQuery(null, "d", true, false, null, null, null, mockContext())),
+                    equalTo(2)
+                );
+                assertThat(
+                    searcher.count(simpleMappedFieldType().rangeQuery("cat", null, false, true, null, null, null, mockContext())),
+                    equalTo(2)
+                );
+                assertThat(
+                    searcher.count(simpleMappedFieldType().rangeQuery(null, null, true, true, null, null, null, mockContext())),
+                    equalTo(3)
+                );
             }
         }
     }
 
     @Override
     protected Query randomRangeQuery(MappedFieldType ft, SearchExecutionContext ctx) {
+        boolean lowerNull = randomBoolean();
+        boolean upperNull = randomBoolean();
         return ft.rangeQuery(
-            randomAlphaOfLengthBetween(0, 1000),
-            randomAlphaOfLengthBetween(0, 1000),
-            randomBoolean(),
-            randomBoolean(),
+            lowerNull ? null : randomAlphaOfLengthBetween(0, 1000),
+            upperNull ? null : randomAlphaOfLengthBetween(0, 1000),
+            lowerNull || randomBoolean(),
+            upperNull || randomBoolean(),
             null,
             null,
             null,
@@ -319,11 +333,11 @@ public class KeywordScriptFieldTypeTests extends AbstractScriptFieldTypeTestCase
         }
     }
 
-    public void testWildcardQueryIsExpensive() throws IOException {
+    public void testWildcardQueryIsExpensive() {
         checkExpensiveQuery(this::randomWildcardQuery);
     }
 
-    public void testWildcardQueryInLoop() throws IOException {
+    public void testWildcardQueryInLoop() {
         checkLoop(this::randomWildcardQuery);
     }
 

x-pack/plugin/runtime-fields/src/test/java/org/elasticsearch/xpack/runtimefields/query/StringScriptFieldRangeQueryTests.java

@@ -18,21 +18,43 @@ import static org.hamcrest.Matchers.is;
 public class StringScriptFieldRangeQueryTests extends AbstractStringScriptFieldQueryTestCase<StringScriptFieldRangeQuery> {
     @Override
     protected StringScriptFieldRangeQuery createTestInstance() {
-        String lower = randomAlphaOfLength(3);
-        String upper = randomValueOtherThan(lower, () -> randomAlphaOfLength(3));
-        if (lower.compareTo(upper) > 0) {
-            String tmp = lower;
-            lower = upper;
-            upper = tmp;
-        }
+        String lower = randomBoolean() ? null : randomAlphaOfLength(3);
+        String upper = randomBoolean() ? null : randomAlphaOfLength(3);
         return new StringScriptFieldRangeQuery(
             randomScript(),
             leafFactory,
             randomAlphaOfLength(5),
             lower,
             upper,
-            randomBoolean(),
-            randomBoolean()
+            lower == null || randomBoolean(),
+            upper == null || randomBoolean()
+        );
+    }
+
+    public void testValidate() {
+        expectThrows(
+            IllegalArgumentException.class,
+            () -> new StringScriptFieldRangeQuery(
+                randomScript(),
+                leafFactory,
+                randomAlphaOfLength(5),
+                null,
+                randomAlphaOfLength(3),
+                false,
+                randomBoolean()
+            )
+        );
+        expectThrows(
+            IllegalArgumentException.class,
+            () -> new StringScriptFieldRangeQuery(
+                randomScript(),
+                leafFactory,
+                randomAlphaOfLength(5),
+                randomAlphaOfLength(3),
+                null,
+                randomBoolean(),
+                false
+            )
         );
     }
 
@@ -65,15 +87,27 @@ public class StringScriptFieldRangeQueryTests extends AbstractStringScriptFieldQ
                 fieldName += "modified";
                 break;
             case 2:
-                lower += "modified";
+                lower = mutate(lower);
+                if (lower == null) {
+                    includeLower = true;
+                }
                 break;
             case 3:
-                upper += "modified";
+                upper = mutate(upper);
+                if (upper == null) {
+                    includeUpper = true;
+                }
                 break;
             case 4:
+                if (lower == null) {
+                    lower = mutate(lower);
+                }
                 includeLower = !includeLower;
                 break;
             case 5:
+                if (upper == null) {
+                    upper = mutate(upper);
+                }
                 includeUpper = !includeUpper;
                 break;
             default:
@@ -82,6 +116,16 @@ public class StringScriptFieldRangeQueryTests extends AbstractStringScriptFieldQ
         return new StringScriptFieldRangeQuery(script, leafFactory, fieldName, lower, upper, includeLower, includeUpper);
     }
 
+    private static String mutate(String term) {
+        if (term == null) {
+            return randomAlphaOfLength(3);
+        }
+        if (randomBoolean()) {
+            return null;
+        }
+        return term + "modified";
+    }
+
     @Override
     public void testMatches() {
         StringScriptFieldRangeQuery query = new StringScriptFieldRangeQuery(randomScript(), leafFactory, "test", "a", "b", true, true);
@@ -95,26 +139,72 @@ public class StringScriptFieldRangeQueryTests extends AbstractStringScriptFieldQ
         assertTrue(query.matches(List.of("ab")));
         assertFalse(query.matches(List.of("b")));
         assertFalse(query.matches(List.of("ba")));
+        query = new StringScriptFieldRangeQuery(randomScript(), leafFactory, "test", "b", "a", randomBoolean(), randomBoolean());
+        assertFalse(query.matches(List.of("a")));
+        assertFalse(query.matches(List.of("ab")));
+        assertFalse(query.matches(List.of("b")));
+        assertFalse(query.matches(List.of("ba")));
+        query = new StringScriptFieldRangeQuery(randomScript(), leafFactory, "test", null, "b", true, false);
+        assertTrue(query.matches(List.of("a")));
+        assertTrue(query.matches(List.of("ab")));
+        assertFalse(query.matches(List.of("b")));
+        assertFalse(query.matches(List.of("ba")));
+        query = new StringScriptFieldRangeQuery(randomScript(), leafFactory, "test", "a", null, false, true);
+        assertFalse(query.matches(List.of("a")));
+        assertTrue(query.matches(List.of("ab")));
+        assertTrue(query.matches(List.of("b")));
+        assertTrue(query.matches(List.of("ba")));
+        assertTrue(query.matches(List.of("z")));
+        query = new StringScriptFieldRangeQuery(randomScript(), leafFactory, "test", null, null, true, true);
+        assertTrue(query.matches(List.of("a")));
+        assertTrue(query.matches(List.of("ab")));
+        assertTrue(query.matches(List.of("b")));
+        assertTrue(query.matches(List.of("ba")));
+        assertTrue(query.matches(List.of("z")));
     }
 
     @Override
     protected void assertToString(StringScriptFieldRangeQuery query) {
         assertThat(query.toString(query.fieldName()), containsString(query.includeLower() ? "[" : "{"));
         assertThat(query.toString(query.fieldName()), containsString(query.includeUpper() ? "]" : "}"));
-        assertThat(query.toString(query.fieldName()), containsString(query.lowerValue() + " TO " + query.upperValue()));
+        String lowerValue = query.lowerValue() == null ? "*" : query.lowerValue();
+        String upperValue = query.upperValue() == null ? "*" : query.upperValue();
+        assertThat(query.toString(query.fieldName()), containsString(lowerValue + " TO " + upperValue));
     }
 
     @Override
     public void testVisit() {
         StringScriptFieldRangeQuery query = createTestInstance();
         ByteRunAutomaton automaton = visitForSingleAutomata(query);
-        BytesRef term = new BytesRef(query.lowerValue() + "a");
-        assertThat(automaton.run(term.bytes, term.offset, term.length), is(true));
-        term = new BytesRef(query.lowerValue());
-        assertThat(automaton.run(term.bytes, term.offset, term.length), is(query.includeLower()));
-        term = new BytesRef(query.upperValue() + "a");
-        assertThat(automaton.run(term.bytes, term.offset, term.length), is(false));
-        term = new BytesRef(query.upperValue());
-        assertThat(automaton.run(term.bytes, term.offset, term.length), is(query.includeUpper()));
+        boolean validRange = true;
+        if (query.lowerValue() != null && query.upperValue() != null) {
+            validRange = query.lowerValue().compareTo(query.upperValue()) < 0;
+        }
+        String termString;
+        if (query.lowerValue() == null) {
+            if (query.upperValue() == null) {
+                termString = randomAlphaOfLength(1);
+            } else {
+                termString = randomValueOtherThanMany(value -> {
+                    int cmp = value.compareTo(query.upperValue());
+                    return query.includeUpper() ? cmp > 0 : cmp >= 0;
+                }, () -> randomAlphaOfLength(1));
+            }
+        } else {
+            termString = query.lowerValue() + "a";
+        }
+        BytesRef term = new BytesRef(termString);
+        assertThat(automaton.run(term.bytes, term.offset, term.length), is(validRange));
+
+        if (query.lowerValue() != null) {
+            term = new BytesRef(query.lowerValue());
+            assertThat(automaton.run(term.bytes, term.offset, term.length), is(query.includeLower() && validRange));
+        }
+        if (query.upperValue() != null) {
+            term = new BytesRef(query.upperValue() + "a");
+            assertThat(automaton.run(term.bytes, term.offset, term.length), is(false));
+            term = new BytesRef(query.upperValue());
+            assertThat(automaton.run(term.bytes, term.offset, term.length), is(query.includeUpper() && validRange));
+        }
     }
 }