Start moving tests to integration tests

These catch *lots* more issues.

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

plugin/src/main/java/org/elasticsearch/xpack/sql/analysis/catalog/Catalog.java

@@ -9,6 +9,7 @@ import java.util.Collection;
 
 
 public interface Catalog {
+    // NOCOMMIT make sure we need all of these methods....
 
     EsIndex getIndex(String index);
 

plugin/src/main/java/org/elasticsearch/xpack/sql/analysis/catalog/EsCatalog.java

@@ -5,11 +5,7 @@
  */
 package org.elasticsearch.xpack.sql.analysis.catalog;
 
-import java.util.ArrayList;
+import com.carrotsearch.hppc.cursors.ObjectObjectCursor;
-import java.util.Collection;
-import java.util.Iterator;
-import java.util.List;
-import java.util.function.Supplier;
 
 import org.elasticsearch.action.support.IndicesOptions;
 import org.elasticsearch.cluster.ClusterState;
@@ -18,8 +14,13 @@ import org.elasticsearch.cluster.metadata.IndexNameExpressionResolver;
 import org.elasticsearch.cluster.metadata.MappingMetaData;
 import org.elasticsearch.cluster.metadata.MetaData;
 import org.elasticsearch.common.Strings;
+import org.elasticsearch.common.regex.Regex;
 
-import static java.util.Collections.singletonList;
+import java.util.ArrayList;
+import java.util.Collection;
+import java.util.Iterator;
+import java.util.List;
+import java.util.function.Supplier;
 
 public class EsCatalog implements Catalog {
 
@@ -103,25 +104,28 @@ public class EsCatalog implements Catalog {
     }
 
     @Override
-    public Collection<EsType> listTypes(String indexPattern, String pattern) {
+    public Collection<EsType> listTypes(String indexPattern, String typePattern) {
         if (!Strings.hasText(indexPattern)) {
             indexPattern = WILDCARD;
         }
 
-        String[] iName = indexNameExpressionResolver.concreteIndexNames(clusterState.get(), IndicesOptions.strictExpandOpenAndForbidClosed(), indexPattern);
+        String[] indices = indexNameExpressionResolver.concreteIndexNames(clusterState.get(),
+                IndicesOptions.strictExpandOpenAndForbidClosed(), indexPattern);
 
-        String cIndex = iName[0];
+        List<EsType> results = new ArrayList<>();
-        IndexMetaData imd = metadata().index(cIndex);
+        for (String index : indices) {
-        
+            IndexMetaData imd = metadata().index(index);
-        if (Strings.hasText(pattern)) {
+            for (ObjectObjectCursor<String, MappingMetaData> entry : imd.getMappings()) {
-            return singletonList(EsType.build(cIndex, pattern, imd.mapping(pattern)));
+                if (false == Strings.hasLength(typePattern) || Regex.simpleMatch(typePattern, entry.key)) {
-        }
+                    results.add(EsType.build(index, entry.key, entry.value));
-        else {
+                }
-            return EsType.build(cIndex, imd.getMappings());
+            }
         }
+        return results;
     }
     
     private String[] resolveIndex(String pattern) {
-        return indexNameExpressionResolver.concreteIndexNames(clusterState.get(), IndicesOptions.strictExpandOpenAndForbidClosed(), pattern);
+        return indexNameExpressionResolver.concreteIndexNames(clusterState.get(), IndicesOptions.strictExpandOpenAndForbidClosed(),
+                pattern);
     }
 }

plugin/src/main/java/org/elasticsearch/xpack/sql/analysis/catalog/EsType.java

@@ -57,16 +57,6 @@ public class EsType {
         return new EsType(index, type, mapping);
     }
 
-    static Collection<EsType> build(String index, ImmutableOpenMap<String, MappingMetaData> mapping) {
-        List<EsType> tps = new ArrayList<>();
-
-        for (ObjectObjectCursor<String, MappingMetaData> entry : mapping) {
-            tps.add(build(index, entry.key, entry.value));
-        }
-
-        return tps;
-    }
-
     @Override
     public String toString() {
         StringBuilder sb = new StringBuilder();

plugin/src/main/java/org/elasticsearch/xpack/sql/plugin/jdbc/http/HttpJdbcAction.java

@@ -5,9 +5,6 @@
  */
 package org.elasticsearch.xpack.sql.plugin.jdbc.http;
 
-import java.io.DataInputStream;
-import java.io.IOException;
-
 import org.elasticsearch.ExceptionsHelper;
 import org.elasticsearch.client.node.NodeClient;
 import org.elasticsearch.common.settings.Settings;
@@ -23,6 +20,9 @@ import org.elasticsearch.xpack.sql.plugin.jdbc.action.JdbcRequest;
 import org.elasticsearch.xpack.sql.plugin.jdbc.action.JdbcResponse;
 import org.elasticsearch.xpack.sql.plugin.jdbc.server.JdbcServerProtoUtils;
 
+import java.io.DataInputStream;
+import java.io.IOException;
+
 import static org.elasticsearch.action.ActionListener.wrap;
 import static org.elasticsearch.rest.BytesRestResponse.TEXT_CONTENT_TYPE;
 import static org.elasticsearch.rest.RestRequest.Method.POST;
@@ -30,7 +30,7 @@ import static org.elasticsearch.rest.RestStatus.BAD_REQUEST;
 import static org.elasticsearch.rest.RestStatus.INTERNAL_SERVER_ERROR;
 import static org.elasticsearch.rest.RestStatus.OK;
 
-public class HttpJdbcAction extends BaseRestHandler {
+public class HttpJdbcAction extends BaseRestHandler { // NOCOMMIT these are call RestJdbcAction even if it isn't REST.
 
     public HttpJdbcAction(Settings settings, RestController controller) {
         super(settings);
@@ -63,12 +63,13 @@ public class HttpJdbcAction extends BaseRestHandler {
         return c -> c.sendResponse(new BytesRestResponse(BAD_REQUEST, TEXT_CONTENT_TYPE, message));
     }
 
-    private static void jdbcResponse(RestChannel channel, JdbcResponse response) {
+    private void jdbcResponse(RestChannel channel, JdbcResponse response) {
         BytesRestResponse restResponse = null;
         
         try {
             restResponse = new BytesRestResponse(OK, TEXT_CONTENT_TYPE, JdbcServerProtoUtils.write(response.response()));
         } catch (IOException ex) {
+            logger.error("error building jdbc response", ex);
             restResponse = new BytesRestResponse(INTERNAL_SERVER_ERROR, TEXT_CONTENT_TYPE, StringUtils.EMPTY);
         }
 

plugin/src/main/java/org/elasticsearch/xpack/sql/plugin/jdbc/server/JdbcServerProtoUtils.java

@@ -22,11 +22,11 @@ public abstract class JdbcServerProtoUtils {
 
     public static BytesReference write(Response response) throws IOException {
         try (BytesStreamOutput array = new BytesStreamOutput();
-             DataOutputStream out = new DataOutputStream(array)) {
+            DataOutputStream out = new DataOutputStream(array)) {
             ProtoUtils.write(out, response);
 
             // serialize payload (if present)
-            if (response instanceof DataResponse) {
+            if (response instanceof DataResponse) { // NOCOMMIT why not implement an interface?
                 RowSetCursor cursor = (RowSetCursor) ((QueryInitResponse) response).data;
 
                 if (cursor != null) {

sql-clients/cli/src/main/java/org/elasticsearch/xpack/sql/cli/CliConfiguration.java

@@ -20,7 +20,7 @@ import org.elasticsearch.xpack.sql.net.client.ConnectionConfiguration;
 
 public class CliConfiguration extends ConnectionConfiguration {
 
-    private IpAndPort ipAndPort;
+    private HostAndPort hostAndPort;
     private String originalUrl;
     private String urlFile = "/";
 
@@ -35,7 +35,6 @@ public class CliConfiguration extends ConnectionConfiguration {
             u = u.substring(0, u.length() - 1);
         }
 
-
         // remove space
         u = u.trim();
 
@@ -58,36 +57,32 @@ public class CliConfiguration extends ConnectionConfiguration {
             }
         }
 
-        // default host
-        String host = "localhost";
-        // is there a host ?
-
         // look for port
         index = hostAndPort.indexOf(":");
         if (index > 0) {
             if (index + 1 >= hostAndPort.length()) {
                 throw new IllegalArgumentException("Invalid port specified");
             }
-            host = hostAndPort.substring(0, index);
+            String host = hostAndPort.substring(0, index);
             String port = hostAndPort.substring(index + 1);
 
-            ipAndPort = new IpAndPort(host, Integer.parseInt(port));
+            this.hostAndPort = new HostAndPort(host, Integer.parseInt(port));
         }
         else {
-            ipAndPort = new IpAndPort(u);
+            this.hostAndPort = new HostAndPort(u);
         }
     }
 
     public URL asUrl() {
         // TODO: need to assemble all the various params here
         try {
-            return new URL(isSSL() ? "https" : "http", ipAndPort.ip, port(), urlFile);
+            return new URL(isSSL() ? "https" : "http", hostAndPort.ip, port(), urlFile);
         } catch (MalformedURLException ex) {
             throw new IllegalArgumentException("Cannot connect to server " + originalUrl, ex);
         }
     }
 
     private int port() {
-        return ipAndPort.port > 0 ? ipAndPort.port : 9200;
+        return hostAndPort.port > 0 ? hostAndPort.port : 9200;
     }
 }

sql-clients/jdbc-proto/src/main/java/org/elasticsearch/xpack/sql/jdbc/net/protocol/ProtoUtils.java

@@ -94,7 +94,9 @@ public abstract class ProtoUtils {
     }
 
     public static String readHeader(DataInput in) throws IOException {
-        if (MAGIC_NUMBER != in.readInt()) {
+        // NOCOMMIT why not just throw?
+        int magic = in.readInt();
+        if (MAGIC_NUMBER != magic) {
             return "Invalid protocol";
         }
         int ver = in.readInt();

sql-clients/jdbc/build.gradle

@@ -1,4 +1,5 @@
 import org.elasticsearch.gradle.Version
+import org.elasticsearch.gradle.test.RunTask
 
 apply plugin: 'elasticsearch.build'
 
@@ -64,3 +65,27 @@ jar {
   from(zipTree(project(':x-pack-elasticsearch:sql-clients:net-client').jar.archivePath))
   from(zipTree(project(':x-pack-elasticsearch:sql-clients:jdbc-proto').jar.archivePath))
 }
+
+apply plugin: 'elasticsearch.rest-test'
+
+integTestCluster {
+  distribution = 'zip' // NOCOMMIT make double sure we want all the modules
+  plugin project(':x-pack-elasticsearch:plugin').path
+  /* Get a "clean" test without the other x-pack features here and check them
+   * all together later on. */
+  setting 'xpack.security.enabled', 'false'
+  setting 'xpack.monitoring.enabled', 'false'
+  setting 'xpack.ml.enabled', 'false'
+  setting 'xpack.watcher.enabled', 'false'
+}
+
+task run(type: RunTask) {
+  distribution = 'zip' // NOCOMMIT make double sure we want all the modules
+  plugin project(':x-pack-elasticsearch:plugin').path
+  /* Get a "clean" test without the other x-pack features here and check them
+   * all together later on. */
+  setting 'xpack.security.enabled', 'false'
+  setting 'xpack.monitoring.enabled', 'false'
+  setting 'xpack.ml.enabled', 'false'
+  setting 'xpack.watcher.enabled', 'false'
+}

sql-clients/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/jdbc/JdbcConfiguration.java

@@ -30,11 +30,11 @@ import org.elasticsearch.xpack.sql.net.client.util.StringUtils;
 
 //TODO: beef this up for Security/SSL
 public class JdbcConfiguration extends ConnectionConfiguration {
-
     static final String URL_PREFIX = "jdbc:es:";
 
     static final String USER = "user";
-    static final String USER_DEFAULT = "";
+
+    static final String PASSWORD = "password";
 
     static final String DEBUG = "debug";
     static final String DEBUG_DEFAULT = "false";
@@ -43,9 +43,9 @@ public class JdbcConfiguration extends ConnectionConfiguration {
     // can be out/err/url
     static final String DEBUG_OUTPUT_DEFAULT = "err";
 
-    private static List<String> KNOWN_OPTIONS = Arrays.asList(DEBUG, DEBUG_OUTPUT);
+    private static final List<String> KNOWN_OPTIONS = Arrays.asList(DEBUG, DEBUG_OUTPUT);
 
-    private IpAndPort ipAndPort;
+    private HostAndPort hostAndPort;
     private String originalUrl;
     private String urlFile = "/";
 
@@ -69,115 +69,119 @@ public class JdbcConfiguration extends ConnectionConfiguration {
             throw new JdbcException("Expected %s url, received %s", URL_PREFIX, u);
         }
 
-        if (u.endsWith("/")) {
+        try {
-            u = u.substring(0, u.length() - 1);
+            if (u.endsWith("/")) {
-        }
+                u = u.substring(0, u.length() - 1);
+            }
 
-        // remove space
+            // remove space
-        u = u.trim();
+            u = u.trim();
 
-        //
+            //
-        // remove prefix jdbc:es prefix
+            // remove prefix jdbc:es prefix
-        //
+            //
+            u = u.substring(URL_PREFIX.length(), u.length());
 
-        u = u.substring(URL_PREFIX.length(), u.length());
+            if (!u.startsWith("//")) {
-
-        if (!u.startsWith("//")) {
-            throw new JdbcException("Invalid URL %s, format should be %s", url, format);
-        }
-
-        // remove //
-        u = u.substring(2);
-
-
-        String hostAndPort = u;
-
-        // / is required if any params are specified
-        // get it out of the way early on
-        int index = u.indexOf("/");
-
-        String params = null;
-        int pIndex = u.indexOf("?");
-        if (pIndex > 0) {
-            if (index < 0) {
                 throw new JdbcException("Invalid URL %s, format should be %s", url, format);
             }
-            if (pIndex + 1 < u.length()) {
-                params = u.substring(pIndex + 1);
-            }
-        }
 
-        // parse url suffix (if any)
+            // remove //
-        if (index >= 0) {
+            u = u.substring(2);
-            hostAndPort = u.substring(0, index);
+
-            if (index + 1 < u.length()) {
+            String hostAndPort = u;
-                urlFile = u.substring(index);
+
-                index = urlFile.indexOf("?");
+            // / is required if any params are specified
-                if (index > 0) {
+            // get it out of the way early on
-                    urlFile = urlFile.substring(0, index);
+            int index = u.indexOf("/");
+    
+            String params = null;
+            int pIndex = u.indexOf("?");
+            if (pIndex > 0) {
+                if (index < 0) {
+                    throw new JdbcException("Invalid URL %s, format should be %s", url, format);
+                }
+                if (pIndex + 1 < u.length()) {
+                    params = u.substring(pIndex + 1);
                 }
             }
-        }
 
-        //
+            // parse url suffix (if any)
-        // parse host
+            if (index >= 0) {
-        //
+                hostAndPort = u.substring(0, index);
-
+                if (index + 1 < u.length()) {
-        // default host
+                    urlFile = u.substring(index);
-        String host = "localhost";
+                    index = urlFile.indexOf("?");
-        // is there a host ?
+                    if (index > 0) {
-
+                        urlFile = urlFile.substring(0, index);
-        // look for port
+                    }
-        index = hostAndPort.indexOf(":");
-        if (index > 0) {
-            if (index + 1 >= hostAndPort.length()) {
-                throw new JdbcException("Invalid port specified");
-            }
-            host = hostAndPort.substring(0, index);
-
-            String port = hostAndPort.substring(index + 1);
-
-            ipAndPort = new IpAndPort(host, Integer.parseInt(port));
-        }
-        else {
-            ipAndPort = new IpAndPort(hostAndPort);
-        }
-
-        //
-        // parse params
-        //
-        
-        if (params != null) {
-            // parse properties
-            List<String> prms = StringUtils.tokenize(params, "&");
-            for (String param : prms) {
-                List<String> args = StringUtils.tokenize(param, "=");
-                Assert.isTrue(args.size() == 2, "Invalid parameter %s, format needs to be key=value", param);
-                String pName = args.get(0);
-                if (!KNOWN_OPTIONS.contains(pName)) {
-                    throw new JdbcException("Unknown parameter [%s] ; did you mean %s", pName, StringUtils.findSimiliar(pName, KNOWN_OPTIONS));    
                 }
-                
-                settings().setProperty(args.get(0), args.get(1));
             }
+
+            //
+            // parse host
+            //
+
+            // look for port
+            index = hostAndPort.lastIndexOf(":");
+            if (index > 0) {
+                if (index + 1 >= hostAndPort.length()) {
+                    throw new JdbcException("Invalid port specified");
+                }
+                String host = hostAndPort.substring(0, index);
+                String port = hostAndPort.substring(index + 1);
+
+                this.hostAndPort = new HostAndPort(host, Integer.parseInt(port));
+            } else {
+                this.hostAndPort = new HostAndPort(hostAndPort);
+            }
+
+            //
+            // parse params
+            //
+            if (params != null) {
+                // parse properties
+                List<String> prms = StringUtils.tokenize(params, "&");
+                for (String param : prms) {
+                    List<String> args = StringUtils.tokenize(param, "=");
+                    Assert.isTrue(args.size() == 2, "Invalid parameter %s, format needs to be key=value", param);
+                    String pName = args.get(0);
+                    if (!KNOWN_OPTIONS.contains(pName)) {
+                        throw new JdbcException("Unknown parameter [%s] ; did you mean %s", pName,
+                                StringUtils.findSimiliar(pName, KNOWN_OPTIONS));
+                    }
+                    
+                    settings().setProperty(args.get(0), args.get(1));
+                }
+            }
+        } catch (JdbcException e) {
+            throw e;
+        } catch (Exception e) {
+            // Add the url to unexpected exceptions
+            throw new IllegalArgumentException("Failed to parse acceptable jdbc url [" + u + "]", e);
         }
     }
 
     public URL asUrl() {
         // TODO: need to assemble all the various params here
         try {
-            return new URL(isSSL() ? "https" : "http", ipAndPort.ip, port(), urlFile);
+            return new URL(isSSL() ? "https" : "http", hostAndPort.ip, port(), urlFile);
         } catch (MalformedURLException ex) {
             throw new JdbcException(ex, "Cannot connect to server %s", originalUrl);
         }
     }
 
     public String userName() {
-        return settings().getProperty(USER, USER_DEFAULT);
+        return settings().getProperty(USER);
+    }
+
+    public String password() {
+        // NOCOMMIT make sure we're doing right by the password. Compare with other jdbc drivers and our security code.
+        return settings().getProperty(PASSWORD);
     }
 
     private int port() {
-        return ipAndPort.port > 0 ? ipAndPort.port : 9200;
+        return hostAndPort.port > 0 ? hostAndPort.port : 9200;
     }
 
     public boolean debug() {

sql-clients/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/jdbc/JdbcDatabaseMetaData.java

@@ -707,7 +707,7 @@ class JdbcDatabaseMetaData implements DatabaseMetaData, JdbcWrapper {
         }
 
         String cat = defaultCatalog();
-        List<String> tables = con.client.metaInfoTables(replaceJdbcWildcardForTables(tableNamePattern));
+        List<String> tables = con.client.metaInfoTables(sqlWildcardToSimplePattern(tableNamePattern));
         Object[][] data = new Object[tables.size()][];
         for (int i = 0; i < data.length; i++) {
             data[i] = new Object[10];
@@ -727,14 +727,21 @@ class JdbcDatabaseMetaData implements DatabaseMetaData, JdbcWrapper {
         return memorySet(info, data);
     }
 
-    // this one goes through the ES API
+    /**
-    private static String replaceJdbcWildcardForTables(String tableName) {
+     * Convert sql wildcards ({@code %} and @{code _}) into {@code Regex#simpleMatch}-style patterns. 
-        return hasText(tableName) ? tableName.replaceAll("%", "*").replace('_', '?') : tableName;
+     */
+    private static String sqlWildcardToSimplePattern(String pattern) {
+        // NOCOMMIT ? isn't supported by simple pattern
+        // NOCOMMIT escape *?
+        return hasText(pattern) ? pattern.replaceAll("%", "*").replace('_', '?') : pattern;
     }
 
-    // this one gets computed to Pattern matching 
+    /**
-    private static String replaceJdbcWildcardForColumns(String tableName) {
+     * Convert sql wildcards ({@code %} and @{code _}) into regex style patterns.
-        return hasText(tableName) ? tableName.replaceAll("%", ".*").replace('_', '.') : tableName;
+     */
+    private static String sqlWildcardToRegexPattern(String pattern) {
+        // NOCOMMIT escape regex bits?
+        return hasText(pattern) ? pattern.replaceAll("%", ".*").replace('_', '.') : pattern;
     }
 
     @Override
@@ -745,7 +752,6 @@ class JdbcDatabaseMetaData implements DatabaseMetaData, JdbcWrapper {
                                     "TABLE_CATALOG"), data);
     }
 
-
     @Override
     public ResultSet getSchemas(String catalog, String schemaPattern) throws SQLException {
         List<ColumnInfo> info = columnInfo("SCHEMATA",
@@ -807,7 +813,7 @@ class JdbcDatabaseMetaData implements DatabaseMetaData, JdbcWrapper {
         }
 
         String cat = defaultCatalog();
-        List<MetaColumnInfo> columns = con.client.metaInfoColumns(replaceJdbcWildcardForTables(tableNamePattern), replaceJdbcWildcardForColumns(columnNamePattern));
+        List<MetaColumnInfo> columns = con.client.metaInfoColumns(sqlWildcardToSimplePattern(tableNamePattern), sqlWildcardToRegexPattern(columnNamePattern));
         Object[][] data = new Object[columns.size()][];
         for (int i = 0; i < data.length; i++) {
             data[i] = new Object[24];

sql-clients/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/jdbc/JdbcDriver.java

@@ -25,6 +25,7 @@ public class JdbcDriver implements java.sql.Driver, Closeable {
             final JdbcDriver d = new JdbcDriver();
             DriverManager.registerDriver(d, d::close);
         } catch (Exception ex) {
+            // NOCOMMIT this seems bad!
             // ignore
         }
     }

sql-clients/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/net/client/HttpClient.java

@@ -5,10 +5,6 @@
  */
 package org.elasticsearch.xpack.sql.jdbc.net.client;
 
-import java.net.MalformedURLException;
-import java.net.URL;
-import java.sql.SQLException;
-
 import org.elasticsearch.xpack.sql.jdbc.jdbc.JdbcConfiguration;
 import org.elasticsearch.xpack.sql.jdbc.jdbc.JdbcException;
 import org.elasticsearch.xpack.sql.jdbc.util.BytesArray;
@@ -16,6 +12,12 @@ import org.elasticsearch.xpack.sql.net.client.ClientException;
 import org.elasticsearch.xpack.sql.net.client.DataOutputConsumer;
 import org.elasticsearch.xpack.sql.net.client.jre.JreHttpUrlConnection;
 
+import java.net.MalformedURLException;
+import java.net.URL;
+import java.security.AccessController;
+import java.security.PrivilegedAction;
+import java.sql.SQLException;
+
 // http client
 // handles nodes discovery, fail-over, errors, etc...
 class HttpClient {
@@ -44,22 +46,26 @@ class HttpClient {
         }
     }
 
-    boolean head(String path) {
+    boolean head(String path) { // NOCOMMIT remove path?
         try {
-            return JreHttpUrlConnection.http(url(path), cfg, JreHttpUrlConnection::head);
+            return AccessController.doPrivileged((PrivilegedAction<Boolean>) () -> {
+                return JreHttpUrlConnection.http(url(path), cfg, JreHttpUrlConnection::head);
+            });
         } catch (ClientException ex) {
             throw new JdbcException(ex, "Transport failure");
         }
     }
 
     BytesArray put(DataOutputConsumer os) throws SQLException {
-        return put("sql/", os);
+        return put("_jdbc/", os);
     }
 
-    BytesArray put(String path, DataOutputConsumer os) throws SQLException {
+    BytesArray put(String path, DataOutputConsumer os) throws SQLException { // NOCOMMIT remove path?
         try {
-            return JreHttpUrlConnection.http(url(path), cfg, con -> {
+            return AccessController.doPrivileged((PrivilegedAction<BytesArray>) () -> {    
-                return new BytesArray(con.put(os));
+                return JreHttpUrlConnection.http(url(path), cfg, con -> {
+                    return new BytesArray(con.put(os));
+                });
             });
         } catch (ClientException ex) {
             throw new JdbcException(ex, "Transport failure");

sql-clients/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/net/client/HttpJdbcClient.java

@@ -5,15 +5,6 @@
  */
 package org.elasticsearch.xpack.sql.jdbc.net.client;
 
-import java.io.Closeable;
-import java.io.DataInput;
-import java.io.DataInputStream;
-import java.io.DataOutput;
-import java.io.IOException;
-import java.sql.SQLException;
-import java.time.Instant;
-import java.util.List;
-
 import org.elasticsearch.xpack.sql.jdbc.jdbc.JdbcConfiguration;
 import org.elasticsearch.xpack.sql.jdbc.jdbc.JdbcException;
 import org.elasticsearch.xpack.sql.jdbc.net.protocol.ErrorResponse;
@@ -25,6 +16,8 @@ import org.elasticsearch.xpack.sql.jdbc.net.protocol.MetaColumnRequest;
 import org.elasticsearch.xpack.sql.jdbc.net.protocol.MetaColumnResponse;
 import org.elasticsearch.xpack.sql.jdbc.net.protocol.MetaTableRequest;
 import org.elasticsearch.xpack.sql.jdbc.net.protocol.MetaTableResponse;
+import org.elasticsearch.xpack.sql.jdbc.net.protocol.Proto.Action;
+import org.elasticsearch.xpack.sql.jdbc.net.protocol.Proto.SqlExceptionType;
 import org.elasticsearch.xpack.sql.jdbc.net.protocol.ProtoUtils;
 import org.elasticsearch.xpack.sql.jdbc.net.protocol.QueryInitRequest;
 import org.elasticsearch.xpack.sql.jdbc.net.protocol.QueryInitResponse;
@@ -32,11 +25,18 @@ import org.elasticsearch.xpack.sql.jdbc.net.protocol.QueryPageRequest;
 import org.elasticsearch.xpack.sql.jdbc.net.protocol.QueryPageResponse;
 import org.elasticsearch.xpack.sql.jdbc.net.protocol.Response;
 import org.elasticsearch.xpack.sql.jdbc.net.protocol.TimeoutInfo;
-import org.elasticsearch.xpack.sql.jdbc.net.protocol.Proto.Action;
-import org.elasticsearch.xpack.sql.jdbc.net.protocol.Proto.SqlExceptionType;
 import org.elasticsearch.xpack.sql.jdbc.util.BytesArray;
 import org.elasticsearch.xpack.sql.jdbc.util.FastByteArrayInputStream;
 
+import java.io.Closeable;
+import java.io.DataInput;
+import java.io.DataInputStream;
+import java.io.DataOutput;
+import java.io.IOException;
+import java.sql.SQLException;
+import java.time.Instant;
+import java.util.List;
+
 public class HttpJdbcClient implements Closeable {
     @FunctionalInterface
     interface DataInputFunction<R> {
@@ -54,6 +54,7 @@ public class HttpJdbcClient implements Closeable {
 
     public boolean ping(long timeoutInMs) {
         long oldTimeout = http.getNetworkTimeout();
+        // NOCOMMIT this seems race condition-y
         http.setNetworkTimeout(timeoutInMs);
         try {
             return http.head("");

sql-clients/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/util/BytesArray.java

@@ -5,12 +5,12 @@
  */
 package org.elasticsearch.xpack.sql.jdbc.util;
 
-import java.io.IOException;
-import java.io.OutputStream;
-
 import org.elasticsearch.xpack.sql.net.client.util.Bytes;
 import org.elasticsearch.xpack.sql.net.client.util.StringUtils;
 
+import java.io.IOException;
+import java.io.OutputStream;
+
 public class BytesArray {
 
     public static final byte[] EMPTY = new byte[0];
@@ -99,6 +99,7 @@ public class BytesArray {
 
     @Override
     public String toString() {
+        // NOCOMMIT I think we're much more likely to want this as hex....
         return StringUtils.asUTFString(bytes, offset, size);
     }
 

sql-clients/jdbc/src/test/java/org/elasticsearch/xpack/sql/jdbc/BasicsIT.java

@@ -0,0 +1,126 @@
+/*
+ * 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.jdbc;
+
+import java.sql.Connection;
+import java.sql.SQLException;
+import java.util.ArrayList;
+import java.util.Comparator;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import static java.util.Collections.emptyList;
+import static java.util.Collections.singletonList;
+import static java.util.Collections.singletonMap;
+
+/**
+ * Test the jdbc driver behavior and the connection to Elasticsearch.
+ */
+public class BasicsIT extends JdbcIntegrationTestCase {
+
+    // NOCOMMIT these might should move into their own test or be deleted entirely
+//    public void test01Ping() throws Exception {
+//        assertThat(client.ping((int) TimeUnit.SECONDS.toMillis(5)), equalTo(true));
+//    }
+//
+//    public void testInfoAction() throws Exception {
+//        InfoResponse esInfo = client.serverInfo();
+//        assertThat(esInfo, notNullValue());
+//        assertThat(esInfo.cluster, is("elasticsearch"));
+//        assertThat(esInfo.node, not(isEmptyOrNullString()));
+//        assertThat(esInfo.versionHash, not(isEmptyOrNullString()));
+//        assertThat(esInfo.versionString, startsWith("5."));
+//        assertThat(esInfo.majorVersion, is(5));
+//        //assertThat(esInfo.minorVersion(), is(0));
+//    }
+//
+//    public void testInfoTable() throws Exception {
+//        List<String> tables = client.metaInfoTables("emp*");
+//        assertThat(tables.size(), greaterThanOrEqualTo(1));
+//        assertThat(tables, hasItem("emp.emp"));
+//    }
+//
+//    public void testInfoColumn() throws Exception {
+//        List<MetaColumnInfo> info = client.metaInfoColumns("em*", null);
+//        for (MetaColumnInfo i : info) {
+//            // NOCOMMIT test these
+//            logger.info(i);
+//        }
+//    }
+    public void testConnectionProperties() throws SQLException {
+        j.consume(c -> {
+            assertFalse(c.isClosed());
+            assertTrue(c.isReadOnly());
+        });
+    }
+
+    /**
+     * Tests that we throw report no transaction isolation and throw sensible errors if you ask for any.
+     */
+    public void testTransactionIsolation() throws Exception {
+        j.consume(c -> {
+            assertEquals(Connection.TRANSACTION_NONE, c.getTransactionIsolation());
+            SQLException e = expectThrows(SQLException.class, () -> c.setTransactionIsolation(Connection.TRANSACTION_SERIALIZABLE));
+            assertEquals("Transactions not supported", e.getMessage());
+            assertEquals(Connection.TRANSACTION_NONE, c.getTransactionIsolation());
+        });
+    }
+
+    public void testShowTablesEmpty() throws Exception {
+        List<Map<String, Object>> results = j.queryForList("SHOW TABLES");
+        assertEquals(emptyList(), results);
+    }
+
+    public void testShowTablesWithAnIndex() throws Exception {
+        index("test", builder -> builder.field("name", "bob"));
+        List<Map<String, Object>> results = j.queryForList("SHOW TABLES");
+        List<Map<String, Object>> expected = new ArrayList<>();
+        Map<String, Object> index = new HashMap<>();
+        index.put("index", "test");
+        index.put("type", "doc");
+        expected.add(index);
+        assertEquals(expected, results);
+    }
+
+    public void testShowTablesWithManyIndices() throws Exception {
+        int indices = between(2, 20);
+        for (int i = 0; i < indices; i++) {
+            index("test" + i, builder -> builder.field("name", "bob"));
+        }
+        List<Map<String, Object>> results = j.queryForList("SHOW TABLES");
+        results.sort(Comparator.comparing(map -> map.get("index").toString()));
+        List<Map<String, Object>> expected = new ArrayList<>();
+        for (int i = 0; i < indices; i++) {
+            Map<String, Object> index = new HashMap<>();
+            index.put("index", "test" + i);
+            index.put("type", "doc");
+            expected.add(index);
+        }
+        expected.sort(Comparator.comparing(map -> map.get("index").toString()));
+        assertEquals(expected, results);
+
+    }
+
+    public void testBasicSelect() throws Exception {
+        index("test", builder -> builder.field("name", "bob"));
+
+        List<Map<String, Object>> results = j.queryForList("SELECT * from test.doc");
+        assertEquals(singletonList(singletonMap("name", "bob")), results);
+    }
+
+    public void testSelectFromMissingTable() throws Exception {
+        SQLException e = expectThrows(SQLException.class, () -> j.queryForList("SELECT * from test.doc"));
+        assertEquals("line 1:15: Cannot resolve index test", e.getMessage());
+    }
+
+    public void testSelectFromMissingType() throws Exception {
+        index("test", builder -> builder.field("name", "bob"));
+
+        SQLException e = expectThrows(SQLException.class, () -> j.queryForList("SELECT * from test.notdoc"));
+        assertEquals("line 1:15: Cannot resolve type notdoc in index test", e.getMessage());
+    }
+}

sql-clients/jdbc/src/test/java/org/elasticsearch/xpack/sql/jdbc/ConnectionInfoTests.java

@@ -3,7 +3,7 @@
  * 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.sql.jdbc;
+package org.elasticsearch.xpack.sql.jdbc;
 
 import org.elasticsearch.test.ESTestCase;
 import org.elasticsearch.xpack.sql.jdbc.jdbc.JdbcConfiguration;

sql-clients/jdbc/src/test/java/org/elasticsearch/xpack/sql/jdbc/DatabaseMetaDataIT.java

@@ -0,0 +1,183 @@
+/*
+ * 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.jdbc;
+
+import java.sql.DatabaseMetaData;
+import java.sql.ResultSet;
+import java.sql.ResultSetMetaData;
+import java.sql.SQLException;
+import java.sql.Types;
+
+import static org.hamcrest.Matchers.is;
+import static org.hamcrest.Matchers.notNullValue;
+import static org.hamcrest.Matchers.startsWith;
+
+/**
+ * Tests for our implementation of {@link DatabaseMetaData}.
+ */
+public class DatabaseMetaDataIT extends JdbcIntegrationTestCase {
+    /**
+     * We do not support procedures so we return an empty set for {@link DatabaseMetaData#getProcedures(String, String, String)}.
+     */
+    public void testMetadataGetProcedures() throws Exception {
+        j.consume(c -> {
+            DatabaseMetaData metaData = c.getMetaData();
+            ResultSet results = metaData.getProcedures(
+                    randomBoolean() ? null : randomAlphaOfLength(5),
+                    randomBoolean() ? null : randomAlphaOfLength(5),
+                    randomBoolean() ? null : randomAlphaOfLength(5));
+            ResultSetMetaData meta = results.getMetaData();
+            int i = 1;
+            assertColumn("PROCEDURE_CAT", "VARCHAR", meta, i++);
+            assertColumn("PROCEDURE_SCHEM", "VARCHAR", meta, i++);
+            assertColumn("PROCEDURE_NAME", "VARCHAR", meta, i++);
+            assertColumn("NUM_INPUT_PARAMS", "INTEGER", meta, i++);
+            assertColumn("NUM_OUTPUT_PARAMS", "INTEGER", meta, i++);
+            assertColumn("NUM_RESULT_SETS", "INTEGER", meta, i++);
+            assertColumn("REMARKS", "VARCHAR", meta, i++);
+            assertColumn("PROCEDURE_TYPE", "SMALLINT", meta, i++);
+            assertColumn("SPECIFIC_NAME", "VARCHAR", meta, i++);
+            assertEquals(i - 1, meta.getColumnCount());
+
+            assertFalse(results.next());
+        });
+    }
+
+    public void testMetadataGetProcedureColumns() throws Exception {
+        j.consume(c -> {
+            DatabaseMetaData metaData = c.getMetaData();
+            ResultSet results = metaData.getProcedureColumns(
+                    randomBoolean() ? null : randomAlphaOfLength(5),
+                    randomBoolean() ? null : randomAlphaOfLength(5),
+                    randomBoolean() ? null : randomAlphaOfLength(5),
+                    randomBoolean() ? null : randomAlphaOfLength(5));
+            ResultSetMetaData meta = results.getMetaData();
+            int i = 1;
+            assertColumn("PROCEDURE_CAT", "VARCHAR", meta, i++);
+            assertColumn("PROCEDURE_SCHEM", "VARCHAR", meta, i++);
+            assertColumn("PROCEDURE_NAME", "VARCHAR", meta, i++);
+            assertColumn("COLUMN_NAME", "VARCHAR", meta, i++);
+            assertColumn("COLUMN_TYPE", "SMALLINT", meta, i++);
+            assertColumn("DATA_TYPE", "INTEGER", meta, i++);
+            assertColumn("TYPE_NAME", "VARCHAR", meta, i++);
+            assertColumn("PRECISION", "INTEGER", meta, i++);
+            assertColumn("LENGTH", "INTEGER", meta, i++);
+            assertColumn("SCALE", "SMALLINT", meta, i++);
+            assertColumn("RADIX", "SMALLINT", meta, i++);
+            assertColumn("NULLABLE", "SMALLINT", meta, i++);
+            assertColumn("REMARKS", "VARCHAR", meta, i++);
+            assertColumn("COLUMN_DEF", "VARCHAR", meta, i++);
+            assertColumn("SQL_DATA_TYPE", "INTEGER", meta, i++);
+            assertColumn("SQL_DATETIME_SUB", "INTEGER", meta, i++);
+            assertColumn("CHAR_OCTET_LENGTH", "INTEGER", meta, i++);
+            assertColumn("ORDINAL_POSITION", "INTEGER", meta, i++);
+            assertColumn("IS_NULLABLE", "VARCHAR", meta, i++);
+            assertColumn("SPECIFIC_NAME", "VARCHAR", meta, i++);
+            assertEquals(i - 1, meta.getColumnCount());
+
+            assertFalse(results.next());
+        });
+    }
+
+    public void testMetadataGetTables() throws Exception {
+        index("test", body -> body.field("name", "bob"));
+        j.consume(c -> {
+            DatabaseMetaData metaData = c.getMetaData();
+            ResultSet results = metaData.getTables("%", "%", "%", null);
+            ResultSetMetaData meta = results.getMetaData();
+            int i = 1;
+            assertColumn("TABLE_CAT", "VARCHAR", meta, i++);
+            assertColumn("TABLE_SCHEM", "VARCHAR", meta, i++);
+            assertColumn("TABLE_NAME", "VARCHAR", meta, i++);
+            assertColumn("TABLE_TYPE", "VARCHAR", meta, i++);
+            assertColumn("REMARKS", "VARCHAR", meta, i++);
+            assertColumn("TYPE_CAT", "VARCHAR", meta, i++);
+            assertColumn("TYPE_SCHEM", "VARCHAR", meta, i++);
+            assertColumn("TYPE_NAME", "VARCHAR", meta, i++);
+            assertColumn("SELF_REFERENCING_COL_NAME", "VARCHAR", meta, i++);
+            assertColumn("REF_GENERATION", "VARCHAR", meta, i++);
+            assertEquals(i - 1, meta.getColumnCount());
+
+            assertTrue(results.next());
+            i = 1;
+            assertThat(results.getString(i++), startsWith("x-pack-elasticsearch_sql-clients_jdbc_"));
+            assertEquals("", results.getString(i++));
+            assertEquals("test.doc", results.getString(i++));
+            assertEquals("TABLE", results.getString(i++));
+            assertEquals("", results.getString(i++));
+            assertEquals(null, results.getString(i++));
+            assertEquals(null, results.getString(i++));
+            assertEquals(null, results.getString(i++));
+            assertEquals(null, results.getString(i++));
+            assertEquals(null, results.getString(i++));
+            assertFalse(results.next());
+
+            results = metaData.getTables("%", "%", "te%", null);
+            assertTrue(results.next());
+            assertEquals("test.doc", results.getString(3));
+            assertFalse(results.next());
+
+            results = metaData.getTables("%", "%", "test.d%", null);
+            assertTrue(results.next());
+            assertEquals("test.doc", results.getString(3));
+            assertFalse(results.next());
+        });
+    }
+
+    public void testMetadataColumns() throws Exception {
+        index("test", body -> body.field("name", "bob"));
+        j.consume(c -> {
+            DatabaseMetaData metaData = c.getMetaData();
+            ResultSet results = metaData.getColumns("%", "%", "%", null);
+            ResultSetMetaData meta = results.getMetaData();
+            int i = 1;
+            assertColumn("TABLE_CAT", "VARCHAR", meta, i++);
+            assertColumn("TABLE_SCHEM", "VARCHAR", meta, i++);
+            assertColumn("TABLE_NAME", "VARCHAR", meta, i++);
+            assertColumn("COLUMN_NAME", "VARCHAR", meta, i++);
+            assertColumn("DATA_TYPE", "INTEGER", meta, i++);
+            assertColumn("TYPE_NAME", "VARCHAR", meta, i++);
+            assertColumn("COLUMN_SIZE", "INTEGER", meta, i++);
+            assertColumn("BUFFER_LENGTH", "NULL", meta, i++);
+            assertColumn("DECIMAL_DIGITS", "INTEGER", meta, i++);
+            assertColumn("NUM_PREC_RADIX", "INTEGER", meta, i++);
+            assertColumn("NULLABLE", "INTEGER", meta, i++);
+            assertColumn("REMARKS", "VARCHAR", meta, i++);
+            assertColumn("COLUMN_DEF", "VARCHAR", meta, i++);
+            assertColumn("SQL_DATA_TYPE", "INTEGER", meta, i++);
+            assertColumn("SQL_DATETIME_SUB", "INTEGER", meta, i++);
+            assertColumn("CHAR_OCTET_LENGTH", "INTEGER", meta, i++);
+            assertColumn("ORDINAL_POSITION", "INTEGER", meta, i++);
+            assertColumn("IS_NULLABLE", "VARCHAR", meta, i++);
+            assertColumn("SCOPE_CATALOG", "VARCHAR", meta, i++);
+            assertColumn("SCOPE_SCHEMA", "VARCHAR", meta, i++);
+            assertColumn("SCOPE_TABLE", "VARCHAR", meta, i++);
+            assertColumn("SOURCE_DATA_TYPE", "SMALLINT", meta, i++);
+            assertColumn("IS_AUTOINCREMENT", "VARCHAR", meta, i++);
+            assertColumn("IS_GENERATEDCOLUMN", "VARCHAR", meta, i++);
+            assertEquals(i - 1, meta.getColumnCount());
+
+            assertTrue(results.next());
+            i = 1;
+            assertThat(results.getString(i++), startsWith("x-pack-elasticsearch_sql-clients_jdbc_"));
+            assertEquals("", results.getString(i++));
+            assertEquals("test.doc", results.getString(i++));
+            assertEquals("name", results.getString(i++));
+            assertEquals(Types.VARCHAR, results.getInt(i++));
+            assertEquals(null, results.getString(i++));
+            assertEquals(null, results.getString(i++));
+            assertEquals(null, results.getString(i++));
+            assertEquals(null, results.getString(i++));
+            assertEquals(null, results.getString(i++));
+            assertFalse(results.next());
+        });
+    }
+
+    private static void assertColumn(String name, String type, ResultSetMetaData meta, int index) throws SQLException {
+        assertEquals(name, meta.getColumnName(index));
+        assertEquals(type, meta.getColumnTypeName(index));
+    }
+}

sql-clients/jdbc/src/test/java/org/elasticsearch/xpack/sql/jdbc/JdbcIntegrationTestCase.java

@@ -0,0 +1,44 @@
+/*
+ * 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.jdbc;
+
+import org.apache.http.HttpEntity;
+import org.apache.http.entity.ContentType;
+import org.apache.http.entity.StringEntity;
+import org.elasticsearch.common.CheckedConsumer;
+import org.elasticsearch.common.xcontent.XContentBuilder;
+import org.elasticsearch.common.xcontent.json.JsonXContent;
+import org.elasticsearch.test.rest.ESRestTestCase;
+import org.elasticsearch.xpack.sql.jdbc.integration.util.JdbcTemplate;
+import org.elasticsearch.xpack.sql.jdbc.jdbc.JdbcDriver;
+import org.junit.Before;
+
+import java.io.IOException;
+import java.sql.DriverManager;
+
+import static java.util.Collections.singletonMap;
+
+public class JdbcIntegrationTestCase extends ESRestTestCase {
+    static {
+        // Initialize the jdbc driver
+        JdbcDriver.jdbcMajorVersion();
+    }
+
+    protected JdbcTemplate j;
+
+    @Before
+    public void setupJdbcTemplate() throws Exception {
+        j = new JdbcTemplate(() -> DriverManager.getConnection("jdbc:es://" + System.getProperty("tests.rest.cluster")));
+    }
+
+    protected void index(String index, CheckedConsumer<XContentBuilder, IOException> body) throws IOException {
+        XContentBuilder builder = JsonXContent.contentBuilder().startObject();
+        body.accept(builder);
+        builder.endObject();
+        HttpEntity doc = new StringEntity(builder.string(), ContentType.APPLICATION_JSON);
+        client().performRequest("PUT", "/" + index + "/doc/1", singletonMap("refresh", "true"), doc);
+    }
+}

sql-clients/jdbc/src/test/java/org/elasticsearch/xpack/sql/jdbc/integration/net/protocol/ProtoTests.java

@@ -1,201 +0,0 @@
-/*
- * 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.jdbc.integration.net.protocol;
-
-import org.elasticsearch.client.Client;
-import org.elasticsearch.common.settings.Settings;
-import org.elasticsearch.common.transport.TransportAddress;
-import org.elasticsearch.test.ESTestCase;
-import org.elasticsearch.transport.client.PreBuiltTransportClient;
-import org.elasticsearch.xpack.sql.jdbc.integration.server.JdbcHttpServer;
-import org.elasticsearch.xpack.sql.jdbc.integration.util.JdbcTemplate;
-import org.elasticsearch.xpack.sql.jdbc.jdbc.JdbcConfiguration;
-import org.elasticsearch.xpack.sql.jdbc.jdbc.JdbcDriver;
-import org.elasticsearch.xpack.sql.jdbc.net.client.HttpJdbcClient;
-import org.elasticsearch.xpack.sql.jdbc.net.protocol.InfoResponse;
-import org.elasticsearch.xpack.sql.jdbc.net.protocol.MetaColumnInfo;
-import org.junit.AfterClass;
-import org.junit.BeforeClass;
-
-import java.net.InetAddress;
-import java.sql.Connection;
-import java.sql.DatabaseMetaData;
-import java.sql.ResultSet;
-import java.sql.SQLException;
-import java.util.List;
-import java.util.Properties;
-import java.util.concurrent.TimeUnit;
-
-import static org.hamcrest.Matchers.equalTo;
-import static org.hamcrest.Matchers.greaterThanOrEqualTo;
-import static org.hamcrest.Matchers.hasItem;
-import static org.hamcrest.Matchers.is;
-import static org.hamcrest.Matchers.isEmptyOrNullString;
-import static org.hamcrest.Matchers.not;
-import static org.hamcrest.Matchers.notNullValue;
-import static org.hamcrest.Matchers.startsWith;
-
-
-public class ProtoTests extends ESTestCase {
-    // NOCOMMIT probably should be an integration test that runs against a running copy of ES with SQL installed
-
-    private static Client esClient;
-    private static JdbcHttpServer server;
-    private static HttpJdbcClient client;
-    private static JdbcDriver driver;
-    private static String jdbcUrl;
-    private static JdbcTemplate j;
-
-    @BeforeClass
-    public static void setUpServer() throws Exception {
-        if (esClient == null) {
-            esClient = new PreBuiltTransportClient(Settings.EMPTY)
-                    .addTransportAddress(new TransportAddress(InetAddress.getLoopbackAddress(), 9300));
-        }
-        if (server == null) {
-            server = new JdbcHttpServer(esClient);
-            server.start(0);
-        }
-
-        if (client == null) {
-            jdbcUrl = server.url();
-            JdbcConfiguration ci = new JdbcConfiguration(jdbcUrl, new Properties());
-            client = new HttpJdbcClient(ci);
-        }
-
-        if (driver == null) {
-            driver = new JdbcDriver();
-        }
-        
-        j = new JdbcTemplate(ProtoTests::con);
-    }
-
-    @AfterClass
-    public static void tearDownServer() {
-        if (server != null) {
-            server.stop();
-            server = null;
-        }
-
-        if (client != null) {
-            client.close();
-            client = null;
-        }
-
-        if (driver != null) {
-            driver.close();
-            driver = null;
-        }
-
-        if (esClient != null) {
-            esClient.close();
-            esClient = null;
-        }
-    }
-
-    private static Connection con() throws SQLException {
-        return driver.connect(jdbcUrl, new Properties());
-    }
-
-    public void test01Ping() throws Exception {
-        assertThat(client.ping((int) TimeUnit.SECONDS.toMillis(5)), equalTo(true));
-    }
-
-    public void testInfoAction() throws Exception {
-        InfoResponse esInfo = client.serverInfo();
-        assertThat(esInfo, notNullValue());
-        assertThat(esInfo.cluster, is("elasticsearch"));
-        assertThat(esInfo.node, not(isEmptyOrNullString()));
-        assertThat(esInfo.versionHash, not(isEmptyOrNullString()));
-        assertThat(esInfo.versionString, startsWith("5."));
-        assertThat(esInfo.majorVersion, is(5));
-        //assertThat(esInfo.minorVersion(), is(0));
-    }
-
-    public void testInfoTable() throws Exception {
-        List<String> tables = client.metaInfoTables("emp*");
-        assertThat(tables.size(), greaterThanOrEqualTo(1));
-        assertThat(tables, hasItem("emp.emp"));
-    }
-
-    public void testInfoColumn() throws Exception {
-        List<MetaColumnInfo> info = client.metaInfoColumns("em*", null);
-        for (MetaColumnInfo i : info) {
-            // NOCOMMIT test these
-            logger.info(i);
-        }
-    }
-
-    public void testBasicJdbc() throws Exception {
-        j.consume(c -> {
-            assertThat(c.isClosed(), is(false));
-            assertThat(c.isReadOnly(), is(true));
-        });
-
-        j.queryToConsole("SHOW TABLES");
-    }
-
-    public void testBasicSelect() throws Exception {
-        j.consume(c -> {
-            assertThat(c.isClosed(), is(false));
-            assertThat(c.isReadOnly(), is(true));
-        });
-
-        j.queryToConsole("SELECT * from \"emp.emp\" ");
-    }
-
-    public void testBasicDemo() throws Exception {
-        j.consume(c -> {
-            assertThat(c.isClosed(), is(false));
-            assertThat(c.isReadOnly(), is(true));
-        });
-
-        RuntimeException e = expectThrows(RuntimeException.class, () ->
-            j.queryToConsole("SELECT name, postalcode, last_score, last_score_date FROM doesnot.exist"));
-        assertEquals("asdfasd", e.getMessage());
-    }
-
-    public void testMetadataGetProcedures() throws Exception {
-        j.consume(c -> {
-            DatabaseMetaData metaData = c.getMetaData();
-            ResultSet results = metaData.getProcedures(null, null, null);
-            assertThat(results, is(notNullValue()));
-            assertThat(results.next(), is(false));
-            assertThat(results.getMetaData().getColumnCount(), is(9));
-        });
-    }
-
-    public void testMetadataGetProcedureColumns() throws Exception {
-        j.consume(c -> {
-            DatabaseMetaData metaData = c.getMetaData();
-            ResultSet results = metaData.getProcedureColumns(null, null, null, null);
-            assertThat(results, is(notNullValue()));
-            assertThat(results.next(), is(false));
-            assertThat(results.getMetaData().getColumnCount(), is(20));
-        });
-    }
-
-    public void testMetadataGetTables() throws Exception {
-        j.consume(c -> {
-            DatabaseMetaData metaData = c.getMetaData();
-            ResultSet results = metaData.getTables("elasticsearch", "", "%", null);
-            assertThat(results, is(notNullValue()));
-            assertThat(results.next(), is(true));
-            assertThat(results.getMetaData().getColumnCount(), is(10));
-        });
-    }
-
-    public void testMetadataColumns() throws Exception {
-        RuntimeException e = expectThrows(RuntimeException.class, () -> j.consume(c -> {
-            DatabaseMetaData metaData = c.getMetaData();
-            ResultSet results = metaData.getColumns("elasticsearch", "", "dep.dep", "%");
-            assertThat(results, is(notNullValue()));
-            assertThat(results.next(), is(true));
-            assertThat(results.getMetaData().getColumnCount(), is(24));
-        }));
-        assertEquals("adsf", e.getMessage());
-    }
-}

sql-clients/jdbc/src/test/java/org/elasticsearch/xpack/sql/jdbc/integration/util/JdbcTemplate.java

@@ -88,7 +88,7 @@ public class JdbcTemplate {
         return buffer;
     }
 
-    public void consume(CheckedConsumer<Connection, SQLException> c) throws Exception {
+    public void consume(CheckedConsumer<Connection, SQLException> c) throws SQLException {
         try (Connection con = conn.get()) {
             c.accept(con);
         }
@@ -229,7 +229,7 @@ public class JdbcTemplate {
             int count = metaData.getColumnCount();
             Map<String, Object> map = new LinkedHashMap<>(count);
 
-            for (int j = 0; j < count; j++) {
+            for (int j = 1; j <= count; j++) {
                 map.put(metaData.getColumnName(j), rs.getObject(j));
             }
             return map;

sql-clients/jdbc/src/test/resources/plugin-security.policy

@@ -0,0 +1,4 @@
+grant {
+  // Policy is required for tests to connect to testing Elasticsearch instance.
+  permission java.net.SocketPermission "*", "connect,resolve";
+};

sql-clients/net-client/src/main/java/org/elasticsearch/xpack/sql/net/client/ClientException.java

@@ -10,16 +10,15 @@ import java.util.Locale;
 import static java.lang.String.format;
 
 public class ClientException extends RuntimeException {
-
-    public ClientException() {
-        super();
-    }
-
     public ClientException(String message, Throwable cause, boolean enableSuppression, boolean writableStackTrace) {
         super(message, cause, enableSuppression, writableStackTrace);
     }
 
-    public ClientException(String message, Object... args) {
+    public ClientException(String message) {
+        super(message);
+    }
+
+    public ClientException(String message, Object... args) { // NOCOMMIT these are not popular in core any more....
         super(format(Locale.ROOT, message, args));
     }
 

sql-clients/net-client/src/main/java/org/elasticsearch/xpack/sql/net/client/ConnectionConfiguration.java

@@ -5,20 +5,22 @@
  */
 package org.elasticsearch.xpack.sql.net.client;
 
+import org.elasticsearch.xpack.sql.net.client.util.StringUtils;
+
 import java.util.Properties;
 import java.util.concurrent.TimeUnit;
 
 public class ConnectionConfiguration {
 
-    public static class IpAndPort {
+    public static class HostAndPort {
         public final String ip;
         public final int port;
 
-        public IpAndPort(String ip) {
+        public HostAndPort(String ip) {
             this(ip, 0);
         }
 
-        public IpAndPort(String ip, int port) {
+        public HostAndPort(String ip, int port) {
             this.ip = ip;
             this.port = port;
         }
@@ -75,6 +77,7 @@ public class ConnectionConfiguration {
     static final String SSL_TRUSTSTORE_TYPE = "ssl.keystore.location";
     static final String SSL_TRUSTSTORE_TYPE_DEFAULT = "ssl.keystore.location";
     
+
     private final Properties settings;
 
     private long connectTimeout;
@@ -83,6 +86,7 @@ public class ConnectionConfiguration {
 
     private long pageTimeout;
     private int pageSize;
+    private final boolean ssl;
 
     public ConnectionConfiguration(Properties props) {
         settings = props != null ? new Properties(props) : new Properties();
@@ -93,6 +97,7 @@ public class ConnectionConfiguration {
         // page
         pageTimeout = Long.parseLong(settings.getProperty(PAGE_TIMEOUT, PAGE_TIMEOUT_DEFAULT));
         pageSize = Integer.parseInt(settings.getProperty(PAGE_SIZE, PAGE_SIZE_DEFAULT));
+        ssl = StringUtils.parseBoolean(settings.getProperty(SSL, SSL_DEFAULT));
     }
 
     protected Properties settings() {
@@ -100,8 +105,7 @@ public class ConnectionConfiguration {
     }
 
     protected boolean isSSL() {
-        //TODO: check params
+        return ssl;
-        return false;
     }
 
     public void setConnectTimeout(long millis) {

sql-clients/net-client/src/main/java/org/elasticsearch/xpack/sql/net/client/jre/JreHttpUrlConnection.java

@@ -12,6 +12,7 @@ import java.io.InputStream;
 import java.io.OutputStream;
 import java.net.HttpURLConnection;
 import java.net.URL;
+import java.nio.charset.StandardCharsets;
 import java.util.function.Function;
 
 import org.elasticsearch.xpack.sql.net.client.ClientException;
@@ -42,7 +43,8 @@ public class JreHttpUrlConnection implements Closeable {
         // HttpURL adds this header by default, HttpS does not
         // adding it here to be consistent
         con.setRequestProperty("Accept-Charset", "UTF-8");
-        con.setRequestProperty("Accept-Encoding", "gzip");
+        // NOCOMMIT if we're going to accept gzip then we need to transparently unzip it on the way out...
+        // con.setRequestProperty("Accept-Encoding", "gzip");
     }
 
     public boolean head() throws ClientException {
@@ -55,16 +57,26 @@ public class JreHttpUrlConnection implements Closeable {
         }
     }
 
-    public Bytes put(DataOutputConsumer doc) throws ClientException {
+    public Bytes put(DataOutputConsumer doc) throws ClientException { // NOCOMMIT why is this called put when it is a post?
         try {
             con.setRequestMethod("POST");
             con.setDoOutput(true);
+            con.setRequestProperty("Content-Type", "application/json");
             try (OutputStream out = con.getOutputStream()) {
                 doc.accept(new DataOutputStream(out));
             }
             if (con.getResponseCode() >= 400) {
-                throw new ClientException("Protocol/client error; server returned %s", con.getResponseMessage());
+                InputStream err = con.getErrorStream();
+                String response;
+                if (err == null) {
+                    response = "server did not return a response";
+                } else {
+                    // NOCOMMIT figure out why this returns weird characters. Can reproduce with unauthorized.
+                    response = new String(IOUtils.asBytes(err).bytes(), StandardCharsets.UTF_8);
+                }
+                throw new ClientException("Protocol/client error; server returned [" + con.getResponseMessage() + "]: " + response);
             }
+            // NOCOMMIT seems weird that we buffer this into a byte stream and then wrap it in a byte array input stream.....
             return IOUtils.asBytes(con.getInputStream());
         } catch (IOException ex) {
             throw new ClientException(ex, "Cannot POST address %s", url);
@@ -121,7 +133,7 @@ public class JreHttpUrlConnection implements Closeable {
     // main call class
     //
 
-    public static <R, E extends Exception> R http(URL url, ConnectionConfiguration cfg, Function<JreHttpUrlConnection, R> handler) throws E {
+    public static <R> R http(URL url, ConnectionConfiguration cfg, Function<JreHttpUrlConnection, R> handler) {
         try (JreHttpUrlConnection con = new JreHttpUrlConnection(url, cfg)) {
             return handler.apply(con);
         }

sql-clients/net-client/src/main/java/org/elasticsearch/xpack/sql/net/client/util/StringUtils.java

@@ -286,4 +286,12 @@ public abstract class StringUtils {
 
         return list;
     }
+
+    public static boolean parseBoolean(String input) {
+        switch(input) {
+        case "true": return true;
+        case "false": return false;
+        default: throw new IllegalArgumentException("must be [true] or [false]");
+        }
+    }
 }