Skip to content

Commit 0c4b316

Browse files
astefanastefan
authored
SQL: test coverage for JdbcResultSet (#32813)
* Tests for JdbcResultSet * Added VARCHAR conversion for different types * Made error messages consistent: they now contain both the type that fails to be converted and the value itself
1 parent f6a5708 commit 0c4b316

File tree

6 files changed

+1624
-111
lines changed

6 files changed

+1624
-111
lines changed

‎x-pack/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/jdbc/JdbcResultSet.java

+22-43
Original file line numberDiff line numberDiff line change
@@ -133,72 +133,37 @@ public String getString(int columnIndex) throws SQLException {
133133

134134
@Override
135135
public boolean getBoolean(int columnIndex) throws SQLException {
136-
Object val = column(columnIndex);
137-
try {
138-
return val != null ? (Boolean) val : false;
139-
} catch (ClassCastException cce) {
140-
throw new SQLException("unable to convert column " + columnIndex + " to a boolean", cce);
141-
}
136+
return column(columnIndex) != null ? getObject(columnIndex, Boolean.class) : false;
142137
}
143138

144139
@Override
145140
public byte getByte(int columnIndex) throws SQLException {
146-
Object val = column(columnIndex);
147-
try {
148-
return val != null ? ((Number) val).byteValue() : 0;
149-
} catch (ClassCastException cce) {
150-
throw new SQLException("unable to convert column " + columnIndex + " to a byte", cce);
151-
}
141+
return column(columnIndex) != null ? getObject(columnIndex, Byte.class) : 0;
152142
}
153143

154144
@Override
155145
public short getShort(int columnIndex) throws SQLException {
156-
Object val = column(columnIndex);
157-
try {
158-
return val != null ? ((Number) val).shortValue() : 0;
159-
} catch (ClassCastException cce) {
160-
throw new SQLException("unable to convert column " + columnIndex + " to a short", cce);
161-
}
146+
return column(columnIndex) != null ? getObject(columnIndex, Short.class) : 0;
162147
}
163148

164149
@Override
165150
public int getInt(int columnIndex) throws SQLException {
166-
Object val = column(columnIndex);
167-
try {
168-
return val != null ? ((Number) val).intValue() : 0;
169-
} catch (ClassCastException cce) {
170-
throw new SQLException("unable to convert column " + columnIndex + " to an int", cce);
171-
}
151+
return column(columnIndex) != null ? getObject(columnIndex, Integer.class) : 0;
172152
}
173153

174154
@Override
175155
public long getLong(int columnIndex) throws SQLException {
176-
Object val = column(columnIndex);
177-
try {
178-
return val != null ? ((Number) val).longValue() : 0;
179-
} catch (ClassCastException cce) {
180-
throw new SQLException("unable to convert column " + columnIndex + " to a long", cce);
181-
}
156+
return column(columnIndex) != null ? getObject(columnIndex, Long.class) : 0;
182157
}
183158

184159
@Override
185160
public float getFloat(int columnIndex) throws SQLException {
186-
Object val = column(columnIndex);
187-
try {
188-
return val != null ? ((Number) val).floatValue() : 0;
189-
} catch (ClassCastException cce) {
190-
throw new SQLException("unable to convert column " + columnIndex + " to a float", cce);
191-
}
161+
return column(columnIndex) != null ? getObject(columnIndex, Float.class) : 0;
192162
}
193163

194164
@Override
195165
public double getDouble(int columnIndex) throws SQLException {
196-
Object val = column(columnIndex);
197-
try {
198-
return val != null ? ((Number) val).doubleValue() : 0;
199-
} catch (ClassCastException cce) {
200-
throw new SQLException("unable to convert column " + columnIndex + " to a double", cce);
201-
}
166+
return column(columnIndex) != null ? getObject(columnIndex, Double.class) : 0;
202167
}
203168

204169
@Override
@@ -272,15 +237,29 @@ public byte[] getBytes(String columnLabel) throws SQLException {
272237

273238
@Override
274239
public Date getDate(String columnLabel) throws SQLException {
240+
// TODO: the error message in case the value in the column cannot be converted to a Date refers to a column index
241+
// (for example - "unable to convert column 4 to a long") and not to the column name, which is a bit confusing.
242+
// Should we reconsider this? Maybe by catching the exception here and rethrowing it with the columnLabel instead.
275243
return getDate(column(columnLabel));
276244
}
277245

278246
private Long dateTime(int columnIndex) throws SQLException {
279247
Object val = column(columnIndex);
248+
JDBCType type = cursor.columns().get(columnIndex - 1).type;
280249
try {
250+
// TODO: the B6 appendix of the jdbc spec does mention CHAR, VARCHAR, LONGVARCHAR, DATE, TIMESTAMP as supported
251+
// jdbc types that should be handled by getDate and getTime methods. From all of those we support VARCHAR and
252+
// TIMESTAMP. Should we consider the VARCHAR conversion as a later enhancement?
253+
if (JDBCType.TIMESTAMP.equals(type)) {
254+
// the cursor can return an Integer if the date-since-epoch is small enough, XContentParser (Jackson) will
255+
// return the "smallest" data type for numbers when parsing
256+
// TODO: this should probably be handled server side
257+
return val == null ? null : ((Number) val).longValue();
258+
};
281259
return val == null ? null : (Long) val;
282260
} catch (ClassCastException cce) {
283-
throw new SQLException("unable to convert column " + columnIndex + " to a long", cce);
261+
throw new SQLException(
262+
format(Locale.ROOT, "Unable to convert value [%.128s] of type [%s] to a Long", val, type.getName()), cce);
284263
}
285264
}
286265

‎x-pack/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/jdbc/TypeConverter.java

+70-17
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@
1010

1111
import java.sql.Date;
1212
import java.sql.JDBCType;
13-
import java.sql.SQLDataException;
1413
import java.sql.SQLException;
1514
import java.sql.SQLFeatureNotSupportedException;
1615
import java.sql.Time;
@@ -56,9 +55,10 @@ private TypeConverter() {
5655

5756
}
5857

59-
private static final long DAY_IN_MILLIS = 60 * 60 * 24;
58+
private static final long DAY_IN_MILLIS = 60 * 60 * 24 * 1000;
6059
private static final Map<Class<?>, JDBCType> javaToJDBC;
6160

61+
6262
static {
6363
Map<Class<?>, JDBCType> aMap = Arrays.stream(DataType.values())
6464
.filter(dataType -> dataType.javaClass() != null
@@ -120,6 +120,7 @@ private static <T> T dateTimeConvert(Long millis, Calendar c, Function<Calendar,
120120
}
121121
}
122122

123+
123124
static long convertFromCalendarToUTC(long value, Calendar cal) {
124125
if (cal == null) {
125126
return value;
@@ -143,11 +144,15 @@ static <T> T convert(Object val, JDBCType columnType, Class<T> type) throws SQLE
143144
return (T) convert(val, columnType);
144145
}
145146

146-
if (type.isInstance(val)) {
147+
// converting a Long to a Timestamp shouldn't be possible according to the spec,
148+
// it feels a little brittle to check this scenario here and I don't particularly like it
149+
// TODO: can we do any better or should we go over the spec and allow getLong(date) to be valid?
150+
if (!(type == Long.class && columnType == JDBCType.TIMESTAMP) && type.isInstance(val)) {
147151
try {
148152
return type.cast(val);
149153
} catch (ClassCastException cce) {
150-
throw new SQLDataException("Unable to convert " + val.getClass().getName() + " to " + columnType, cce);
154+
throw new SQLException(format(Locale.ROOT, "Unable to convert value [%.128s] of type [%s] to a %s", val,
155+
columnType.getName(), type.getName()), cce);
151156
}
152157
}
153158

@@ -205,7 +210,8 @@ static <T> T convert(Object val, JDBCType columnType, Class<T> type) throws SQLE
205210
if (type == OffsetDateTime.class) {
206211
return (T) asOffsetDateTime(val, columnType);
207212
}
208-
throw new SQLException("Conversion from type [" + columnType + "] to [" + type.getName() + "] not supported");
213+
throw new SQLException(format(Locale.ROOT, "Unable to convert value [%.128s] of type [%s] to a %s", val,
214+
columnType.getName(), type.getName()));
209215
}
210216

211217
/**
@@ -336,8 +342,11 @@ private static Boolean asBoolean(Object val, JDBCType columnType) throws SQLExce
336342
case FLOAT:
337343
case DOUBLE:
338344
return Boolean.valueOf(Integer.signum(((Number) val).intValue()) != 0);
345+
case VARCHAR:
346+
return Boolean.valueOf((String) val);
339347
default:
340-
throw new SQLException("Conversion from type [" + columnType + "] to [Boolean] not supported");
348+
throw new SQLException(
349+
format(Locale.ROOT, "Unable to convert value [%.128s] of type [%s] to a Boolean", val, columnType.getName()));
341350

342351
}
343352
}
@@ -355,10 +364,16 @@ private static Byte asByte(Object val, JDBCType columnType) throws SQLException
355364
case FLOAT:
356365
case DOUBLE:
357366
return safeToByte(safeToLong(((Number) val).doubleValue()));
367+
case VARCHAR:
368+
try {
369+
return Byte.valueOf((String) val);
370+
} catch (NumberFormatException e) {
371+
throw new SQLException(format(Locale.ROOT, "Unable to convert value [%.128s] of type [VARCHAR] to a Byte", val), e);
372+
}
358373
default:
359374
}
360375

361-
throw new SQLException("Conversion from type [" + columnType + "] to [Byte] not supported");
376+
throw new SQLException(format(Locale.ROOT, "Unable to convert value [%.128s] of type [%s] to a Byte", val, columnType.getName()));
362377
}
363378

364379
private static Short asShort(Object val, JDBCType columnType) throws SQLException {
@@ -374,10 +389,16 @@ private static Short asShort(Object val, JDBCType columnType) throws SQLExceptio
374389
case FLOAT:
375390
case DOUBLE:
376391
return safeToShort(safeToLong(((Number) val).doubleValue()));
392+
case VARCHAR:
393+
try {
394+
return Short.valueOf((String) val);
395+
} catch (NumberFormatException e) {
396+
throw new SQLException(format(Locale.ROOT, "Unable to convert value [%.128s] of type [VARCHAR] to a Short", val), e);
397+
}
377398
default:
378399
}
379400

380-
throw new SQLException("Conversion from type [" + columnType + "] to [Short] not supported");
401+
throw new SQLException(format(Locale.ROOT, "Unable to convert value [%.128s] of type [%s] to a Short", val, columnType.getName()));
381402
}
382403

383404
private static Integer asInteger(Object val, JDBCType columnType) throws SQLException {
@@ -393,10 +414,18 @@ private static Integer asInteger(Object val, JDBCType columnType) throws SQLExce
393414
case FLOAT:
394415
case DOUBLE:
395416
return safeToInt(safeToLong(((Number) val).doubleValue()));
417+
case VARCHAR:
418+
try {
419+
return Integer.valueOf((String) val);
420+
} catch (NumberFormatException e) {
421+
throw new SQLException(
422+
format(Locale.ROOT, "Unable to convert value [%.128s] of type [VARCHAR] to an Integer", val), e);
423+
}
396424
default:
397425
}
398426

399-
throw new SQLException("Conversion from type [" + columnType + "] to [Integer] not supported");
427+
throw new SQLException(
428+
format(Locale.ROOT, "Unable to convert value [%.128s] of type [%s] to an Integer", val, columnType.getName()));
400429
}
401430

402431
private static Long asLong(Object val, JDBCType columnType) throws SQLException {
@@ -412,12 +441,21 @@ private static Long asLong(Object val, JDBCType columnType) throws SQLException
412441
case FLOAT:
413442
case DOUBLE:
414443
return safeToLong(((Number) val).doubleValue());
415-
case TIMESTAMP:
416-
return ((Number) val).longValue();
444+
//TODO: should we support conversion to TIMESTAMP?
445+
//The spec says that getLong() should support the following types conversions:
446+
//TINYINT, SMALLINT, INTEGER, BIGINT, REAL, FLOAT, DOUBLE, DECIMAL, NUMERIC, BIT, BOOLEAN, CHAR, VARCHAR, LONGVARCHAR
447+
//case TIMESTAMP:
448+
// return ((Number) val).longValue();
449+
case VARCHAR:
450+
try {
451+
return Long.valueOf((String) val);
452+
} catch (NumberFormatException e) {
453+
throw new SQLException(format(Locale.ROOT, "Unable to convert value [%.128s] of type [VARCHAR] to a Long", val), e);
454+
}
417455
default:
418456
}
419457

420-
throw new SQLException("Conversion from type [" + columnType + "] to [Long] not supported");
458+
throw new SQLException(format(Locale.ROOT, "Unable to convert value [%.128s] of type [%s] to a Long", val, columnType.getName()));
421459
}
422460

423461
private static Float asFloat(Object val, JDBCType columnType) throws SQLException {
@@ -433,10 +471,16 @@ private static Float asFloat(Object val, JDBCType columnType) throws SQLExceptio
433471
case FLOAT:
434472
case DOUBLE:
435473
return Float.valueOf((((float) ((Number) val).doubleValue())));
474+
case VARCHAR:
475+
try {
476+
return Float.valueOf((String) val);
477+
} catch (NumberFormatException e) {
478+
throw new SQLException(format(Locale.ROOT, "Unable to convert value [%.128s] of type [VARCHAR] to a Float", val), e);
479+
}
436480
default:
437481
}
438482

439-
throw new SQLException("Conversion from type [" + columnType + "] to [Float] not supported");
483+
throw new SQLException(format(Locale.ROOT, "Unable to convert value [%.128s] of type [%s] to a Float", val, columnType.getName()));
440484
}
441485

442486
private static Double asDouble(Object val, JDBCType columnType) throws SQLException {
@@ -451,32 +495,41 @@ private static Double asDouble(Object val, JDBCType columnType) throws SQLExcept
451495
case REAL:
452496
case FLOAT:
453497
case DOUBLE:
498+
454499
return Double.valueOf(((Number) val).doubleValue());
500+
case VARCHAR:
501+
try {
502+
return Double.valueOf((String) val);
503+
} catch (NumberFormatException e) {
504+
throw new SQLException(format(Locale.ROOT, "Unable to convert value [%.128s] of type [VARCHAR] to a Double", val), e);
505+
}
455506
default:
456507
}
457508

458-
throw new SQLException("Conversion from type [" + columnType + "] to [Double] not supported");
509+
throw new SQLException(
510+
format(Locale.ROOT, "Unable to convert value [%.128s] of type [%s] to a Double", val, columnType.getName()));
459511
}
460512

461513
private static Date asDate(Object val, JDBCType columnType) throws SQLException {
462514
if (columnType == JDBCType.TIMESTAMP) {
463515
return new Date(utcMillisRemoveTime(((Number) val).longValue()));
464516
}
465-
throw new SQLException("Conversion from type [" + columnType + "] to [Date] not supported");
517+
throw new SQLException(format(Locale.ROOT, "Unable to convert value [%.128s] of type [%s] to a Date", val, columnType.getName()));
466518
}
467519

468520
private static Time asTime(Object val, JDBCType columnType) throws SQLException {
469521
if (columnType == JDBCType.TIMESTAMP) {
470522
return new Time(utcMillisRemoveDate(((Number) val).longValue()));
471523
}
472-
throw new SQLException("Conversion from type [" + columnType + "] to [Time] not supported");
524+
throw new SQLException(format(Locale.ROOT, "Unable to convert value [%.128s] of type [%s] to a Time", val, columnType.getName()));
473525
}
474526

475527
private static Timestamp asTimestamp(Object val, JDBCType columnType) throws SQLException {
476528
if (columnType == JDBCType.TIMESTAMP) {
477529
return new Timestamp(((Number) val).longValue());
478530
}
479-
throw new SQLException("Conversion from type [" + columnType + "] to [Timestamp] not supported");
531+
throw new SQLException(
532+
format(Locale.ROOT, "Unable to convert value [%.128s] of type [%s] to a Timestamp", val, columnType.getName()));
480533
}
481534

482535
private static byte[] asByteArray(Object val, JDBCType columnType) {

0 commit comments

Comments
 (0)