Browse Source

SQL: Improve error message when unable to translate to ES query DSL (#37129)

Improve error message returned to the client when an SQL statement
cannot be translated to a ES query DSL. Cases:

1. WHERE clause evaluates to FALSE => No results returned
1. Missing FROM clause => Local execution, e.g.: SELECT SIN(PI())
3. Special SQL command => Only valid of SQL iface, e.g.: SHOW TABLES

Fixes: #37040
Marios Trivyzas 6 years ago
parent
commit
e778abaac5

+ 35 - 0
x-pack/plugin/sql/qa/single-node/src/test/java/org/elasticsearch/xpack/sql/qa/single_node/RestSqlIT.java

@@ -5,11 +5,46 @@
  */
 package org.elasticsearch.xpack.sql.qa.single_node;
 
+import org.apache.http.entity.ContentType;
+import org.apache.http.entity.StringEntity;
 import org.elasticsearch.xpack.sql.qa.rest.RestSqlTestCase;
 
+import java.io.IOException;
+
+import static org.hamcrest.Matchers.containsString;
+
 /**
  * Integration test for the rest sql action. The one that speaks json directly to a
  * user rather than to the JDBC driver or CLI.
  */
 public class RestSqlIT extends RestSqlTestCase {
+
+
+    public void testErrorMessageForTranslatingQueryWithWhereEvaluatingToFalse() throws IOException {
+        index("{\"foo\":1}");
+        expectBadRequest(() -> runSql(
+            new StringEntity("{\"query\":\"SELECT * FROM test WHERE foo = 1 AND foo = 2\"}",
+                ContentType.APPLICATION_JSON), "/translate/"),
+            containsString("Cannot generate a query DSL for an SQL query that either its WHERE clause evaluates " +
+                "to FALSE or doesn't operate on a table (missing a FROM clause), sql statement: " +
+                "[SELECT * FROM test WHERE foo = 1 AND foo = 2]"));
+    }
+
+    public void testErrorMessageForTranslatingQueryWithLocalExecution() throws IOException {
+        index("{\"foo\":1}");
+        expectBadRequest(() -> runSql(
+            new StringEntity("{\"query\":\"SELECT SIN(PI())\"}",
+                ContentType.APPLICATION_JSON), "/translate/"),
+            containsString("Cannot generate a query DSL for an SQL query that either its WHERE clause evaluates " +
+                "to FALSE or doesn't operate on a table (missing a FROM clause), sql statement: [SELECT SIN(PI())]"));
+    }
+
+    public void testErrorMessageForTranslatingSQLCommandStatement() throws IOException {
+        index("{\"foo\":1}");
+        expectBadRequest(() -> runSql(
+            new StringEntity("{\"query\":\"SHOW FUNCTIONS\"}",
+                ContentType.APPLICATION_JSON), "/translate/"),
+            containsString("Cannot generate a query DSL for a special SQL command " +
+                "(e.g.: DESCRIBE, SHOW), sql statement: [SHOW FUNCTIONS]"));
+    }
 }

+ 3 - 4
x-pack/plugin/sql/qa/src/main/java/org/elasticsearch/xpack/sql/qa/rest/RestSqlTestCase.java

@@ -6,7 +6,6 @@
 package org.elasticsearch.xpack.sql.qa.rest;
 
 import com.fasterxml.jackson.core.io.JsonStringEncoder;
-
 import org.apache.http.HttpEntity;
 import org.apache.http.entity.ContentType;
 import org.apache.http.entity.StringEntity;
@@ -281,7 +280,7 @@ public abstract class RestSqlTestCase extends ESRestTestCase implements ErrorsTe
             containsString("line 1:12: [SCORE()] cannot be an argument to a function"));
     }
 
-    private void expectBadRequest(CheckedSupplier<Map<String, Object>, Exception> code, Matcher<String> errorMessageMatcher) {
+    protected void expectBadRequest(CheckedSupplier<Map<String, Object>, Exception> code, Matcher<String> errorMessageMatcher) {
         try {
             Map<String, Object> result = code.get();
             fail("expected ResponseException but got " + result);
@@ -310,7 +309,7 @@ public abstract class RestSqlTestCase extends ESRestTestCase implements ErrorsTe
         return runSql(new StringEntity("{\"query\":\"" + sql + "\"" + mode(mode) + "}", ContentType.APPLICATION_JSON), suffix);
     }
 
-    private Map<String, Object> runSql(HttpEntity sql, String suffix) throws IOException {
+    protected Map<String, Object> runSql(HttpEntity sql, String suffix) throws IOException {
         Request request = new Request("POST", "/_sql" + suffix);
         request.addParameter("error_trace", "true");   // Helps with debugging in case something crazy happens on the server.
         request.addParameter("pretty", "true");        // Improves error reporting readability
@@ -719,7 +718,7 @@ public abstract class RestSqlTestCase extends ESRestTestCase implements ErrorsTe
         return Strings.isEmpty(mode) ? StringUtils.EMPTY : ",\"mode\":\"" + mode + "\"";
     }
 
-    private void index(String... docs) throws IOException {
+    protected void index(String... docs) throws IOException {
         Request request = new Request("POST", "/test/_doc/_bulk");
         request.addParameter("refresh", "true");
         StringBuilder bulk = new StringBuilder();

+ 10 - 1
x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/execution/PlanExecutor.java

@@ -15,7 +15,9 @@ import org.elasticsearch.xpack.sql.analysis.index.IndexResolver;
 import org.elasticsearch.xpack.sql.execution.search.SourceGenerator;
 import org.elasticsearch.xpack.sql.expression.function.FunctionRegistry;
 import org.elasticsearch.xpack.sql.optimizer.Optimizer;
+import org.elasticsearch.xpack.sql.plan.physical.CommandExec;
 import org.elasticsearch.xpack.sql.plan.physical.EsQueryExec;
+import org.elasticsearch.xpack.sql.plan.physical.LocalExec;
 import org.elasticsearch.xpack.sql.planner.Planner;
 import org.elasticsearch.xpack.sql.planner.PlanningException;
 import org.elasticsearch.xpack.sql.proto.SqlTypedParamValue;
@@ -66,8 +68,15 @@ public class PlanExecutor {
             if (exec instanceof EsQueryExec) {
                 EsQueryExec e = (EsQueryExec) exec;
                 listener.onResponse(SourceGenerator.sourceBuilder(e.queryContainer(), cfg.filter(), cfg.pageSize()));
+            } else if (exec instanceof LocalExec) {
+                listener.onFailure(new PlanningException("Cannot generate a query DSL for an SQL query that either " +
+                    "its WHERE clause evaluates to FALSE or doesn't operate on a table (missing a FROM clause), sql statement: [{}]",
+                    sql));
+            } else if (exec instanceof CommandExec) {
+                listener.onFailure(new PlanningException("Cannot generate a query DSL for a special SQL command " +
+                    "(e.g.: DESCRIBE, SHOW), sql statement: [{}]", sql));
             } else {
-                listener.onFailure(new PlanningException("Cannot generate a query DSL for {}", sql));
+                listener.onFailure(new PlanningException("Cannot generate a query DSL, sql statement: [{}]", sql));
             }
         }, listener::onFailure));
     }