Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SQL: test coverage for JdbcResultSet #32813

Merged
merged 11 commits into from
Aug 31, 2018
Merged

SQL: test coverage for JdbcResultSet #32813

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
deb0b03
Tests and fixes for JdbcResultSet
astefan Jul 27, 2018
34bd90f
Merge remote-tracking branch 'remotes/origin/master' into 32078_fix
astefan Aug 13, 2018
54834c9
Replaced randomMillisSinceEpoch() with randomLong()
astefan Aug 21, 2018
7ce8fd8
Simplified code that can be re-used
astefan Aug 22, 2018
9d802a9
Merge branch 'master' of https://github.com/elastic/elasticsearch int…
astefan Aug 22, 2018
906ca43
Reverted small change
astefan Aug 22, 2018
f14cb8a
Made error messages consistent: they now contain both the type that f…
astefan Aug 28, 2018
493fc85
Merge remote-tracking branch 'remotes/origin/master' into 32078_fix
astefan Aug 28, 2018
c32a997
JdbcResultSet tests are probably not needed for with-security setup
astefan Aug 28, 2018
24ec3bf
Merge branch 'master' of https://github.com/elastic/elasticsearch int…
astefan Aug 31, 2018
6325807
Merge branch 'master' of https://github.com/elastic/elasticsearch int…
astefan Aug 31, 2018
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
65 changes: 22 additions & 43 deletions ...ck/plugin/sql/jdbc/src/main/java/org/elasticsearch/xpack/sql/jdbc/jdbc/JdbcResultSet.java
View file
Open in desktop
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
Open in desktop
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;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

yikes

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