From 2026198dd4014ccc57ab88685472124edce74c3a Mon Sep 17 00:00:00 2001 From: Costin Leau Date: Thu, 12 Oct 2017 00:03:41 +0300 Subject: [PATCH] Add size for column tests (elastic/x-pack-elasticsearch#2685) Add displaySize to columnInfo Original commit: elastic/x-pack-elasticsearch@ed1d265e98d17b6a34f7dd03e93870e197c694e0 --- .../elasticsearch/xpack/sql/SqlActionIT.java | 4 ++-- .../sql/jdbc/net/protocol/ColumnInfo.java | 15 ++++++++++----- .../sql/jdbc/net/protocol/ColumnInfoTests.java | 12 ++++++------ .../sql/jdbc/jdbc/JdbcDatabaseMetaData.java | 6 +++--- .../scalar/datetime/WeekOfWeekYear.java | 2 +- .../xpack/sql/plugin/RestSqlJdbcAction.java | 7 ++++--- .../sql/plugin/sql/action/SqlResponse.java | 18 +++++++++++++++--- .../plugin/sql/action/TransportSqlAction.java | 2 +- .../xpack/sql/type/JdbcUtils.java | 4 ++-- .../xpack/sql/plugin/CliFormatterTests.java | 10 +++++----- .../plugin/sql/action/SqlResponseTests.java | 3 +-- 11 files changed, 50 insertions(+), 33 deletions(-) diff --git a/plugin/src/test/java/org/elasticsearch/xpack/sql/SqlActionIT.java b/plugin/src/test/java/org/elasticsearch/xpack/sql/SqlActionIT.java index f18b312c6b7..20abdd66df6 100644 --- a/plugin/src/test/java/org/elasticsearch/xpack/sql/SqlActionIT.java +++ b/plugin/src/test/java/org/elasticsearch/xpack/sql/SqlActionIT.java @@ -35,8 +35,8 @@ public class SqlActionIT extends AbstractSqlIntegTestCase { assertThat(response.columns(), hasSize(2)); int dataIndex = dataBeforeCount ? 0 : 1; int countIndex = dataBeforeCount ? 1 : 0; - assertEquals(new ColumnInfo("data", "text", JDBCType.VARCHAR), response.columns().get(dataIndex)); - assertEquals(new ColumnInfo("count", "long", JDBCType.BIGINT), response.columns().get(countIndex)); + assertEquals(new ColumnInfo("data", "text", JDBCType.VARCHAR, 0), response.columns().get(dataIndex)); + assertEquals(new ColumnInfo("count", "long", JDBCType.BIGINT, 20), response.columns().get(countIndex)); assertThat(response.rows(), hasSize(2)); assertEquals("bar", response.rows().get(0).get(dataIndex)); diff --git a/sql/jdbc-proto/src/main/java/org/elasticsearch/xpack/sql/jdbc/net/protocol/ColumnInfo.java b/sql/jdbc-proto/src/main/java/org/elasticsearch/xpack/sql/jdbc/net/protocol/ColumnInfo.java index 7a8475ba34b..d1e19656698 100644 --- a/sql/jdbc-proto/src/main/java/org/elasticsearch/xpack/sql/jdbc/net/protocol/ColumnInfo.java +++ b/sql/jdbc-proto/src/main/java/org/elasticsearch/xpack/sql/jdbc/net/protocol/ColumnInfo.java @@ -13,9 +13,10 @@ import java.util.Objects; public class ColumnInfo { public String catalog, schema, table, label, name; + public int displaySize; public JDBCType type; - public ColumnInfo(String name, JDBCType type, String table, String catalog, String schema, String label) { + public ColumnInfo(String name, JDBCType type, String table, String catalog, String schema, String label, int displaySize) { if (name == null) { throw new IllegalArgumentException("[name] must not be null"); } @@ -40,6 +41,7 @@ public class ColumnInfo { this.catalog = catalog; this.schema = schema; this.label = label; + this.displaySize = displaySize; } ColumnInfo(DataInput in) throws IOException { @@ -49,6 +51,7 @@ public class ColumnInfo { catalog = in.readUTF(); schema = in.readUTF(); label = in.readUTF(); + displaySize = in.readInt(); } void writeTo(DataOutput out) throws IOException { @@ -58,11 +61,12 @@ public class ColumnInfo { out.writeUTF(catalog); out.writeUTF(schema); out.writeUTF(label); + out.writeInt(displaySize); } public int displaySize() { - // NOCOMMIT look at this one..... - return -1; + // 0 - means unknown + return displaySize; } @Override @@ -95,11 +99,12 @@ public class ColumnInfo { && table.equals(other.table) && catalog.equals(other.catalog) && schema.equals(other.schema) - && label.equals(other.label); + && label.equals(other.label) + && displaySize == other.displaySize; } @Override public int hashCode() { - return Objects.hash(name, type, table, catalog, schema, label); + return Objects.hash(name, type, table, catalog, schema, label, displaySize); } } diff --git a/sql/jdbc-proto/src/test/java/org/elasticsearch/xpack/sql/jdbc/net/protocol/ColumnInfoTests.java b/sql/jdbc-proto/src/test/java/org/elasticsearch/xpack/sql/jdbc/net/protocol/ColumnInfoTests.java index de7ea85e0ba..ab76432e412 100644 --- a/sql/jdbc-proto/src/test/java/org/elasticsearch/xpack/sql/jdbc/net/protocol/ColumnInfoTests.java +++ b/sql/jdbc-proto/src/test/java/org/elasticsearch/xpack/sql/jdbc/net/protocol/ColumnInfoTests.java @@ -14,15 +14,15 @@ import static org.elasticsearch.xpack.sql.test.RoundTripTestUtils.assertRoundTri public class ColumnInfoTests extends ESTestCase { static ColumnInfo varcharInfo(String name) { - return new ColumnInfo(name, JDBCType.VARCHAR, "", "", "", ""); + return new ColumnInfo(name, JDBCType.VARCHAR, "", "", "", "", 0); } static ColumnInfo intInfo(String name) { - return new ColumnInfo(name, JDBCType.INTEGER, "", "", "", ""); + return new ColumnInfo(name, JDBCType.INTEGER, "", "", "", "", 11); } static ColumnInfo doubleInfo(String name) { - return new ColumnInfo(name, JDBCType.DOUBLE, "", "", "", ""); + return new ColumnInfo(name, JDBCType.DOUBLE, "", "", "", "", 25); } static Object randomValueFor(ColumnInfo info) { @@ -37,7 +37,7 @@ public class ColumnInfoTests extends ESTestCase { static ColumnInfo randomColumnInfo() { return new ColumnInfo(randomAlphaOfLength(5), randomFrom(JDBCType.values()), randomAlphaOfLength(5), randomAlphaOfLength(5), - randomAlphaOfLength(5), randomAlphaOfLength(5)); + randomAlphaOfLength(5), randomAlphaOfLength(5), randomInt(25)); } public void testRoundTrip() throws IOException { @@ -46,9 +46,9 @@ public class ColumnInfoTests extends ESTestCase { public void testToString() { assertEquals("test.doc.a", - new ColumnInfo("a", JDBCType.VARCHAR, "test.doc", "as", "ads", "lab").toString()); + new ColumnInfo("a", JDBCType.VARCHAR, "test.doc", "as", "ads", "lab", 0).toString()); assertEquals("test.doc.a", - new ColumnInfo("a", JDBCType.VARCHAR, "test.doc", "", "", "").toString()); + new ColumnInfo("a", JDBCType.VARCHAR, "test.doc", "", "", "", 0).toString()); assertEquals("string", varcharInfo("string").toString()); assertEquals("int", intInfo("int").toString()); assertEquals("d", doubleInfo("d").toString()); diff --git a/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/jdbc/JdbcDatabaseMetaData.java b/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/jdbc/JdbcDatabaseMetaData.java index 1cd95372e77..79a92a6869d 100644 --- a/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/jdbc/JdbcDatabaseMetaData.java +++ b/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/jdbc/JdbcDatabaseMetaData.java @@ -760,14 +760,14 @@ class JdbcDatabaseMetaData implements DatabaseMetaData, JdbcWrapper { public ResultSet getCatalogs() throws SQLException { Object[][] data = { { defaultCatalog() } }; return memorySet(con.cfg, columnInfo("CATALOGS", - "TABLE_CAT"), data); + "TABLE_CAT"), data); } @Override public ResultSet getTableTypes() throws SQLException { Object[][] data = { { "TABLE" } }; return memorySet(con.cfg, columnInfo("TABLE_TYPES", - "TABLE_TYPE"), data); + "TABLE_TYPE"), data); } @Override @@ -1200,7 +1200,7 @@ class JdbcDatabaseMetaData implements DatabaseMetaData, JdbcWrapper { } // it's not, use the default and move on } - columns.add(new ColumnInfo(name, type, tableName, "INFORMATION_SCHEMA", EMPTY, EMPTY)); + columns.add(new ColumnInfo(name, type, tableName, "INFORMATION_SCHEMA", EMPTY, EMPTY, 0)); } else { throw new JdbcException("Invalid metadata schema definition"); diff --git a/sql/server/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/datetime/WeekOfWeekYear.java b/sql/server/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/datetime/WeekOfWeekYear.java index a46452a5061..c1984a9938d 100644 --- a/sql/server/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/datetime/WeekOfWeekYear.java +++ b/sql/server/src/main/java/org/elasticsearch/xpack/sql/expression/function/scalar/datetime/WeekOfWeekYear.java @@ -29,7 +29,7 @@ public class WeekOfWeekYear extends DateTimeFunction { @Override protected ChronoField chronoField() { - return ChronoField.ALIGNED_WEEK_OF_YEAR; // NOCOMMIT is this right? + return ChronoField.ALIGNED_WEEK_OF_YEAR; } @Override diff --git a/sql/server/src/main/java/org/elasticsearch/xpack/sql/plugin/RestSqlJdbcAction.java b/sql/server/src/main/java/org/elasticsearch/xpack/sql/plugin/RestSqlJdbcAction.java index 7a75239652e..dd717407185 100644 --- a/sql/server/src/main/java/org/elasticsearch/xpack/sql/plugin/RestSqlJdbcAction.java +++ b/sql/server/src/main/java/org/elasticsearch/xpack/sql/plugin/RestSqlJdbcAction.java @@ -56,6 +56,7 @@ import java.util.regex.Pattern; import static java.util.stream.Collectors.toList; import static org.elasticsearch.common.Strings.hasText; import static org.elasticsearch.rest.RestRequest.Method.POST; +import static org.elasticsearch.xpack.sql.util.StringUtils.EMPTY; public class RestSqlJdbcAction extends AbstractSqlProtocolRestAction { private final SqlLicenseChecker sqlLicenseChecker; @@ -158,7 +159,7 @@ public class RestSqlJdbcAction extends AbstractSqlProtocolRestAction { List columns = new ArrayList<>(response.columns().size()); for (SqlResponse.ColumnInfo info : response.columns()) { types.add(info.jdbcType()); - columns.add(new ColumnInfo(info.name(), info.jdbcType(), "", "", "", "")); + columns.add(new ColumnInfo(info.name(), info.jdbcType(), EMPTY, EMPTY, EMPTY, EMPTY, info.displaySize())); } return new QueryInitResponse(System.nanoTime() - start, serializeCursor(response.cursor(), types), columns, new SqlResponsePayload(types, response.rows())); @@ -174,7 +175,7 @@ public class RestSqlJdbcAction extends AbstractSqlProtocolRestAction { } catch (IOException e) { throw new IllegalArgumentException("error reading the cursor"); } - SqlRequest sqlRequest = new SqlRequest("", SqlRequest.DEFAULT_TIME_ZONE, 0, cursor); + SqlRequest sqlRequest = new SqlRequest(EMPTY, SqlRequest.DEFAULT_TIME_ZONE, 0, cursor); long start = System.nanoTime(); return channel -> client.execute(SqlAction.INSTANCE, sqlRequest, toActionListener(request, channel, response -> { return new QueryPageResponse(System.nanoTime() - start, serializeCursor(response.cursor(), types), @@ -197,4 +198,4 @@ public class RestSqlJdbcAction extends AbstractSqlProtocolRestAction { throw new RuntimeException("unexpected trouble building the cursor", e); } } -} +} \ No newline at end of file diff --git a/sql/server/src/main/java/org/elasticsearch/xpack/sql/plugin/sql/action/SqlResponse.java b/sql/server/src/main/java/org/elasticsearch/xpack/sql/plugin/sql/action/SqlResponse.java index 422b0862c4c..1cdad810970 100644 --- a/sql/server/src/main/java/org/elasticsearch/xpack/sql/plugin/sql/action/SqlResponse.java +++ b/sql/server/src/main/java/org/elasticsearch/xpack/sql/plugin/sql/action/SqlResponse.java @@ -170,17 +170,20 @@ public class SqlResponse extends ActionResponse implements ToXContentObject { private final String name; private final String esType; private final JDBCType jdbcType; + private final int displaySize; - public ColumnInfo(String name, String esType, JDBCType jdbcType) { + public ColumnInfo(String name, String esType, JDBCType jdbcType, int displaySize) { this.name = name; this.esType = esType; this.jdbcType = jdbcType; + this.displaySize = displaySize; } ColumnInfo(StreamInput in) throws IOException { name = in.readString(); esType = in.readString(); jdbcType = JDBCType.valueOf(in.readVInt()); + displaySize = in.readVInt(); } @Override @@ -188,6 +191,7 @@ public class SqlResponse extends ActionResponse implements ToXContentObject { out.writeString(name); out.writeString(esType); out.writeVInt(jdbcType.getVendorTypeNumber()); + out.writeVInt(displaySize); } @Override @@ -220,6 +224,13 @@ public class SqlResponse extends ActionResponse implements ToXContentObject { return jdbcType; } + /** + * Used by JDBC + */ + public int displaySize() { + return displaySize; + } + @Override public boolean equals(Object obj) { if (obj == null || obj.getClass() != getClass()) { @@ -228,12 +239,13 @@ public class SqlResponse extends ActionResponse implements ToXContentObject { ColumnInfo other = (ColumnInfo) obj; return name.equals(other.name) && esType.equals(other.esType) - && jdbcType.equals(other.jdbcType); + && jdbcType.equals(other.jdbcType) + && displaySize == other.displaySize; } @Override public int hashCode() { - return Objects.hash(name, esType, jdbcType); + return Objects.hash(name, esType, jdbcType, displaySize); } @Override diff --git a/sql/server/src/main/java/org/elasticsearch/xpack/sql/plugin/sql/action/TransportSqlAction.java b/sql/server/src/main/java/org/elasticsearch/xpack/sql/plugin/sql/action/TransportSqlAction.java index f6140934a49..303d2120923 100644 --- a/sql/server/src/main/java/org/elasticsearch/xpack/sql/plugin/sql/action/TransportSqlAction.java +++ b/sql/server/src/main/java/org/elasticsearch/xpack/sql/plugin/sql/action/TransportSqlAction.java @@ -69,7 +69,7 @@ public class TransportSqlAction extends HandledTransportAction(cursor.schema().types().size()); for (Schema.Entry entry : cursor.schema()) { - columns.add(new ColumnInfo(entry.name(), entry.type().esName(), entry.type().sqlType())); + columns.add(new ColumnInfo(entry.name(), entry.type().esName(), entry.type().sqlType(), entry.type().displaySize())); } columns = unmodifiableList(columns); } diff --git a/sql/server/src/main/java/org/elasticsearch/xpack/sql/type/JdbcUtils.java b/sql/server/src/main/java/org/elasticsearch/xpack/sql/type/JdbcUtils.java index 0862f256e2f..486939125a6 100644 --- a/sql/server/src/main/java/org/elasticsearch/xpack/sql/type/JdbcUtils.java +++ b/sql/server/src/main/java/org/elasticsearch/xpack/sql/type/JdbcUtils.java @@ -69,10 +69,10 @@ abstract class JdbcUtils { case FLOAT: case DOUBLE: return 25; case VARCHAR: - case VARBINARY: return -1; + case VARBINARY: return 0; case TIMESTAMP: return 20; default: - return -1; + return 0; } } diff --git a/sql/server/src/test/java/org/elasticsearch/xpack/sql/plugin/CliFormatterTests.java b/sql/server/src/test/java/org/elasticsearch/xpack/sql/plugin/CliFormatterTests.java index bcddaad4083..f51c93bc4f9 100644 --- a/sql/server/src/test/java/org/elasticsearch/xpack/sql/plugin/CliFormatterTests.java +++ b/sql/server/src/test/java/org/elasticsearch/xpack/sql/plugin/CliFormatterTests.java @@ -18,11 +18,11 @@ import static org.hamcrest.Matchers.arrayWithSize; public class CliFormatterTests extends ESTestCase { private final SqlResponse firstResponse = new SqlResponse(Cursor.EMPTY, 10, 5, Arrays.asList( - new ColumnInfo("foo", "string", JDBCType.VARCHAR), - new ColumnInfo("bar", "long", JDBCType.BIGINT), - new ColumnInfo("15charwidename!", "double", JDBCType.DOUBLE), - new ColumnInfo("superduperwidename!!!", "double", JDBCType.DOUBLE), - new ColumnInfo("baz", "keyword", JDBCType.VARCHAR)), + new ColumnInfo("foo", "string", JDBCType.VARCHAR, 0), + new ColumnInfo("bar", "long", JDBCType.BIGINT, 15), + new ColumnInfo("15charwidename!", "double", JDBCType.DOUBLE, 25), + new ColumnInfo("superduperwidename!!!", "double", JDBCType.DOUBLE, 25), + new ColumnInfo("baz", "keyword", JDBCType.VARCHAR, 0)), Arrays.asList( Arrays.asList("15charwidedata!", 1, 6.888, 12, "rabbit"), Arrays.asList("dog", 1.7976931348623157E308, 123124.888, 9912, "goat"))); diff --git a/sql/server/src/test/java/org/elasticsearch/xpack/sql/plugin/sql/action/SqlResponseTests.java b/sql/server/src/test/java/org/elasticsearch/xpack/sql/plugin/sql/action/SqlResponseTests.java index c6e7aad1e04..5921589d395 100644 --- a/sql/server/src/test/java/org/elasticsearch/xpack/sql/plugin/sql/action/SqlResponseTests.java +++ b/sql/server/src/test/java/org/elasticsearch/xpack/sql/plugin/sql/action/SqlResponseTests.java @@ -9,7 +9,6 @@ import org.elasticsearch.common.io.stream.NamedWriteableRegistry; import org.elasticsearch.test.AbstractStreamableTestCase; import org.elasticsearch.xpack.sql.execution.search.ScrollCursorTests; import org.elasticsearch.xpack.sql.plugin.SqlPlugin; -import org.elasticsearch.xpack.sql.plugin.sql.action.SqlResponse; import org.elasticsearch.xpack.sql.plugin.sql.action.SqlResponse.ColumnInfo; import org.elasticsearch.xpack.sql.session.Cursor; @@ -36,7 +35,7 @@ public class SqlResponseTests extends AbstractStreamableTestCase { if (randomBoolean()) { columns = new ArrayList<>(columnCount); for (int i = 0; i < columnCount; i++) { - columns.add(new ColumnInfo(randomAlphaOfLength(10), randomAlphaOfLength(10), randomFrom(JDBCType.values()))); + columns.add(new ColumnInfo(randomAlphaOfLength(10), randomAlphaOfLength(10), randomFrom(JDBCType.values()), randomInt(25))); } }