From d0735910494326d85171a382b7776199952b95d7 Mon Sep 17 00:00:00 2001 From: Andrei Stefan Date: Mon, 2 Jul 2018 09:51:29 +0300 Subject: [PATCH 1/2] Small bug fixes --- .../org/elasticsearch/xpack/sql/jdbc/jdbc/JdbcResultSet.java | 2 +- .../org/elasticsearch/xpack/sql/jdbc/jdbc/TypeConverter.java | 2 +- .../xpack/sql/jdbc/jdbc/TypeConverterTests.java | 5 +++-- 3 files changed, 5 insertions(+), 4 deletions(-) diff --git a/x-pack/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/jdbc/JdbcResultSet.java b/x-pack/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/jdbc/JdbcResultSet.java index 351ac73a88f28..201ae251ca0df 100644 --- a/x-pack/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/jdbc/JdbcResultSet.java +++ b/x-pack/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/jdbc/JdbcResultSet.java @@ -344,7 +344,7 @@ public T getObject(int columnIndex, Class type) throws SQLException { throw new SQLException("type is null"); } - return getObject(columnIndex, type); + return convert(columnIndex, type); } private T convert(int columnIndex, Class type) throws SQLException { diff --git a/x-pack/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/jdbc/TypeConverter.java b/x-pack/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/jdbc/TypeConverter.java index 1e24a03c8b31c..782a17257d424 100644 --- a/x-pack/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/jdbc/TypeConverter.java +++ b/x-pack/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/jdbc/TypeConverter.java @@ -254,7 +254,7 @@ static Object convert(Object v, JDBCType columnType) throws SQLException { case REAL: return floatValue(v); // Float might be represented as string for infinity and NaN values case TIMESTAMP: - return ((Number) v).longValue(); + return new Timestamp(((Number) v).longValue()); default: throw new SQLException("Unexpected column type [" + columnType.getName() + "]"); diff --git a/x-pack/plugin/sql/jdbc/src/test/java/org/elasticsearch/xpack/sql/jdbc/jdbc/TypeConverterTests.java b/x-pack/plugin/sql/jdbc/src/test/java/org/elasticsearch/xpack/sql/jdbc/jdbc/TypeConverterTests.java index 0182ea63f637d..51c130a39118e 100644 --- a/x-pack/plugin/sql/jdbc/src/test/java/org/elasticsearch/xpack/sql/jdbc/jdbc/TypeConverterTests.java +++ b/x-pack/plugin/sql/jdbc/src/test/java/org/elasticsearch/xpack/sql/jdbc/jdbc/TypeConverterTests.java @@ -14,6 +14,7 @@ import org.joda.time.ReadableDateTime; import java.sql.JDBCType; +import java.sql.Timestamp; import static org.hamcrest.Matchers.instanceOf; @@ -41,8 +42,8 @@ public void testDoubleAsNative() throws Exception { public void testTimestampAsNative() throws Exception { DateTime now = DateTime.now(); - assertThat(convertAsNative(now, JDBCType.TIMESTAMP), instanceOf(Long.class)); - assertEquals(now.getMillis(), convertAsNative(now, JDBCType.TIMESTAMP)); + assertThat(convertAsNative(now, JDBCType.TIMESTAMP), instanceOf(Timestamp.class)); + assertEquals(now.getMillis(), ((Timestamp) convertAsNative(now, JDBCType.TIMESTAMP)).getTime()); } private Object convertAsNative(Object value, JDBCType type) throws Exception { From 07c5498bb6289628d64c3d294372ddc6c2fd25ef Mon Sep 17 00:00:00 2001 From: Andrei Stefan Date: Tue, 3 Jul 2018 11:40:50 +0300 Subject: [PATCH 2/2] Test for getObject retrieving Timestamp values. Check for recursive calls to getObject that could lead to StackOverflowError --- .../qa/sql/jdbc/JdbcIntegrationTestCase.java | 6 +- .../xpack/qa/sql/jdbc/ResultSetTestCase.java | 82 +++++++++++++++++++ 2 files changed, 87 insertions(+), 1 deletion(-) create mode 100644 x-pack/qa/sql/src/main/java/org/elasticsearch/xpack/qa/sql/jdbc/ResultSetTestCase.java diff --git a/x-pack/qa/sql/src/main/java/org/elasticsearch/xpack/qa/sql/jdbc/JdbcIntegrationTestCase.java b/x-pack/qa/sql/src/main/java/org/elasticsearch/xpack/qa/sql/jdbc/JdbcIntegrationTestCase.java index a2b524c20b070..a339222445a1a 100644 --- a/x-pack/qa/sql/src/main/java/org/elasticsearch/xpack/qa/sql/jdbc/JdbcIntegrationTestCase.java +++ b/x-pack/qa/sql/src/main/java/org/elasticsearch/xpack/qa/sql/jdbc/JdbcIntegrationTestCase.java @@ -82,7 +82,11 @@ protected Connection useDataSource() throws SQLException { } public static void index(String index, CheckedConsumer body) throws IOException { - Request request = new Request("PUT", "/" + index + "/doc/1"); + index(index, "1", body); + } + + public static void index(String index, String documentId, CheckedConsumer body) throws IOException { + Request request = new Request("PUT", "/" + index + "/doc/" + documentId); request.addParameter("refresh", "true"); XContentBuilder builder = JsonXContent.contentBuilder().startObject(); body.accept(builder); diff --git a/x-pack/qa/sql/src/main/java/org/elasticsearch/xpack/qa/sql/jdbc/ResultSetTestCase.java b/x-pack/qa/sql/src/main/java/org/elasticsearch/xpack/qa/sql/jdbc/ResultSetTestCase.java new file mode 100644 index 0000000000000..861a6dccaba57 --- /dev/null +++ b/x-pack/qa/sql/src/main/java/org/elasticsearch/xpack/qa/sql/jdbc/ResultSetTestCase.java @@ -0,0 +1,82 @@ +/* + * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one + * or more contributor license agreements. Licensed under the Elastic License; + * you may not use this file except in compliance with the Elastic License. + */ +package org.elasticsearch.xpack.qa.sql.jdbc; + +import java.io.IOException; +import java.sql.Connection; +import java.sql.PreparedStatement; +import java.sql.ResultSet; +import java.sql.ResultSetMetaData; +import java.sql.SQLException; +import java.sql.Timestamp; +import java.util.Date; + +public class ResultSetTestCase extends JdbcIntegrationTestCase { + public void testGettingTimestamp() throws Exception { + long randomMillis = randomLongBetween(0, System.currentTimeMillis()); + + index("library", "1", builder -> { + builder.field("name", "Don Quixote"); + builder.field("page_count", 1072); + builder.timeField("release_date", new Date(randomMillis)); + builder.timeField("republish_date", null); + }); + index("library", "2", builder -> { + builder.field("name", "1984"); + builder.field("page_count", 328); + builder.timeField("release_date", new Date(-649036800000L)); + builder.timeField("republish_date", new Date(599616000000L)); + }); + + try (Connection connection = esJdbc()) { + try (PreparedStatement statement = connection.prepareStatement("SELECT name, release_date, republish_date FROM library")) { + try (ResultSet results = statement.executeQuery()) { + ResultSetMetaData resultSetMetaData = results.getMetaData(); + + results.next(); + assertEquals(3, resultSetMetaData.getColumnCount()); + assertEquals(randomMillis, results.getTimestamp("release_date").getTime()); + assertEquals(randomMillis, results.getTimestamp(2).getTime()); + assertTrue(results.getObject(2) instanceof Timestamp); + assertEquals(randomMillis, ((Timestamp) results.getObject("release_date")).getTime()); + + assertNull(results.getTimestamp(3)); + assertNull(results.getObject("republish_date")); + + assertTrue(results.next()); + assertEquals(599616000000L, results.getTimestamp("republish_date").getTime()); + assertEquals(-649036800000L, ((Timestamp) results.getObject(2)).getTime()); + + assertFalse(results.next()); + } + } + } + } + + /* + * Checks StackOverflowError fix for https://github.com/elastic/elasticsearch/pull/31735 + */ + public void testNoInfiniteRecursiveGetObjectCalls() throws SQLException, IOException { + index("library", "1", builder -> { + builder.field("name", "Don Quixote"); + builder.field("page_count", 1072); + }); + Connection conn = esJdbc(); + PreparedStatement statement = conn.prepareStatement("SELECT * FROM library"); + ResultSet results = statement.executeQuery(); + + try { + results.next(); + results.getObject("name"); + results.getObject("page_count"); + results.getObject(1); + results.getObject(1, String.class); + results.getObject("page_count", Integer.class); + } catch (StackOverflowError soe) { + fail("Infinite recursive call on getObject() method"); + } + } +}