More "normalization" to es-norms

Turned on checkstyle but turned off line length for sql because
now is not the time. That caught a few things like redundant
modifiers, out of order modifiers, and multiple top level classes
per file. Fixing them gave me a great little tour. So I'm going to
go and do the same in the plugin code now.

Original commit: elastic/x-pack-elasticsearch@8b52d8845d

dev-tools/checkstyle_suppressions.xml

@@ -9,7 +9,8 @@
   <suppress files="plugin[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]xpack[/\\]sql[/\\]parser[/\\]SqlBase(Base(Listener|Visitor)|Lexer|Listener|Parser|Visitor).java" checks="." />
 
   <!-- NOCOMMIT Temporary-->
-  <suppress files="plugin[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]xpack[/\\]sql[/\\].*.java" checks="." />
+  <suppress files="plugin[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]xpack[/\\]sql[/\\].*.java" checks="LineLength" />
+  <suppress files="sql-clients[/\\].*.java" checks="LineLength" />
 
   <suppress files="plugin[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]xpack[/\\]common[/\\]action[/\\]XPackDeleteByQueryAction.java" checks="LineLength" />
   <suppress files="plugin[/\\]src[/\\]main[/\\]java[/\\]org[/\\]elasticsearch[/\\]xpack[/\\]ml[/\\]action[/\\]StopDatafeedAction.java" checks="LineLength" />

plugin/src/main/java/org/elasticsearch/xpack/sql/analysis/analyzer/Analyzer.java

@@ -968,21 +968,21 @@ public class Analyzer extends RuleExecutor<LogicalPlan> {
             return f.name().equals(seenFunction.name()) && f.arguments().equals(seenFunction.arguments());
         }
     }
-}
 
-abstract class AnalyzeRule<SubPlan extends LogicalPlan> extends Rule<SubPlan, LogicalPlan> {
+    static abstract class AnalyzeRule<SubPlan extends LogicalPlan> extends Rule<SubPlan, LogicalPlan> {
 
-    // transformUp (post-order) - that is first children and then the node
+        // transformUp (post-order) - that is first children and then the node
-    // but with a twist; only if the tree is not resolved or analyzed
+        // but with a twist; only if the tree is not resolved or analyzed
-    @Override
+        @Override
-    public final LogicalPlan apply(LogicalPlan plan) {
+        public final LogicalPlan apply(LogicalPlan plan) {
-        return plan.transformUp(t -> t.analyzed() || (skipResolved() && t.resolved()) ? t : rule(t), typeToken());
+            return plan.transformUp(t -> t.analyzed() || (skipResolved() && t.resolved()) ? t : rule(t), typeToken());
-    }
+        }
 
-    @Override
+        @Override
-    protected abstract LogicalPlan rule(SubPlan plan);
+        protected abstract LogicalPlan rule(SubPlan plan);
 
-    protected boolean skipResolved() {
+        protected boolean skipResolved() {
-        return true;
+            return true;
+        }
     }
 }

sql-clients/build.gradle

@@ -30,9 +30,6 @@ subprojects {
     testCompile "org.elasticsearch.test:framework:${version}"
   }
 
-  tasks.checkstyleMain.enabled = false // not worth it yet
-  tasks.checkstyleTest.enabled = false // not worth it yet
-
   forbiddenApisMain {
     // does not depend on core, so only jdk and http signatures should be checked
     signaturesURLs = [PrecommitTasks.getResource('/forbidden/jdk-signatures.txt')]

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

@@ -9,7 +9,7 @@ import java.sql.ResultSet;
 
 final class DatabaseMetadataProxy extends DebuggingInvoker {
 
-    public DatabaseMetadataProxy(DebugLog log, Object result, Object parent) {
+    DatabaseMetadataProxy(DebugLog log, Object result, Object parent) {
         super(log, result, parent);
     }
 

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

@@ -27,7 +27,6 @@ import java.sql.ResultSet;
 import java.sql.ResultSetMetaData;
 import java.sql.Statement;
 import java.util.HashMap;
-import java.util.Locale;
 import java.util.Map;
 
 public final class Debug {

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

@@ -52,7 +52,7 @@ public class JdbcConnection implements Connection, JdbcWrapper {
         userName = connectionInfo.userName();
     }
 
-    private final void checkOpen() throws SQLException {
+    private void checkOpen() throws SQLException {
         if (isClosed()) {
             throw new SQLException("Connection is closed");
         }

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

@@ -1209,36 +1209,36 @@ class JdbcDatabaseMetaData implements DatabaseMetaData, JdbcWrapper {
     private static ResultSet memorySet(List<ColumnInfo> columns, Object[][] data) {
         return new JdbcResultSet(null, new InMemoryCursor(columns, data));
     }
-}
 
-class InMemoryCursor implements Cursor {
+    static class InMemoryCursor implements Cursor {
 
-    private final List<ColumnInfo> columns;
+        private final List<ColumnInfo> columns;
-    private final Object[][] data;
+        private final Object[][] data;
 
-    private int row = -1;
+        private int row = -1;
 
-    InMemoryCursor(List<ColumnInfo> info, Object[][] data) {
+        InMemoryCursor(List<ColumnInfo> info, Object[][] data) {
-        this.columns = info;
+            this.columns = info;
-        this.data = data;
+            this.data = data;
-    }
-
-    @Override
-    public List<ColumnInfo> columns() {
-        return columns;
-    }
-
-    @Override
-    public boolean next() {
-        if (!ObjectUtils.isEmpty(data) && row < data.length - 1) {
-            row++;
-            return true;
         }
-        return false;
-    }
 
-    @Override
+        @Override
-    public Object column(int column) {
+        public List<ColumnInfo> columns() {
-        return data[row][column];
+            return columns;
+        }
+
+        @Override
+        public boolean next() {
+            if (!ObjectUtils.isEmpty(data) && row < data.length - 1) {
+                row++;
+                return true;
+            }
+            return false;
+        }
+
+        @Override
+        public Object column(int column) {
+            return data[row][column];
+        }
     }
 }

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

@@ -146,7 +146,7 @@ class JdbcResultSetMetaData implements ResultSetMetaData, JdbcWrapper {
         }
     }
 
-    private final ColumnInfo column(int column) throws SQLException {
+    private ColumnInfo column(int column) throws SQLException {
         checkOpen();
         if (column < 1 || column > columns.size()) {
             throw new SQLException("Invalid column index [" + column + "]");

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

@@ -21,9 +21,33 @@ import java.util.TimeZone;
 import java.util.function.Function;
 
 import static java.lang.String.format;
-import static java.sql.Types.*;
+import static java.sql.Types.BIGINT;
+import static java.sql.Types.BINARY;
+import static java.sql.Types.BIT;
+import static java.sql.Types.BOOLEAN;
+import static java.sql.Types.CHAR;
 import static java.sql.Types.DATE;
-import static java.util.Calendar.*;
+import static java.sql.Types.DOUBLE;
+import static java.sql.Types.FLOAT;
+import static java.sql.Types.INTEGER;
+import static java.sql.Types.LONGVARBINARY;
+import static java.sql.Types.LONGVARCHAR;
+import static java.sql.Types.REAL;
+import static java.sql.Types.SMALLINT;
+import static java.sql.Types.TIME;
+import static java.sql.Types.TIMESTAMP;
+import static java.sql.Types.TIMESTAMP_WITH_TIMEZONE;
+import static java.sql.Types.TINYINT;
+import static java.sql.Types.VARBINARY;
+import static java.sql.Types.VARCHAR;
+import static java.util.Calendar.DAY_OF_MONTH;
+import static java.util.Calendar.ERA;
+import static java.util.Calendar.HOUR_OF_DAY;
+import static java.util.Calendar.MILLISECOND;
+import static java.util.Calendar.MINUTE;
+import static java.util.Calendar.MONTH;
+import static java.util.Calendar.SECOND;
+import static java.util.Calendar.YEAR;
 
 abstract class TypeConverter {
 

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

@@ -27,7 +27,7 @@ class DefaultCursor implements Cursor {
         this.page = page;
     }
 
-    private static final String simplifyScrollId(String scrollId) {
+    private static String simplifyScrollId(String scrollId) {
         return StringUtils.hasText(scrollId) ? scrollId : null;
     }
 

sql-clients/jdbc/src/test/java/org/elasticsearch/xpack/sql/jdbc/integration/query/QuerySuite.java

@@ -6,8 +6,10 @@
 package org.elasticsearch.xpack.sql.jdbc.integration.query;
 
 import java.sql.Connection;
+import java.sql.SQLException;
 import java.util.function.Supplier;
 
+import org.elasticsearch.common.CheckedSupplier;
 import org.elasticsearch.xpack.sql.jdbc.integration.query.filter.FilterSpecTests;
 import org.elasticsearch.xpack.sql.jdbc.integration.query.function.aggregate.AggSpecTests;
 import org.elasticsearch.xpack.sql.jdbc.integration.query.function.scalar.datetime.DateTimeSpecTests;
@@ -69,11 +71,11 @@ public class QuerySuite {
         EsDataLoader.loadData();
     }
 
-    public static Supplier<Connection> h2Con() {
+    public static CheckedSupplier<Connection, SQLException> h2Con() {
         return H2;
     }
 
-    public static Supplier<Connection> esCon() {
+    public static CheckedSupplier<Connection, SQLException> esCon() {
         return ES_JDBC_SERVER;
     }
 

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

@@ -6,8 +6,8 @@
 package org.elasticsearch.xpack.sql.jdbc.integration.util;
 
 import org.elasticsearch.client.Client;
+import org.elasticsearch.common.CheckedSupplier;
 import org.elasticsearch.xpack.sql.jdbc.integration.server.JdbcHttpServer;
-import org.elasticsearch.xpack.sql.jdbc.integration.util.JdbcTemplate.JdbcSupplier;
 import org.elasticsearch.xpack.sql.jdbc.jdbc.JdbcDriver;
 import org.junit.rules.ExternalResource;
 
@@ -17,7 +17,7 @@ import java.util.Properties;
 
 import static org.junit.Assert.assertNotNull;
 
-public class EsJdbcServer extends ExternalResource implements JdbcSupplier<Connection> {
+public class EsJdbcServer extends ExternalResource implements CheckedSupplier<Connection, SQLException> {
     private JdbcHttpServer server;
     private String jdbcUrl;
     private JdbcDriver driver;
@@ -53,7 +53,8 @@ public class EsJdbcServer extends ExternalResource implements JdbcSupplier<Conne
         server = null;
     }
 
-    public Connection jdbc() throws SQLException {
+    @Override
+    public Connection get() throws SQLException {
         assertNotNull("ES JDBC Driver is null - make sure ES is properly run as a @ClassRule", driver);
         return driver.connect(jdbcUrl, properties);
     }

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

@@ -5,7 +5,7 @@
  */
 package org.elasticsearch.xpack.sql.jdbc.integration.util;
 
-import org.elasticsearch.xpack.sql.jdbc.integration.util.JdbcTemplate.JdbcSupplier;
+import org.elasticsearch.common.CheckedSupplier;
 import org.junit.rules.ExternalResource;
 
 import java.sql.Connection;
@@ -13,7 +13,7 @@ import java.sql.DriverManager;
 import java.sql.SQLException;
 import java.util.Properties;
 
-public class H2 extends ExternalResource implements JdbcSupplier<Connection> {
+public class H2 extends ExternalResource implements CheckedSupplier<Connection, SQLException> {
     static {
         try {
             Class.forName("org.h2.Driver");
@@ -40,7 +40,7 @@ public class H2 extends ExternalResource implements JdbcSupplier<Connection> {
 
     @Override
     protected void before() throws Throwable {
-        keepAlive = jdbc();
+        keepAlive = get();
     }
 
     @Override
@@ -53,7 +53,8 @@ public class H2 extends ExternalResource implements JdbcSupplier<Connection> {
         keepAlive = null;
     }
 
-    public Connection jdbc() throws SQLException {
+    @Override
+    public Connection get() throws SQLException {
         return DriverManager.getConnection(url, DEFAULT_PROPS);
     }
 }

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

@@ -7,6 +7,7 @@ package org.elasticsearch.xpack.sql.jdbc.integration.util;
 
 import org.elasticsearch.common.CheckedConsumer;
 import org.elasticsearch.common.CheckedFunction;
+import org.elasticsearch.common.CheckedSupplier;
 import org.elasticsearch.xpack.sql.net.client.SuppressForbidden;
 
 import java.sql.Connection;
@@ -19,33 +20,17 @@ import java.util.ArrayList;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
-import java.util.function.Supplier;
 
 // poor's man JdbcTemplate
 public class JdbcTemplate {
+    private static final int MAX_WIDTH = 20;
 
-    private final Supplier<Connection> conn;
+    private final CheckedSupplier<Connection, SQLException> conn;
 
-    public JdbcTemplate(JdbcSupplier<Connection> conn) {
+    public JdbcTemplate(CheckedSupplier<Connection, SQLException> conn) {
         this.conn = conn;
     }
 
-    public static interface JdbcSupplier<T> extends Supplier<T> {
-
-        @Override
-        default T get() {
-            try {
-                return jdbc();
-            } catch (SQLException ex) {
-                throw new RuntimeException(ex);
-            }
-        }
-
-        T jdbc() throws SQLException;
-    }
-
-    private static final int MAX_WIDTH = 20;
-
     @SuppressForbidden(reason="temporary")
     public static CheckedFunction<ResultSet, Void, SQLException> resultSetToConsole() {
         // NOCOMMIT this doesn't really test anything. If we want to log the whole result set we can do that too, but we have to add assertions

sql-clients/test-utils/src/main/java/org/elasticsearch/xpack/sql/test/server/ProtoHandler.java

@@ -23,7 +23,7 @@ import com.sun.net.httpserver.HttpHandler;
 
 public abstract class ProtoHandler<R> implements HttpHandler, AutoCloseable {
 
-    protected final static Logger log = ESLoggerFactory.getLogger(ProtoHandler.class.getName());
+    protected static final Logger log = ESLoggerFactory.getLogger(ProtoHandler.class.getName());
     private final TimeValue TV = TimeValue.timeValueSeconds(5);
     protected final NodeInfo info;
     protected final String clusterName;