From 98b6dffcbf0f5696028e0ae9c5d51ea8dca90753 Mon Sep 17 00:00:00 2001 From: Costin Leau Date: Fri, 23 Nov 2018 21:07:54 +0200 Subject: [PATCH] SQL: SYS COLUMNS returns ODBC specific schema Due to some unresolvable type conflict between the expected definition in JDBC vs ODBC, the driver mode is now passed over so that certain command can change their results accordingly (in this case SYS COLUMNS) Fix #35376 --- .../plan/logical/command/sys/SysColumns.java | 50 +++++++---- .../sql/plugin/TransportSqlQueryAction.java | 2 +- .../plugin/TransportSqlTranslateAction.java | 2 +- .../xpack/sql/session/Configuration.java | 19 +++-- .../logical/command/sys/SysColumnsTests.java | 84 ++++++++++++++++++- .../logical/command/sys/SysParserTests.java | 11 +-- 6 files changed, 137 insertions(+), 31 deletions(-) diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plan/logical/command/sys/SysColumns.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plan/logical/command/sys/SysColumns.java index 48682b2e134f6..e5a8a19be7819 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plan/logical/command/sys/SysColumns.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/plan/logical/command/sys/SysColumns.java @@ -11,6 +11,7 @@ import org.elasticsearch.xpack.sql.expression.Attribute; import org.elasticsearch.xpack.sql.expression.predicate.regex.LikePattern; import org.elasticsearch.xpack.sql.plan.logical.command.Command; +import org.elasticsearch.xpack.sql.proto.Mode; import org.elasticsearch.xpack.sql.session.Rows; import org.elasticsearch.xpack.sql.session.SchemaRowSet; import org.elasticsearch.xpack.sql.session.SqlSession; @@ -58,21 +59,29 @@ protected NodeInfo info() { @Override public List output() { + return output(false); + } + + private List output(boolean odbcCompatible) { + // https://github.com/elastic/elasticsearch/issues/35376 + // ODBC expects some fields as SHORT while JDBC as Integer + // which causes conversion issues and CCE + DataType clientBasedType = odbcCompatible ? SHORT : INTEGER; return asList(keyword("TABLE_CAT"), keyword("TABLE_SCHEM"), keyword("TABLE_NAME"), keyword("COLUMN_NAME"), - field("DATA_TYPE", INTEGER), + field("DATA_TYPE", clientBasedType), keyword("TYPE_NAME"), field("COLUMN_SIZE", INTEGER), field("BUFFER_LENGTH", INTEGER), - field("DECIMAL_DIGITS", INTEGER), - field("NUM_PREC_RADIX", INTEGER), - field("NULLABLE", INTEGER), + field("DECIMAL_DIGITS", clientBasedType), + field("NUM_PREC_RADIX", clientBasedType), + field("NULLABLE", clientBasedType), keyword("REMARKS"), keyword("COLUMN_DEF"), - field("SQL_DATA_TYPE", INTEGER), - field("SQL_DATETIME_SUB", INTEGER), + field("SQL_DATA_TYPE", clientBasedType), + field("SQL_DATETIME_SUB", clientBasedType), field("CHAR_OCTET_LENGTH", INTEGER), field("ORDINAL_POSITION", INTEGER), keyword("IS_NULLABLE"), @@ -88,11 +97,13 @@ public List output() { @Override public void execute(SqlSession session, ActionListener listener) { + boolean isOdbcClient = session.settings().mode() == Mode.ODBC; + List output = output(isOdbcClient); String cluster = session.indexResolver().clusterName(); // bail-out early if the catalog is present but differs if (Strings.hasText(catalog) && !cluster.equals(catalog)) { - listener.onResponse(Rows.empty(output())); + listener.onResponse(Rows.empty(output)); return; } @@ -104,15 +115,15 @@ public void execute(SqlSession session, ActionListener listener) { session.indexResolver().resolveAsSeparateMappings(idx, regex, ActionListener.wrap(esIndices -> { List> rows = new ArrayList<>(); for (EsIndex esIndex : esIndices) { - fillInRows(cluster, esIndex.name(), esIndex.mapping(), null, rows, columnMatcher); + fillInRows(cluster, esIndex.name(), esIndex.mapping(), null, rows, columnMatcher, isOdbcClient); } - listener.onResponse(Rows.of(output(), rows)); + listener.onResponse(Rows.of(output, rows)); }, listener::onFailure)); } static void fillInRows(String clusterName, String indexName, Map mapping, String prefix, List> rows, - Pattern columnMatcher) { + Pattern columnMatcher, boolean isOdbcClient) { int pos = 0; for (Map.Entry entry : mapping.entrySet()) { pos++; // JDBC is 1-based so we start with 1 here @@ -128,24 +139,24 @@ static void fillInRows(String clusterName, String indexName, Map listener.onResponse(new SqlTranslateResponse(searchSourceBuilder)), listener::onFailure)); diff --git a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/session/Configuration.java b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/session/Configuration.java index eafb71d12a8f2..0161fcedb0532 100644 --- a/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/session/Configuration.java +++ b/x-pack/plugin/sql/src/main/java/org/elasticsearch/xpack/sql/session/Configuration.java @@ -8,6 +8,7 @@ import org.elasticsearch.common.Nullable; import org.elasticsearch.common.unit.TimeValue; import org.elasticsearch.index.query.QueryBuilder; +import org.elasticsearch.xpack.sql.proto.Mode; import org.elasticsearch.xpack.sql.proto.Protocol; import java.util.TimeZone; @@ -15,22 +16,24 @@ // Typed object holding properties for a given action public class Configuration { public static final Configuration DEFAULT = new Configuration(TimeZone.getTimeZone("UTC"), - Protocol.FETCH_SIZE, Protocol.REQUEST_TIMEOUT, Protocol.PAGE_TIMEOUT, null); + Protocol.FETCH_SIZE, Protocol.REQUEST_TIMEOUT, Protocol.PAGE_TIMEOUT, null, Mode.PLAIN); - private TimeZone timeZone; - private int pageSize; - private TimeValue requestTimeout; - private TimeValue pageTimeout; + private final TimeZone timeZone; + private final int pageSize; + private final TimeValue requestTimeout; + private final TimeValue pageTimeout; + private final Mode mode; @Nullable private QueryBuilder filter; - public Configuration(TimeZone tz, int pageSize, TimeValue requestTimeout, TimeValue pageTimeout, QueryBuilder filter) { + public Configuration(TimeZone tz, int pageSize, TimeValue requestTimeout, TimeValue pageTimeout, QueryBuilder filter, Mode mode) { this.timeZone = tz; this.pageSize = pageSize; this.requestTimeout = requestTimeout; this.pageTimeout = pageTimeout; this.filter = filter; + this.mode = mode == null ? Mode.PLAIN : mode; } public TimeZone timeZone() { @@ -52,4 +55,8 @@ public TimeValue pageTimeout() { public QueryBuilder filter() { return filter; } + + public Mode mode() { + return mode; + } } diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/plan/logical/command/sys/SysColumnsTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/plan/logical/command/sys/SysColumnsTests.java index 0b82530022386..68533748eb1c2 100644 --- a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/plan/logical/command/sys/SysColumnsTests.java +++ b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/plan/logical/command/sys/SysColumnsTests.java @@ -16,7 +16,7 @@ public class SysColumnsTests extends ESTestCase { public void testSysColumns() { List> rows = new ArrayList<>(); - SysColumns.fillInRows("test", "index", TypesTests.loadMapping("mapping-multi-field-variation.json", true), null, rows, null); + SysColumns.fillInRows("test", "index", TypesTests.loadMapping("mapping-multi-field-variation.json", true), null, rows, null, false); assertEquals(16, rows.size()); assertEquals(24, rows.get(0).size()); @@ -58,6 +58,70 @@ public void testSysColumns() { assertEquals(Integer.MAX_VALUE, bufferLength(row)); } + public void testSysColumnsInOdbcMode() { + List> rows = new ArrayList<>(); + SysColumns.fillInRows("test", "index", TypesTests.loadMapping("mapping-multi-field-variation.json", true), null, rows, null, true); + assertEquals(16, rows.size()); + assertEquals(24, rows.get(0).size()); + + List row = rows.get(0); + assertEquals("bool", name(row)); + assertEquals((short) Types.BOOLEAN, sqlType(row)); + assertEquals(null, radix(row)); + assertEquals(1, bufferLength(row)); + + row = rows.get(1); + assertEquals("int", name(row)); + assertEquals((short) Types.INTEGER, sqlType(row)); + assertEquals(Short.class, radix(row).getClass()); + assertEquals(4, bufferLength(row)); + assertNull(decimalPrecision(row)); + assertEquals(Short.class, nullable(row).getClass()); + assertEquals(Short.class, sqlDataType(row).getClass()); + assertEquals(Short.class, sqlDataTypeSub(row).getClass()); + + row = rows.get(2); + assertEquals("text", name(row)); + assertEquals((short) Types.VARCHAR, sqlType(row)); + assertEquals(null, radix(row)); + assertEquals(Integer.MAX_VALUE, bufferLength(row)); + assertNull(decimalPrecision(row)); + assertEquals(Short.class, nullable(row).getClass()); + assertEquals(Short.class, sqlDataType(row).getClass()); + assertEquals(Short.class, sqlDataTypeSub(row).getClass()); + + row = rows.get(4); + assertEquals("date", name(row)); + assertEquals((short) Types.TIMESTAMP, sqlType(row)); + assertEquals(null, radix(row)); + assertEquals(24, precision(row)); + assertEquals(8, bufferLength(row)); + assertNull(decimalPrecision(row)); + assertEquals(Short.class, nullable(row).getClass()); + assertEquals(Short.class, sqlDataType(row).getClass()); + assertEquals(Short.class, sqlDataTypeSub(row).getClass()); + + row = rows.get(7); + assertEquals("some.dotted", name(row)); + assertEquals((short) Types.STRUCT, sqlType(row)); + assertEquals(null, radix(row)); + assertEquals(-1, bufferLength(row)); + assertNull(decimalPrecision(row)); + assertEquals(Short.class, nullable(row).getClass()); + assertEquals(Short.class, sqlDataType(row).getClass()); + assertEquals(Short.class, sqlDataTypeSub(row).getClass()); + + row = rows.get(15); + assertEquals("some.ambiguous.normalized", name(row)); + assertEquals((short) Types.VARCHAR, sqlType(row)); + assertEquals(null, radix(row)); + assertEquals(Integer.MAX_VALUE, bufferLength(row)); + assertNull(decimalPrecision(row)); + assertEquals(Short.class, nullable(row).getClass()); + assertEquals(Short.class, sqlDataType(row).getClass()); + assertEquals(Short.class, sqlDataTypeSub(row).getClass()); + } + private static Object name(List list) { return list.get(3); } @@ -77,4 +141,20 @@ private static Object bufferLength(List list) { private static Object radix(List list) { return list.get(9); } -} + + private static Object nullable(List list) { + return list.get(10); + } + + private static Object decimalPrecision(List list) { + return list.get(8); + } + + private static Object sqlDataType(List list) { + return list.get(13); + } + + private static Object sqlDataTypeSub(List list) { + return list.get(14); + } +} \ No newline at end of file diff --git a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/plan/logical/command/sys/SysParserTests.java b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/plan/logical/command/sys/SysParserTests.java index 974bdd2de8f97..7d40047cb3f41 100644 --- a/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/plan/logical/command/sys/SysParserTests.java +++ b/x-pack/plugin/sql/src/test/java/org/elasticsearch/xpack/sql/plan/logical/command/sys/SysParserTests.java @@ -15,6 +15,7 @@ import org.elasticsearch.xpack.sql.expression.function.FunctionRegistry; import org.elasticsearch.xpack.sql.parser.SqlParser; import org.elasticsearch.xpack.sql.plan.logical.command.Command; +import org.elasticsearch.xpack.sql.session.Configuration; import org.elasticsearch.xpack.sql.session.SqlSession; import org.elasticsearch.xpack.sql.type.DataType; import org.elasticsearch.xpack.sql.type.EsField; @@ -50,7 +51,7 @@ private Tuple sql(String sql) { return Void.TYPE; }).when(resolver).resolveAsSeparateMappings(any(), any(), any()); - SqlSession session = new SqlSession(null, null, null, resolver, null, null, null); + SqlSession session = new SqlSession(Configuration.DEFAULT, null, null, resolver, null, null, null); return new Tuple<>(cmd, session); } @@ -58,10 +59,10 @@ public void testSysTypes() throws Exception { Command cmd = sql("SYS TYPES").v1(); List names = asList("BYTE", "LONG", "BINARY", "NULL", "INTEGER", "SHORT", "HALF_FLOAT", "SCALED_FLOAT", "FLOAT", "DOUBLE", - "KEYWORD", "TEXT", "IP", "BOOLEAN", "DATE", - "INTERVAL_YEAR", "INTERVAL_MONTH", "INTERVAL_DAY", "INTERVAL_HOUR", "INTERVAL_MINUTE", "INTERVAL_SECOND", - "INTERVAL_YEAR_TO_MONTH", "INTERVAL_DAY_TO_HOUR", "INTERVAL_DAY_TO_MINUTE", "INTERVAL_DAY_TO_SECOND", - "INTERVAL_HOUR_TO_MINUTE", "INTERVAL_HOUR_TO_SECOND", "INTERVAL_MINUTE_TO_SECOND", + "KEYWORD", "TEXT", "IP", "BOOLEAN", "DATE", + "INTERVAL_YEAR", "INTERVAL_MONTH", "INTERVAL_DAY", "INTERVAL_HOUR", "INTERVAL_MINUTE", "INTERVAL_SECOND", + "INTERVAL_YEAR_TO_MONTH", "INTERVAL_DAY_TO_HOUR", "INTERVAL_DAY_TO_MINUTE", "INTERVAL_DAY_TO_SECOND", + "INTERVAL_HOUR_TO_MINUTE", "INTERVAL_HOUR_TO_SECOND", "INTERVAL_MINUTE_TO_SECOND", "UNSUPPORTED", "OBJECT", "NESTED"); cmd.execute(null, ActionListener.wrap(r -> {