Skip to content

SQL: Add BigDecimal support to JDBC #56015

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.

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

Merged
merged 9 commits into from
May 4, 2020
Merged
Original file line number Diff line number Diff line change
Expand Up @@ -123,7 +123,13 @@ public void setDouble(int parameterIndex, double x) throws SQLException {

@Override
public void setBigDecimal(int parameterIndex, BigDecimal x) throws SQLException {
setObject(parameterIndex, x, Types.BIGINT);
// ES lacks proper BigDecimal support, so this function simply maps a BigDecimal to a double, while verifying that no definition
// is lost (i.e. the original value can be conveyed as a double).
// While long (i.e. BIGINT) has a larger scale (than double), double has the higher precision more appropriate for BigDecimal.
if (x.compareTo(BigDecimal.valueOf(x.doubleValue())) != 0) {
throw new SQLException("BigDecimal value [" + x + "] out of supported double's range.");
}
setDouble(parameterIndex, x.doubleValue());
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,6 +5,8 @@
*/
package org.elasticsearch.xpack.sql.jdbc;

import org.elasticsearch.common.SuppressForbidden;

import java.io.InputStream;
import java.io.Reader;
import java.math.BigDecimal;
Expand Down Expand Up @@ -300,7 +302,6 @@ private Date asDate(int columnIndex) throws SQLException {
}

try {

return JdbcDateUtils.asDate(val.toString());
} catch (Exception e) {
throw new SQLException(
Expand Down Expand Up @@ -525,7 +526,11 @@ public boolean isClosed() {
@Override
@Deprecated
public BigDecimal getBigDecimal(int columnIndex, int scale) throws SQLException {
throw new SQLFeatureNotSupportedException("BigDecimal not supported");
BigDecimal bd = getBigDecimal(columnIndex);
// The API doesn't allow for specifying a rounding behavior, although BigDecimals did have a way of controlling rounding, even
// before the API got deprecated => default to fail if scaling can't return an exactly equal value, since this behavior was
// expected by (old) callers as well.
return bd == null ? null : bd.setScale(scale);
}

@Override
Expand All @@ -546,8 +551,9 @@ public InputStream getBinaryStream(int columnIndex) throws SQLException {

@Override
@Deprecated
@SuppressForbidden(reason="implementing deprecated method")
public BigDecimal getBigDecimal(String columnLabel, int scale) throws SQLException {
throw new SQLFeatureNotSupportedException("BigDecimal not supported");
return getBigDecimal(column(columnLabel), scale);
}

@Override
Expand Down Expand Up @@ -594,12 +600,12 @@ public Reader getCharacterStream(String columnLabel) throws SQLException {

@Override
public BigDecimal getBigDecimal(int columnIndex) throws SQLException {
throw new SQLFeatureNotSupportedException("BigDecimal not supported");
return convert(columnIndex, BigDecimal.class);
}

@Override
public BigDecimal getBigDecimal(String columnLabel) throws SQLException {
throw new SQLFeatureNotSupportedException("BigDecimal not supported");
return getBigDecimal(column(columnLabel));
}

@Override
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import org.elasticsearch.xpack.sql.proto.StringUtils;

import java.io.IOException;
import java.math.BigDecimal;
import java.sql.Date;
import java.sql.SQLException;
import java.sql.SQLFeatureNotSupportedException;
Expand Down Expand Up @@ -178,6 +179,9 @@ static <T> T convert(Object val, EsType columnType, Class<T> type, String typeSt
if (type == byte[].class) {
return (T) asByteArray(val, columnType, typeString);
}
if (type == BigDecimal.class) {
return (T) asBigDecimal(val, columnType, typeString);
}
//
// JDK 8 types
//
Expand Down Expand Up @@ -537,6 +541,36 @@ private static byte[] asByteArray(Object val, EsType columnType, String typeStri
throw new SQLFeatureNotSupportedException();
}

private static BigDecimal asBigDecimal(Object val, EsType columnType, String typeString) throws SQLException {
switch (columnType) {
case BOOLEAN:
return (Boolean) val ? BigDecimal.ONE : BigDecimal.ZERO;
case BYTE:
case SHORT:
case INTEGER:
case LONG:
return BigDecimal.valueOf(((Number) val).longValue());
case FLOAT:
case HALF_FLOAT:
// floats are passed in as doubles here, so we need to dip into string to keep original float's (reduced) precision.
return new BigDecimal(String.valueOf(((Number) val).floatValue()));
case DOUBLE:
case SCALED_FLOAT:
return BigDecimal.valueOf(((Number) val).doubleValue());
case KEYWORD:
case TEXT:
case CONSTANT_KEYWORD:
try {
return new BigDecimal((String) val);
} catch (NumberFormatException nfe) {
return failConversion(val, columnType, typeString, BigDecimal.class, nfe);
}
// TODO: should we implement numeric - interval types conversions too; ever needed? ODBC does mandate it
// https://docs.microsoft.com/en-us/sql/odbc/reference/appendixes/converting-data-from-c-to-sql-data-types
}
return failConversion(val, columnType, typeString, BigDecimal.class);
}

private static LocalDate asLocalDate(Object val, EsType columnType, String typeString) throws SQLException {
throw new SQLFeatureNotSupportedException();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@

import org.elasticsearch.common.collect.Tuple;

import java.math.BigDecimal;
import java.sql.Connection;
import java.sql.JDBCType;
import java.sql.ParameterMetaData;
Expand All @@ -16,16 +17,12 @@
import java.sql.SQLException;
import java.sql.SQLSyntaxErrorException;

import static org.hamcrest.Matchers.equalTo;
import static org.hamcrest.Matchers.startsWith;

public class PreparedStatementTestCase extends JdbcIntegrationTestCase {

public void testSupportedTypes() throws Exception {
index("library", builder -> {
builder.field("name", "Don Quixote");
builder.field("page_count", 1072);
});

Comment on lines -24 to -28
Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've removed an index involvement mostly due to #56013. Using an index for this test doesn't seem to bring any particular value, but can revert if I'm missing anything.

String stringVal = randomAlphaOfLength(randomIntBetween(0, 1000));
int intVal = randomInt();
long longVal = randomLong();
Expand All @@ -34,10 +31,11 @@ public void testSupportedTypes() throws Exception {
boolean booleanVal = randomBoolean();
byte byteVal = randomByte();
short shortVal = randomShort();
BigDecimal bigDecimalVal = BigDecimal.valueOf(randomDouble());

try (Connection connection = esJdbc()) {
try (PreparedStatement statement = connection.prepareStatement(
"SELECT ?, ?, ?, ?, ?, ?, ?, ?, ?, name FROM library WHERE page_count=?")) {
"SELECT ?, ?, ?, ?, ?, ?, ?, ?, ?, ?")) {
statement.setString(1, stringVal);
statement.setInt(2, intVal);
statement.setLong(3, longVal);
Expand All @@ -47,7 +45,7 @@ public void testSupportedTypes() throws Exception {
statement.setBoolean(7, booleanVal);
statement.setByte(8, byteVal);
statement.setShort(9, shortVal);
statement.setInt(10, 1072);
statement.setBigDecimal(10, bigDecimalVal);

try (ResultSet results = statement.executeQuery()) {
ResultSetMetaData resultSetMetaData = results.getMetaData();
Expand All @@ -67,13 +65,23 @@ public void testSupportedTypes() throws Exception {
assertEquals(booleanVal, results.getBoolean(7));
assertEquals(byteVal, results.getByte(8));
assertEquals(shortVal, results.getShort(9));
assertEquals("Don Quixote", results.getString(10));
assertEquals(bigDecimalVal, results.getBigDecimal(10));
assertFalse(results.next());
}
}
}
}

public void testOutOfRangeBigDecimal() throws Exception {
try (Connection connection = esJdbc()) {
try (PreparedStatement statement = connection.prepareStatement("SELECT ?")) {
BigDecimal tooLarge = BigDecimal.valueOf(Double.MAX_VALUE).add(BigDecimal.ONE);
SQLException ex = expectThrows(SQLException.class, () -> statement.setBigDecimal(1, tooLarge));
assertThat(ex.getMessage(), equalTo("BigDecimal value [" + tooLarge + "] out of supported double's range."));
}
}
}

public void testUnsupportedParameterUse() throws Exception {
index("library", builder -> {
builder.field("name", "Don Quixote");
Expand Down
Loading