From c6fdf9ed8aee9b210eed2dae632f422861ff87f7 Mon Sep 17 00:00:00 2001 From: Andrei Stefan Date: Tue, 17 Dec 2019 10:00:54 +0200 Subject: [PATCH] Handle NULL in ResultSet's getDate() method (#50184) (cherry picked from commit 08214eb1338fef5c8082c3f8b84c24dd53224ebe) --- .../xpack/sql/jdbc/JdbcResultSet.java | 10 ++-- .../xpack/sql/qa/jdbc/ResultSetTestCase.java | 52 +++++++++++++++++++ 2 files changed, 58 insertions(+), 4 deletions(-) diff --git a/x-pack/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/JdbcResultSet.java b/x-pack/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/JdbcResultSet.java index 6002ef14968..71d46ad66d9 100644 --- a/x-pack/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/JdbcResultSet.java +++ b/x-pack/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/JdbcResultSet.java @@ -259,6 +259,11 @@ class JdbcResultSet implements ResultSet, JdbcWrapper { private Long dateTimeAsMillis(int columnIndex) throws SQLException { Object val = column(columnIndex); EsType type = columnType(columnIndex); + + if (val == null) { + return null; + } + try { // TODO: the B6 appendix of the jdbc spec does mention CHAR, VARCHAR, LONGVARCHAR, DATE, TIMESTAMP as supported // jdbc types that should be handled by getDate and getTime methods. From all of those we support VARCHAR and @@ -267,9 +272,6 @@ class JdbcResultSet implements ResultSet, JdbcWrapper { // the cursor can return an Integer if the date-since-epoch is small enough, XContentParser (Jackson) will // return the "smallest" data type for numbers when parsing // TODO: this should probably be handled server side - if (val == null) { - return null; - } return asDateTimeField(val, JdbcDateUtils::dateTimeAsMillisSinceEpoch, Function.identity()); } if (DATE == type) { @@ -278,7 +280,7 @@ class JdbcResultSet implements ResultSet, JdbcWrapper { if (TIME == type) { return timeAsMillisSinceEpoch(val.toString()); } - return val == null ? null : (Long) val; + return (Long) val; } catch (ClassCastException cce) { throw new SQLException( format(Locale.ROOT, "Unable to convert value [%.128s] of type [%s] to a Long", val, type.getName()), cce); diff --git a/x-pack/plugin/sql/qa/src/main/java/org/elasticsearch/xpack/sql/qa/jdbc/ResultSetTestCase.java b/x-pack/plugin/sql/qa/src/main/java/org/elasticsearch/xpack/sql/qa/jdbc/ResultSetTestCase.java index b842193efc9..e59316ed66f 100644 --- a/x-pack/plugin/sql/qa/src/main/java/org/elasticsearch/xpack/sql/qa/jdbc/ResultSetTestCase.java +++ b/x-pack/plugin/sql/qa/src/main/java/org/elasticsearch/xpack/sql/qa/jdbc/ResultSetTestCase.java @@ -1275,6 +1275,54 @@ public class ResultSetTestCase extends JdbcIntegrationTestCase { assertTrue(results.getObject("test_boolean") instanceof Boolean); }); } + + public void testGettingNullValues() throws Exception { + String query = "SELECT CAST(NULL AS BOOLEAN) b, CAST(NULL AS TINYINT) t, CAST(NULL AS SMALLINT) s, CAST(NULL AS INTEGER) i," + + "CAST(NULL AS BIGINT) bi, CAST(NULL AS DOUBLE) d, CAST(NULL AS REAL) r, CAST(NULL AS FLOAT) f, CAST(NULL AS VARCHAR) v," + + "CAST(NULL AS DATE) dt, CAST(NULL AS TIME) tm, CAST(NULL AS TIMESTAMP) ts"; + doWithQuery(query, (results) -> { + results.next(); + + assertNull(results.getObject("b")); + assertFalse(results.getBoolean("b")); + + assertNull(results.getObject("t")); + assertEquals(0, results.getByte("t")); + + assertNull(results.getObject("s")); + assertEquals(0, results.getShort("s")); + + assertNull(results.getObject("i")); + assertEquals(0, results.getInt("i")); + + assertNull(results.getObject("bi")); + assertEquals(0, results.getLong("bi")); + + assertNull(results.getObject("d")); + assertEquals(0.0d, results.getDouble("d"), 0d); + + assertNull(results.getObject("r")); + assertEquals(0.0f, results.getFloat("r"), 0f); + + assertNull(results.getObject("f")); + assertEquals(0.0f, results.getFloat("f"), 0f); + + assertNull(results.getObject("v")); + assertNull(results.getString("v")); + + assertNull(results.getObject("dt")); + assertNull(results.getDate("dt")); + assertNull(results.getDate("dt", randomCalendar())); + + assertNull(results.getObject("tm")); + assertNull(results.getTime("tm")); + assertNull(results.getTime("tm", randomCalendar())); + + assertNull(results.getObject("ts")); + assertNull(results.getTimestamp("ts")); + assertNull(results.getTimestamp("ts", randomCalendar())); + }); + } /* * Checks StackOverflowError fix for https://github.com/elastic/elasticsearch/pull/31735 @@ -1841,4 +1889,8 @@ public class ResultSetTestCase extends JdbcIntegrationTestCase { private ZoneId getZoneFromOffset(Long randomLongDate) { return ZoneId.of(ZoneId.of(timeZoneId).getRules().getOffset(Instant.ofEpochMilli(randomLongDate)).toString()); } + + private Calendar randomCalendar() { + return Calendar.getInstance(randomTimeZone(), Locale.ROOT); + } }