Skip to content

Commit

Permalink
SQL: test coverage for JdbcResultSet (#32813)
Browse files Browse the repository at this point in the history 
* 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
  • Loading branch information
@astefan
astefan committed Aug 31, 2018
1 parent cd378d5 commit 00b241a
Show file tree
Hide file tree
Showing 6 changed files with 1,624 additions and 111 deletions.
65 changes: 22 additions & 43 deletions ...ck/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/jdbc/JdbcResultSet.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -133,72 +133,37 @@ public String getString(int columnIndex) throws SQLException {

@Override
public boolean getBoolean(int columnIndex) throws SQLException {
Object val = column(columnIndex);
try {
return val != null ? (Boolean) val : false;
} catch (ClassCastException cce) {
throw new SQLException("unable to convert column " + columnIndex + " to a boolean", cce);
}
return column(columnIndex) != null ? getObject(columnIndex, Boolean.class) : false;
}

@Override
public byte getByte(int columnIndex) throws SQLException {
Object val = column(columnIndex);
try {
return val != null ? ((Number) val).byteValue() : 0;
} catch (ClassCastException cce) {
throw new SQLException("unable to convert column " + columnIndex + " to a byte", cce);
}
return column(columnIndex) != null ? getObject(columnIndex, Byte.class) : 0;
}

@Override
public short getShort(int columnIndex) throws SQLException {
Object val = column(columnIndex);
try {
return val != null ? ((Number) val).shortValue() : 0;
} catch (ClassCastException cce) {
throw new SQLException("unable to convert column " + columnIndex + " to a short", cce);
}
return column(columnIndex) != null ? getObject(columnIndex, Short.class) : 0;
}

@Override
public int getInt(int columnIndex) throws SQLException {
Object val = column(columnIndex);
try {
return val != null ? ((Number) val).intValue() : 0;
} catch (ClassCastException cce) {
throw new SQLException("unable to convert column " + columnIndex + " to an int", cce);
}
return column(columnIndex) != null ? getObject(columnIndex, Integer.class) : 0;
}

@Override
public long getLong(int columnIndex) throws SQLException {
Object val = column(columnIndex);
try {
return val != null ? ((Number) val).longValue() : 0;
} catch (ClassCastException cce) {
throw new SQLException("unable to convert column " + columnIndex + " to a long", cce);
}
return column(columnIndex) != null ? getObject(columnIndex, Long.class) : 0;
}

@Override
public float getFloat(int columnIndex) throws SQLException {
Object val = column(columnIndex);
try {
return val != null ? ((Number) val).floatValue() : 0;
} catch (ClassCastException cce) {
throw new SQLException("unable to convert column " + columnIndex + " to a float", cce);
}
return column(columnIndex) != null ? getObject(columnIndex, Float.class) : 0;
}

@Override
public double getDouble(int columnIndex) throws SQLException {
Object val = column(columnIndex);
try {
return val != null ? ((Number) val).doubleValue() : 0;
} catch (ClassCastException cce) {
throw new SQLException("unable to convert column " + columnIndex + " to a double", cce);
}
return column(columnIndex) != null ? getObject(columnIndex, Double.class) : 0;
}

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

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

private Long dateTime(int columnIndex) throws SQLException {
Object val = column(columnIndex);
JDBCType type = cursor.columns().get(columnIndex - 1).type;
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
// TIMESTAMP. Should we consider the VARCHAR conversion as a later enhancement?
if (JDBCType.TIMESTAMP.equals(type)) {
// 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
return val == null ? null : ((Number) val).longValue();
};
return val == null ? null : (Long) val;
} catch (ClassCastException cce) {
throw new SQLException("unable to convert column " + columnIndex + " to a long", cce);
throw new SQLException(
format(Locale.ROOT, "Unable to convert value [%.128s] of type [%s] to a Long", val, type.getName()), cce);
}
}

Expand Down
87 changes: 70 additions & 17 deletions ...ck/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/jdbc/TypeConverter.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@

import java.sql.Date;
import java.sql.JDBCType;
import java.sql.SQLDataException;
import java.sql.SQLException;
import java.sql.SQLFeatureNotSupportedException;
import java.sql.Time;
Expand Down Expand Up @@ -56,9 +55,10 @@ private TypeConverter() {

}

private static final long DAY_IN_MILLIS = 60 * 60 * 24;
private static final long DAY_IN_MILLIS = 60 * 60 * 24 * 1000;
private static final Map<Class<?>, JDBCType> javaToJDBC;


static {
Map<Class<?>, JDBCType> aMap = Arrays.stream(DataType.values())
.filter(dataType -> dataType.javaClass() != null
Expand Down Expand Up @@ -120,6 +120,7 @@ private static <T> T dateTimeConvert(Long millis, Calendar c, Function<Calendar,
}
}


static long convertFromCalendarToUTC(long value, Calendar cal) {
if (cal == null) {
return value;
Expand All @@ -143,11 +144,15 @@ static <T> T convert(Object val, JDBCType columnType, Class<T> type) throws SQLE
return (T) convert(val, columnType);
}

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

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

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

}
}
Expand All @@ -355,10 +364,16 @@ private static Byte asByte(Object val, JDBCType columnType) throws SQLException
case FLOAT:
case DOUBLE:
return safeToByte(safeToLong(((Number) val).doubleValue()));
case VARCHAR:
try {
return Byte.valueOf((String) val);
} catch (NumberFormatException e) {
throw new SQLException(format(Locale.ROOT, "Unable to convert value [%.128s] of type [VARCHAR] to a Byte", val), e);
}
default:
}

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

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

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

private static Integer asInteger(Object val, JDBCType columnType) throws SQLException {
Expand All @@ -393,10 +414,18 @@ private static Integer asInteger(Object val, JDBCType columnType) throws SQLExce
case FLOAT:
case DOUBLE:
return safeToInt(safeToLong(((Number) val).doubleValue()));
case VARCHAR:
try {
return Integer.valueOf((String) val);
} catch (NumberFormatException e) {
throw new SQLException(
format(Locale.ROOT, "Unable to convert value [%.128s] of type [VARCHAR] to an Integer", val), e);
}
default:
}

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

private static Long asLong(Object val, JDBCType columnType) throws SQLException {
Expand All @@ -412,12 +441,21 @@ private static Long asLong(Object val, JDBCType columnType) throws SQLException
case FLOAT:
case DOUBLE:
return safeToLong(((Number) val).doubleValue());
case TIMESTAMP:
return ((Number) val).longValue();
//TODO: should we support conversion to TIMESTAMP?
//The spec says that getLong() should support the following types conversions:
//TINYINT, SMALLINT, INTEGER, BIGINT, REAL, FLOAT, DOUBLE, DECIMAL, NUMERIC, BIT, BOOLEAN, CHAR, VARCHAR, LONGVARCHAR
//case TIMESTAMP:
// return ((Number) val).longValue();
case VARCHAR:
try {
return Long.valueOf((String) val);
} catch (NumberFormatException e) {
throw new SQLException(format(Locale.ROOT, "Unable to convert value [%.128s] of type [VARCHAR] to a Long", val), e);
}
default:
}

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

private static Float asFloat(Object val, JDBCType columnType) throws SQLException {
Expand All @@ -433,10 +471,16 @@ private static Float asFloat(Object val, JDBCType columnType) throws SQLExceptio
case FLOAT:
case DOUBLE:
return Float.valueOf((((float) ((Number) val).doubleValue())));
case VARCHAR:
try {
return Float.valueOf((String) val);
} catch (NumberFormatException e) {
throw new SQLException(format(Locale.ROOT, "Unable to convert value [%.128s] of type [VARCHAR] to a Float", val), e);
}
default:
}

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

private static Double asDouble(Object val, JDBCType columnType) throws SQLException {
Expand All @@ -451,32 +495,41 @@ private static Double asDouble(Object val, JDBCType columnType) throws SQLExcept
case REAL:
case FLOAT:
case DOUBLE:

return Double.valueOf(((Number) val).doubleValue());
case VARCHAR:
try {
return Double.valueOf((String) val);
} catch (NumberFormatException e) {
throw new SQLException(format(Locale.ROOT, "Unable to convert value [%.128s] of type [VARCHAR] to a Double", val), e);
}
default:
}

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

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

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

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

private static byte[] asByteArray(Object val, JDBCType columnType) {
Expand Down
Loading

0 comments on commit 00b241a

Please sign in to comment.