SQL: Add more error integration tests (elastic/x-pack-elasticsearch#3134)

We didn't have many integration tests for errors other than the security
errors. This adds some and sets up a way we can make sure we are
consistent across the REST, JDBC, and CLI.

relates elastic/x-pack-elasticsearch#3033

Original commit: elastic/x-pack-elasticsearch@debbb2ec46

qa/sql/src/main/java/org/elasticsearch/xpack/qa/sql/ErrorsTestCase.java

@@ -0,0 +1,18 @@
+/*
+ * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
+ * or more contributor license agreements. Licensed under the Elastic License;
+ * you may not use this file except in compliance with the Elastic License.
+ */
+package org.elasticsearch.xpack.qa.sql;
+
+/**
+ * Interface implemented once per SQL access method to ensure that we
+ * test the same minimal set of error cases. Note that this does not
+ * include security related failures, those are tracked in another test.
+ */
+public interface ErrorsTestCase {
+    void testSelectInvalidSql() throws Exception;
+    void testSelectFromMissingIndex() throws Exception;
+    void testSelectMissingField() throws Exception;
+    void testSelectMissingFunction() throws Exception;
+}

qa/sql/src/main/java/org/elasticsearch/xpack/qa/sql/cli/ErrorsTestCase.java

@@ -5,12 +5,35 @@
  */
 package org.elasticsearch.xpack.qa.sql.cli;
 
+import java.io.IOException;
+
 /**
  * Tests for error messages.
  */
-public abstract class ErrorsTestCase extends CliIntegrationTestCase {
+public abstract class ErrorsTestCase extends CliIntegrationTestCase implements org.elasticsearch.xpack.qa.sql.ErrorsTestCase {
-    public void testSelectFromMissingTable() throws Exception {
+    @Override
+    public void testSelectInvalidSql() throws Exception {
+        assertEquals("[1;31mBad request [[22;3;33mFound 1 problem(s)", command("SELECT * FRO"));
+        assertEquals("line 1:8: Cannot determine columns for *[1;23;31m][0m", readLine());
+    }
+
+    @Override
+    public void testSelectFromMissingIndex() throws IOException {
         assertEquals("[1;31mBad request [[22;3;33mFound 1 problem(s)", command("SELECT * FROM test"));
         assertEquals("line 1:15: Unknown index [test][1;23;31m][0m", readLine());
     }
+
+    @Override
+    public void testSelectMissingField() throws IOException {
+        index("test", body -> body.field("test", "test"));
+        assertEquals("[1;31mBad request [[22;3;33mFound 1 problem(s)", command("SELECT missing FROM test"));
+        assertEquals("line 1:8: Unknown column [missing][1;23;31m][0m", readLine());
+    }
+
+    @Override
+    public void testSelectMissingFunction() throws Exception {
+        index("test", body -> body.field("foo", 1));
+        assertEquals("[1;31mBad request [[22;3;33mFound 1 problem(s)", command("SELECT missing(foo) FROM test"));
+        assertEquals("line 1:8: Unknown function [missing][1;23;31m][0m", readLine());
+    }
 }

qa/sql/src/main/java/org/elasticsearch/xpack/qa/sql/jdbc/ErrorsTestCase.java

@@ -11,11 +11,38 @@ import java.sql.SQLException;
 /**
  * Tests for exceptions and their messages.
  */
-public class ErrorsTestCase extends JdbcIntegrationTestCase {
+public class ErrorsTestCase extends JdbcIntegrationTestCase implements org.elasticsearch.xpack.qa.sql.ErrorsTestCase {
-    public void testSelectFromMissingTable() throws Exception {
+    @Override
+    public void testSelectInvalidSql() throws Exception {
         try (Connection c = esJdbc()) {
-            SQLException e = expectThrows(SQLException.class, () -> c.prepareStatement("SELECT * from test").executeQuery());
+            SQLException e = expectThrows(SQLException.class, () -> c.prepareStatement("SELECT * FRO").executeQuery());
+            assertEquals("Found 1 problem(s)\nline 1:8: Cannot determine columns for *", e.getMessage());
+        }
+    }
+
+    @Override
+    public void testSelectFromMissingIndex() throws SQLException {
+        try (Connection c = esJdbc()) {
+            SQLException e = expectThrows(SQLException.class, () -> c.prepareStatement("SELECT * FROM test").executeQuery());
             assertEquals("Found 1 problem(s)\nline 1:15: Unknown index [test]", e.getMessage());
         }
     }
+
+    @Override
+    public void testSelectMissingField() throws Exception {
+        index("test", body -> body.field("test", "test"));
+        try (Connection c = esJdbc()) {
+            SQLException e = expectThrows(SQLException.class, () -> c.prepareStatement("SELECT missing FROM test").executeQuery());
+            assertEquals("Found 1 problem(s)\nline 1:8: Unknown column [missing]", e.getMessage());
+        }
+    }
+
+    @Override
+    public void testSelectMissingFunction() throws Exception {
+        index("test", body -> body.field("foo", 1));
+        try (Connection c = esJdbc()) {
+            SQLException e = expectThrows(SQLException.class, () -> c.prepareStatement("SELECT missing(foo) FROM test").executeQuery());
+            assertEquals("Found 1 problem(s)\nline 1:8: Unknown function [missing]", e.getMessage());
+        }
+    }
 }

qa/sql/src/main/java/org/elasticsearch/xpack/qa/sql/rest/RestSqlTestCase.java

@@ -4,6 +4,7 @@
  * you may not use this file except in compliance with the Elastic License.
  */
 package org.elasticsearch.xpack.qa.sql.rest;
+
 import org.apache.http.HttpEntity;
 import org.apache.http.entity.ContentType;
 import org.apache.http.entity.StringEntity;
@@ -15,6 +16,7 @@ import org.elasticsearch.common.xcontent.XContentHelper;
 import org.elasticsearch.common.xcontent.json.JsonXContent;
 import org.elasticsearch.test.NotEqualMessageBuilder;
 import org.elasticsearch.test.rest.ESRestTestCase;
+import org.elasticsearch.xpack.qa.sql.ErrorsTestCase;
 import org.hamcrest.Matcher;
 
 import java.io.IOException;
@@ -25,6 +27,7 @@ import java.util.Arrays;
 import java.util.HashMap;
 import java.util.Locale;
 import java.util.Map;
+import java.util.TreeMap;
 
 import static java.util.Collections.emptyList;
 import static java.util.Collections.singletonList;
@@ -36,7 +39,7 @@ 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 abstract class RestSqlTestCase extends ESRestTestCase {
+public abstract class RestSqlTestCase extends ESRestTestCase implements ErrorsTestCase {
     /**
      * Builds that map that is returned in the header for each column.
      */
@@ -124,11 +127,18 @@ public abstract class RestSqlTestCase extends ESRestTestCase {
                 new StringEntity("{\"query\":\"SELECT DAY_OF_YEAR(test), COUNT(*) FROM test\"}", ContentType.APPLICATION_JSON)));
     }
 
-    public void missingIndex() throws IOException {
+    @Override
-        expectBadRequest(() -> runSql("SELECT foo FROM missing"), containsString("1:17: index [missing] does not exist"));
+    public void testSelectInvalidSql() throws Exception {
+        expectBadRequest(() -> runSql("SELECT * FRO"), containsString("1:8: Cannot determine columns for *"));
     }
 
-    public void testMissingField() throws IOException {
+    @Override
+    public void testSelectFromMissingIndex() throws IOException {
+        expectBadRequest(() -> runSql("SELECT * FROM missing"), containsString("1:15: Unknown index [missing]"));
+    }
+
+    @Override
+    public void testSelectMissingField() throws IOException {
         StringBuilder bulk = new StringBuilder();
         bulk.append("{\"index\":{\"_id\":\"1\"}}\n");
         bulk.append("{\"test\":\"test\"}\n");
@@ -138,6 +148,17 @@ public abstract class RestSqlTestCase extends ESRestTestCase {
         expectBadRequest(() -> runSql("SELECT foo FROM test"), containsString("1:8: Unknown column [foo]"));
     }
 
+    @Override
+    public void testSelectMissingFunction() throws Exception {
+        StringBuilder bulk = new StringBuilder();
+        bulk.append("{\"index\":{\"_id\":\"1\"}}\n");
+        bulk.append("{\"foo\":1}\n");
+        client().performRequest("POST", "/test/test/_bulk", singletonMap("refresh", "true"),
+                new StringEntity(bulk.toString(), ContentType.APPLICATION_JSON));
+
+        expectBadRequest(() -> runSql("SELECT missing(foo) FROM test"), containsString("1:8: Unknown function [missing]"));
+    }
+
     private void expectBadRequest(ThrowingRunnable code, Matcher<String> errorMessageMatcher) {
         ResponseException e = expectThrows(ResponseException.class, code);
         assertEquals(400, e.getResponse().getStatusLine().getStatusCode());
@@ -157,9 +178,10 @@ public abstract class RestSqlTestCase extends ESRestTestCase {
     }
 
     private Map<String, Object> runSql(String suffix, HttpEntity sql) throws IOException {
-        Map<String, String> params = new HashMap<>();
+        Map<String, String> params = new TreeMap<>();
-        params.put("error_trace", "true");
+        params.put("error_trace", "true");   // Helps with debugging in case something crazy happens on the server.
-        params.put("format", "json");
+        params.put("pretty", "true");        // Improves error reporting readability
+        params.put("format", "json");        // JSON is easier to parse then a table
         Response response = client().performRequest("POST", "/_sql" + suffix, params, sql);
         try (InputStream content = response.getEntity().getContent()) {
             return XContentHelper.convertToMap(JsonXContent.jsonXContent, content, false);