Add verification for QSTR attribute

carlosdelest · Sep 11, 2024 · 7b2cbaa · 7b2cbaa
1 parent 9d5e7b3
commit 7b2cbaa
Show file tree

Hide file tree

Showing 4 changed files with 37 additions and 8 deletions.
diff --git a/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Verifier.java b/x-pack/plugin/esql/src/main/java/org/elasticsearch/xpack/esql/analysis/Verifier.java
@@ -668,7 +668,7 @@ private static void checkMatchCommand(LogicalPlan plan, Set<Failure> failures) {
     private static void checkFullTextQueryFunctions(LogicalPlan plan, Set<Failure> failures) {
         if (plan instanceof Filter f) {
             Expression condition = f.condition();
-            if (condition instanceof FullTextFunction) {
+            if (condition instanceof FullTextFunction ftf) {
                 // Similar to cases present in org.elasticsearch.xpack.esql.optimizer.rules.PushDownAndCombineFilters -
                 // we can't check if it can be pushed down as we don't have yet information about the fields present in the
                 // StringQueryPredicate
@@ -683,6 +683,15 @@ private static void checkFullTextQueryFunctions(LogicalPlan plan, Set<Failure> f
                         );
                     }
                 });
+                if (ftf.query().foldable() == false) {
+                    failures.add(
+                        fail(
+                            ftf,
+                            "Query in {} function needs to be statically resolved. References to fields are not allowed.",
+                            ftf.functionName()
+                        )
+                    );
+                }
             }
         }
     }

diff --git a/...main/java/org/elasticsearch/xpack/esql/expression/function/fulltext/FullTextFunction.java b/...main/java/org/elasticsearch/xpack/esql/expression/function/fulltext/FullTextFunction.java
@@ -23,6 +23,8 @@
 import java.util.List;
 
 import static java.util.Collections.singletonList;
+import static org.elasticsearch.xpack.esql.core.expression.TypeResolutions.ParamOrdinal.DEFAULT;
+import static org.elasticsearch.xpack.esql.core.expression.TypeResolutions.isString;
 
 /**
  * Base class for full-text functions that use ES queries to match documents.
@@ -54,6 +56,15 @@ public DataType dataType() {
         return DataType.BOOLEAN;
     }
 
+    @Override
+    protected TypeResolution resolveType() {
+        if (childrenResolved() == false) {
+            return new TypeResolution("Unresolved children");
+        }
+
+        return isString(query(), sourceText(), DEFAULT);
+    }
+
     public Expression query() {
         return query;
     }

diff --git a/...n/java/org/elasticsearch/xpack/esql/expression/function/fulltext/QueryStringFunction.java b/...n/java/org/elasticsearch/xpack/esql/expression/function/fulltext/QueryStringFunction.java
@@ -11,7 +11,6 @@
 import org.elasticsearch.common.io.stream.NamedWriteableRegistry;
 import org.elasticsearch.common.io.stream.StreamInput;
 import org.elasticsearch.xpack.esql.core.expression.Expression;
-import org.elasticsearch.xpack.esql.core.expression.Foldables;
 import org.elasticsearch.xpack.esql.core.querydsl.query.Query;
 import org.elasticsearch.xpack.esql.core.querydsl.query.QueryStringQuery;
 import org.elasticsearch.xpack.esql.core.tree.NodeInfo;
@@ -55,16 +54,19 @@ private QueryStringFunction(StreamInput in) throws IOException {
         super(in);
     }
 
+    @Override
+    public String functionName() {
+        return "QSTR";
+    }
+
     @Override
     public Query asQuery() {
-        Object queryAsObject = Foldables.valueOf(query());
-        String queryAsString = null;
+        Object queryAsObject = query().fold();
         if (queryAsObject instanceof BytesRef queryAsBytesRef) {
-            queryAsString = queryAsBytesRef.utf8ToString();
+            return new QueryStringQuery(source(), queryAsBytesRef.utf8ToString(), Map.of(), null);
         } else {
-            queryAsString = queryAsObject.toString();
+            throw new IllegalArgumentException("Query in QSTR needs to be resolved to a string");
         }
-        return new QueryStringQuery(source(), queryAsString, Map.of(), null);
     }
 
     @Override

diff --git a/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/VerifierTests.java b/x-pack/plugin/esql/src/test/java/org/elasticsearch/xpack/esql/analysis/VerifierTests.java
@@ -1110,8 +1110,15 @@ public void testQueryStringFunctionsNotAllowed() throws Exception {
             error("from test | grok last_name \"%{WORD:foo}\" | where qstr( \"Anna\")")
         );
         assertEquals("1:27: Full text functions cannot be used after KEEP", error("from test | keep emp_no | where qstr( \"Anna\")"));
+    }
 
-        // TODO Keep adding tests for all unsupported commands
+    public void testQueryStringFunctionOperands() throws Exception {
+        assumeTrue("skipping because QSTR is not enabled", EsqlCapabilities.Cap.QSTR_FUNCTION.isEnabled());
+        assertEquals("1:19: argument of [qstr(10)] must be [string], found value [10] type [integer]", error("from test | where qstr(10)"));
+        assertEquals(
+            "1:19: Query in QSTR function needs to be statically resolved. References to fields are not allowed.",
+            error("from test | where qstr(first_name)")
+        );
     }
 
     public void testCoalesceWithMixedNumericTypes() {