From bf84de2fc4d688fddb8d05aa11f9d0672ce97051 Mon Sep 17 00:00:00 2001 From: Nik Everett Date: Fri, 23 Jun 2017 16:19:31 -0400 Subject: [PATCH] Opinions Original commit: elastic/x-pack-elasticsearch@ae64b6355e54e799fc32cb6fe0ab3ceb4f47c4cb --- .../elasticsearch/xpack/sql/jdbc/net/protocol/ProtoUtils.java | 2 +- .../xpack/sql/jdbc/net/client/HttpJdbcClient.java | 4 +++- 2 files changed, 4 insertions(+), 2 deletions(-) diff --git a/sql-clients/jdbc-proto/src/main/java/org/elasticsearch/xpack/sql/jdbc/net/protocol/ProtoUtils.java b/sql-clients/jdbc-proto/src/main/java/org/elasticsearch/xpack/sql/jdbc/net/protocol/ProtoUtils.java index 6b698f879d8..65da2859204 100644 --- a/sql-clients/jdbc-proto/src/main/java/org/elasticsearch/xpack/sql/jdbc/net/protocol/ProtoUtils.java +++ b/sql-clients/jdbc-proto/src/main/java/org/elasticsearch/xpack/sql/jdbc/net/protocol/ProtoUtils.java @@ -114,7 +114,7 @@ public abstract class ProtoUtils { public static T readValue(DataInput in, int type) throws IOException { Object result; byte hasNext = in.readByte(); - if (hasNext == 0) { + if (hasNext == 0) { // NOCOMMIT feels like a bitmask at the start of the row would be better. return null; } switch (type) { diff --git a/sql-clients/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/net/client/HttpJdbcClient.java b/sql-clients/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/net/client/HttpJdbcClient.java index 9b8fd2c2dca..2dce19c7cca 100644 --- a/sql-clients/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/net/client/HttpJdbcClient.java +++ b/sql-clients/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/net/client/HttpJdbcClient.java @@ -120,7 +120,7 @@ public class HttpJdbcClient implements Closeable { // read the data // allocate columns int rows = in.readInt(); - page.resize(rows); + page.resize(rows); // NOCOMMIT I believe this is duplicated with readData readData(in, page, rows); return response.requestId; @@ -192,6 +192,7 @@ public class HttpJdbcClient implements Closeable { Response response = ProtoUtils.readResponse(in, header); + // NOCOMMIT why not move the throw login into readResponse? if (response instanceof ExceptionResponse) { ExceptionResponse ex = (ExceptionResponse) response; throw SqlExceptionType.asException(ex.asSql, ex.message); @@ -201,6 +202,7 @@ public class HttpJdbcClient implements Closeable { throw new JdbcException("%s", error.stack); } if (response instanceof Response) { + // NOCOMMIT I'd feel more comfortable either returning Response and passing the class in and calling responseClass.cast(response) return (R) response; }