SQL: NOCOMMIT cleanup

Removes a few NOCOMMITs that are tracked other places and updates
a few with plans on how to work on them.

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

plugin/src/main/java/org/elasticsearch/xpack/sql/plugin/SecurityCatalogFilter.java

@@ -31,7 +31,6 @@ import java.util.function.BiFunction;
  * statements like {@code SHOW TABLES} and {@code DESCRIBE TABLE}. 
  */
 public class SecurityCatalogFilter implements FilteredCatalog.Filter {
-    // NOCOMMIT need to figure out sql on aliases that expand to many indices
     private static final IndicesOptions OPTIONS = IndicesOptions.strictSingleIndexNoExpandForbidClosed();
 
     private final ThreadContext threadContext;

sql/cli/src/main/java/org/elasticsearch/xpack/sql/cli/CliHttpClient.java

@@ -36,13 +36,11 @@ public class CliHttpClient implements AutoCloseable {
 
     public QueryInitResponse queryInit(String query, int fetchSize) {
         // TODO allow customizing the time zone
-        // NOCOMMIT figure out Timeouts....
         QueryInitRequest request = new QueryInitRequest(query, fetchSize, TimeZone.getTimeZone("UTC"), new TimeoutInfo(0, 0, 0));
         return (QueryInitResponse) sendRequest(request);
     }
 
     public QueryPageResponse nextPage(byte[] cursor) {
-        // NOCOMMIT figure out Timeouts....
         QueryPageRequest request = new QueryPageRequest(cursor, new TimeoutInfo(0, 0, 0));
         return (QueryPageResponse) sendRequest(request);
     }

sql/cli/src/main/java/org/elasticsearch/xpack/sql/cli/ResponseToString.java

@@ -69,7 +69,7 @@ abstract class ResponseToString {
         return sb;
     }
 
-    @SuppressForbidden(reason="ignore for now") // NOCOMMIT figure this out
+    @SuppressForbidden(reason="ignore for now") // NOCOMMIT replace this with saving the file and printing a message
     private static void displayGraphviz(String str) {
         try {
             // save the content to a temp file

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

@@ -16,12 +16,11 @@ import java.util.Objects;
 import static org.elasticsearch.xpack.sql.jdbc.net.protocol.ProtoUtils.classOf;
 import static org.elasticsearch.xpack.sql.jdbc.net.protocol.ProtoUtils.readValue;
 import static org.elasticsearch.xpack.sql.jdbc.net.protocol.ProtoUtils.writeValue;
+
 /**
  * Stores a page of data in a columnar format.
  */
 public class Page implements Payload {
-    private final List<ColumnInfo> columnInfo;
-
     /**
      * The actual data, one array per column.
      */

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

@@ -20,7 +20,7 @@ public class ProtoUtils {
         // NOCOMMIT <T> feels slippery here
         Object result;
         byte hasNext = in.readByte();
-        if (hasNext == 0) { // NOCOMMIT feels like a bitmask at the start of the row would be better.
+        if (hasNext == 0) { // TODO feels like a bitmask at the start of the row would be better.
             return null;
         }
         // NOCOMMIT we ought to make sure we use all of these

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

@@ -77,11 +77,11 @@ public class QueryInitResponse extends AbstractQueryResponse {
         }
         QueryInitResponse other = (QueryInitResponse) obj;
         return columns.equals(other.columns);
-        // NOCOMMIT data
+        // NOCOMMIT equals should take into account data
     }
 
     @Override
     public int hashCode() {
-        return Objects.hash(super.hashCode(), columns); // NOCOMMIT data
+        return Objects.hash(super.hashCode(), columns); // NOCOMMIT equals should take into account data
     }
 }

sql/server/src/main/java/org/elasticsearch/xpack/sql/plugin/sql/action/SqlResponse.java

@@ -27,6 +27,7 @@ public class SqlResponse extends ActionResponse implements ToXContentObject {
     private long size;
     private int columnCount;
     private List<ColumnInfo> columns;
+    // NOCOMMIT investigate reusing Page here - it probably is much more efficient
     private List<List<Object>> rows;
 
     public SqlResponse() {