SQL: Fix error message on bad index (elastic/x-pack-elasticsearch#3312)

Fixes the error message that SQL produces when it sees unsupported
indexes. It was always returning all broken indexes as "unknown" even
though we have much better error messages. It was just throwing them
away.

I caught this originally when backporting to 6.x where we had a test
that we produced a useful error message when the user attempted to run
SQL on an index with more than one type. We couldn't run that in the
feature/sql branch because it is inside the full cluster restart tests
and the "old" version of Elasticsearch used in those tests in
feature/sql is 6.x which doesn't allow indexes with multiple types. When
I backported to 6.x the test failed because it hadn't been run before.

In addition to fixing that test and the problem, this adds another test
that will reveal the problem when run in the feature/sql and master
branch.

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

qa/full-cluster-restart/src/test/java/org/elasticsearch/xpack/restart/FullClusterRestartIT.java

@@ -290,7 +290,8 @@ public class FullClusterRestartIT extends ESRestTestCase {
         ResponseException e = expectThrows(ResponseException.class, () -> client().performRequest("POST", "/_xpack/sql", emptyMap(),
                 new StringEntity("{\"query\":\"SELECT * FROM testsqlfailsonindexwithtwotypes\"}", ContentType.APPLICATION_JSON)));
         assertEquals(400, e.getResponse().getStatusLine().getStatusCode());
-        assertThat(e.getMessage(), containsString("Invalid index testsqlfailsonindexwithtwotypes; contains more than one type"));
+        assertThat(e.getMessage(), containsString(
+            "[testsqlfailsonindexwithtwotypes] contains more than one type [type1, type2] so it is incompatible with sql"));
     }
 
     private String loadWatch(String watch) throws IOException {

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

@@ -13,6 +13,7 @@ package org.elasticsearch.xpack.qa.sql;
 public interface ErrorsTestCase {
     void testSelectInvalidSql() throws Exception;
     void testSelectFromMissingIndex() throws Exception;
+    void testSelectFromIndexWithoutTypes() throws Exception;
     void testSelectMissingField() throws Exception;
     void testSelectMissingFunction() throws Exception;
 }

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

@@ -6,6 +6,10 @@
 package org.elasticsearch.xpack.qa.sql.cli;
 
 import java.io.IOException;
+import org.apache.http.entity.ContentType;
+import org.apache.http.entity.StringEntity;
+
+import static java.util.Collections.emptyMap;
 
 /**
  * Tests for error messages.
@@ -23,6 +27,15 @@ public abstract class ErrorsTestCase extends CliIntegrationTestCase implements o
         assertEquals("line 1:15: Unknown index [test][1;23;31m][0m", readLine());
     }
 
+    @Override
+    public void testSelectFromIndexWithoutTypes() throws Exception {
+        // Create an index without any types
+        client().performRequest("PUT", "/test", emptyMap(), new StringEntity("{}", ContentType.APPLICATION_JSON));
+
+        assertEquals("[1;31mBad request [[22;3;33mFound 1 problem(s)", command("SELECT * FROM test"));
+        assertEquals("line 1:15: [test] doesn't have any types so it is incompatible with sql[1;23;31m][0m", readLine());
+    }
+
     @Override
     public void testSelectMissingField() throws IOException {
         index("test", body -> body.field("test", "test"));

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

@@ -7,6 +7,10 @@ package org.elasticsearch.xpack.qa.sql.jdbc;
 
 import java.sql.Connection;
 import java.sql.SQLException;
+import org.apache.http.entity.ContentType;
+import org.apache.http.entity.StringEntity;
+
+import static java.util.Collections.emptyMap;
 
 /**
  * Tests for exceptions and their messages.
@@ -28,6 +32,17 @@ public class ErrorsTestCase extends JdbcIntegrationTestCase implements org.elast
         }
     }
 
+    @Override
+    public void testSelectFromIndexWithoutTypes() throws Exception {
+        // Create an index without any types
+        client().performRequest("PUT", "/test", emptyMap(), new StringEntity("{}", ContentType.APPLICATION_JSON));
+
+        try (Connection c = esJdbc()) {
+            SQLException e = expectThrows(SQLException.class, () -> c.prepareStatement("SELECT * FROM test").executeQuery());
+            assertEquals("Found 1 problem(s)\nline 1:15: [test] doesn't have any types so it is incompatible with sql", e.getMessage());
+        }
+    }
+
     @Override
     public void testSelectMissingField() throws Exception {
         index("test", body -> body.field("test", "test"));

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

@@ -30,6 +30,7 @@ import java.util.Map;
 import java.util.TreeMap;
 
 import static java.util.Collections.emptyList;
+import static java.util.Collections.emptyMap;
 import static java.util.Collections.singletonList;
 import static java.util.Collections.singletonMap;
 import static java.util.Collections.unmodifiableMap;
@@ -137,6 +138,15 @@ public abstract class RestSqlTestCase extends ESRestTestCase implements ErrorsTe
         expectBadRequest(() -> runSql("SELECT * FROM missing"), containsString("1:15: Unknown index [missing]"));
     }
 
+    @Override
+    public void testSelectFromIndexWithoutTypes() throws Exception {
+        // Create an index without any types
+        client().performRequest("PUT", "/test", emptyMap(), new StringEntity("{}", ContentType.APPLICATION_JSON));
+
+        expectBadRequest(() -> runSql("SELECT * FROM test"),
+            containsString("1:15: [test] doesn't have any types so it is incompatible with sql"));
+    }
+
     @Override
     public void testSelectMissingField() throws IOException {
         StringBuilder bulk = new StringBuilder();

sql/server/src/main/java/org/elasticsearch/xpack/sql/analysis/index/GetIndexResult.java

@@ -20,7 +20,7 @@ public final class GetIndexResult {
     }
     public static GetIndexResult notFound(String name) {
         Objects.requireNonNull(name, "name must not be null");
-        return invalid("Index '" + name + "' does not exist");
+        return invalid("Unknown index [" + name + "]");
     }
 
     private final EsIndex index;

sql/server/src/main/java/org/elasticsearch/xpack/sql/plan/logical/UnresolvedRelation.java

@@ -61,7 +61,7 @@ public class UnresolvedRelation extends LeafPlan implements Unresolvable {
 
     @Override
     public int hashCode() {
-        return Objects.hash(table);
+        return Objects.hash(location(), table, alias, unresolvedMsg);
     }
 
     @Override
@@ -75,6 +75,9 @@ public class UnresolvedRelation extends LeafPlan implements Unresolvable {
         }
 
         UnresolvedRelation other = (UnresolvedRelation) obj;
-        return Objects.equals(table, other.table);
+        return location().equals(other.location())
+            && table.equals(other.table)
+            && Objects.equals(alias, other.alias)
+            && unresolvedMsg.equals(other.unresolvedMsg);
     }
 }

sql/server/src/test/java/org/elasticsearch/xpack/sql/plan/logical/UnresolvedRelationTests.java

@@ -0,0 +1,40 @@
+/*
+ * 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.sql.plan.logical;
+
+import org.elasticsearch.test.ESTestCase;
+import org.elasticsearch.xpack.sql.plan.TableIdentifier;
+import org.elasticsearch.xpack.sql.tree.Location;
+import java.util.ArrayList;
+import java.util.List;
+import java.util.function.Function;
+
+import static org.elasticsearch.test.EqualsHashCodeTestUtils.checkEqualsAndHashCode;
+
+public class UnresolvedRelationTests extends ESTestCase {
+    public void testEqualsAndHashCode() {
+        Location location = new Location(between(1, 1000), between(1, 1000));
+        TableIdentifier table = new TableIdentifier(location, randomAlphaOfLength(5));
+        String alias = randomBoolean() ? null : randomAlphaOfLength(5);
+        String unresolvedMessage = randomAlphaOfLength(5);
+        UnresolvedRelation relation = new UnresolvedRelation(location, table, alias, unresolvedMessage);
+        List<Function<UnresolvedRelation, UnresolvedRelation>> mutators = new ArrayList<>();
+        mutators.add(r -> new UnresolvedRelation(
+            new Location(r.location().getLineNumber() + 1, r.location().getColumnNumber()), r.table(), r.alias(), r.unresolvedMessage()));
+        mutators.add(r -> new UnresolvedRelation(
+            r.location(), new TableIdentifier(r.location(), r.table().index() + "m"), r.alias(), r.unresolvedMessage()));
+        mutators.add(r -> new UnresolvedRelation(
+            r.location(),
+            r.table(),
+            randomValueOtherThan(r.alias(), () -> randomBoolean() ? null : randomAlphaOfLength(5)),
+            r.unresolvedMessage()));
+        mutators.add(r -> new UnresolvedRelation(
+            r.location(), r.table(), r.alias(), randomValueOtherThan(r.unresolvedMessage(), () -> randomAlphaOfLength(5))));
+        checkEqualsAndHashCode(relation,
+            r -> new UnresolvedRelation(r.location(), r.table(), r.alias(), r.unresolvedMessage()),
+            r -> randomFrom(mutators).apply(r));
+    }
+}