From 6cc8b68491d7f2c1b30d622888ed75ed880edcf4 Mon Sep 17 00:00:00 2001 From: Grant Nicholas Date: Tue, 30 Nov 2021 15:00:19 -0600 Subject: [PATCH] 3.1 driver broke float handling for nan/inf ResultSet.getFloat and ResultSet.getObject(Float.class) did not agree with each other --- .../parser/ClickHouseFloatParser.java | 58 +++++++++++ .../parser/ClickHouseValueParser.java | 9 +- .../response/ClickHouseResultSetTest.java | 55 +++++++++++ .../parser/ClickHouseValueParserTest.java | 97 ++++++++++++++++++- 4 files changed, 212 insertions(+), 7 deletions(-) create mode 100644 clickhouse-jdbc/src/main/java/ru/yandex/clickhouse/response/parser/ClickHouseFloatParser.java diff --git a/clickhouse-jdbc/src/main/java/ru/yandex/clickhouse/response/parser/ClickHouseFloatParser.java b/clickhouse-jdbc/src/main/java/ru/yandex/clickhouse/response/parser/ClickHouseFloatParser.java new file mode 100644 index 000000000..d8a623f49 --- /dev/null +++ b/clickhouse-jdbc/src/main/java/ru/yandex/clickhouse/response/parser/ClickHouseFloatParser.java @@ -0,0 +1,58 @@ +package ru.yandex.clickhouse.response.parser; + +import java.sql.SQLException; +import java.util.TimeZone; + +import ru.yandex.clickhouse.except.ClickHouseUnknownException; +import ru.yandex.clickhouse.response.ByteFragment; +import ru.yandex.clickhouse.response.ClickHouseColumnInfo; + +final class ClickHouseFloatParser extends ClickHouseValueParser { + + private static ClickHouseFloatParser instance; + + static ClickHouseFloatParser getInstance() { + if (instance == null) { + instance = new ClickHouseFloatParser(); + } + return instance; + } + + private ClickHouseFloatParser() { + // prevent instantiation + } + + @Override + public Float parse(ByteFragment value, ClickHouseColumnInfo columnInfo, + TimeZone resultTimeZone) throws SQLException + { + if (value.isNull()) { + return null; + } + if (value.isNaN()) { + return Float.valueOf(Float.NaN); + } + String s = value.asString(); + switch (s) { + case "+inf": + case "inf": + return Float.valueOf(Float.POSITIVE_INFINITY); + case "-inf": + return Float.valueOf(Float.NEGATIVE_INFINITY); + default: + try { + return Float.valueOf(s); + } catch (NumberFormatException nfe) { + throw new ClickHouseUnknownException( + "Error parsing '" + s + "' as Float", + nfe); + } + } + } + + @Override + protected Float getDefaultValue() { + return Float.valueOf(0); + } + +} diff --git a/clickhouse-jdbc/src/main/java/ru/yandex/clickhouse/response/parser/ClickHouseValueParser.java b/clickhouse-jdbc/src/main/java/ru/yandex/clickhouse/response/parser/ClickHouseValueParser.java index 03ad23a53..1294d2beb 100644 --- a/clickhouse-jdbc/src/main/java/ru/yandex/clickhouse/response/parser/ClickHouseValueParser.java +++ b/clickhouse-jdbc/src/main/java/ru/yandex/clickhouse/response/parser/ClickHouseValueParser.java @@ -41,10 +41,7 @@ public abstract class ClickHouseValueParser { Boolean.FALSE); register(Date.class, ClickHouseSQLDateParser.getInstance()); register(Double.class, ClickHouseDoubleParser.getInstance()); - register(Float.class, - Float::valueOf, - Float.valueOf(0f), - Float.valueOf(Float.NaN)); + register(Float.class, ClickHouseFloatParser.getInstance()); register(Instant.class, ClickHouseInstantParser.getInstance()); register(Integer.class, Integer::decode, Integer.valueOf(0)); register(LocalDate.class, ClickHouseLocalDateParser.getInstance()); @@ -152,8 +149,8 @@ public static final double parseDouble(ByteFragment value, ClickHouseColumnInfo public static final float parseFloat(ByteFragment value, ClickHouseColumnInfo columnInfo) throws SQLException { - Double d = getParser(Double.class).parse(value, columnInfo, null); - return d != null ? d.floatValue() : 0.0f; + Float f = getParser(Float.class).parse(value, columnInfo, null); + return f != null ? f.floatValue() : 0.0f; } /** diff --git a/clickhouse-jdbc/src/test/java/ru/yandex/clickhouse/response/ClickHouseResultSetTest.java b/clickhouse-jdbc/src/test/java/ru/yandex/clickhouse/response/ClickHouseResultSetTest.java index e760946ff..cbf94f7ec 100644 --- a/clickhouse-jdbc/src/test/java/ru/yandex/clickhouse/response/ClickHouseResultSetTest.java +++ b/clickhouse-jdbc/src/test/java/ru/yandex/clickhouse/response/ClickHouseResultSetTest.java @@ -167,6 +167,7 @@ public void withTotals() throws Exception { assertEquals(70511139L, rs.getLong(2)); } + @Test public void withTotalsAndEmptyStrings() throws Exception { String response = "SiteName\tCountry\n" + "String\tString\n" + @@ -266,6 +267,60 @@ public void withTotalsSingleIntColumn() throws Exception { assertEquals(0L, rs.getLong(1)); } + @Test + public void withTotalsSingleFloatColumn() throws Exception { + String response = + "Code\n" + + "Float32\n" + + "1.0\n" + + "NaN\n" + + "nan\n" + + "Infinity\n" + + "+Infinity\n" + + "-Infinity\n" + + "inf\n" + + "+inf\n" + + "-inf\n" + + "\n" // with totals separator row + + "0"; // with totals values row + + ByteArrayInputStream is = new ByteArrayInputStream(response.getBytes("UTF-8")); + + ClickHouseResultSet rs = buildResultSet(is, 1024, "db", "table", true, null, null, props); + + rs.next(); + assertEquals(1f, rs.getObject(1)); + + rs.next(); + assertTrue(Float.isNaN((Float) rs.getObject(1))); + + rs.next(); + assertTrue(Float.isNaN((Float) rs.getObject(1))); + + rs.next(); + assertEquals(Float.POSITIVE_INFINITY, rs.getObject(1)); + + rs.next(); + assertEquals(Float.POSITIVE_INFINITY, rs.getObject(1)); + + rs.next(); + assertEquals(Float.NEGATIVE_INFINITY, rs.getObject(1)); + + rs.next(); + assertEquals(Float.POSITIVE_INFINITY, rs.getObject(1)); + + rs.next(); + assertEquals(Float.POSITIVE_INFINITY, rs.getObject(1)); + + rs.next(); + assertEquals(Float.NEGATIVE_INFINITY, rs.getObject(1)); + + assertFalse(rs.next()); + + rs.getTotals(); + assertEquals(0L, rs.getLong(1)); + } + @Test public void withTotalsSingleNullableColumn() throws Exception { String response = diff --git a/clickhouse-jdbc/src/test/java/ru/yandex/clickhouse/response/parser/ClickHouseValueParserTest.java b/clickhouse-jdbc/src/test/java/ru/yandex/clickhouse/response/parser/ClickHouseValueParserTest.java index 3ff78f943..44d5c174c 100644 --- a/clickhouse-jdbc/src/test/java/ru/yandex/clickhouse/response/parser/ClickHouseValueParserTest.java +++ b/clickhouse-jdbc/src/test/java/ru/yandex/clickhouse/response/parser/ClickHouseValueParserTest.java @@ -1,7 +1,7 @@ package ru.yandex.clickhouse.response.parser; import java.sql.SQLException; - +import org.testng.annotations.DataProvider; import org.testng.annotations.Test; import ru.yandex.clickhouse.response.ByteFragment; @@ -13,6 +13,40 @@ import static org.testng.Assert.fail; public class ClickHouseValueParserTest { + /** + * Generates test data for floats. + */ + @DataProvider(name = "float_test_data") + public Object[][] floatTestData() { + return new Object [][] { + {"100.0", 100.0f}, + {"NaN", Float.NaN}, + {"Infinity", Float.POSITIVE_INFINITY}, + {"+Infinity", Float.POSITIVE_INFINITY}, + {"-Infinity", Float.NEGATIVE_INFINITY}, + {"nan", Float.NaN}, + {"inf", Float.POSITIVE_INFINITY}, + {"+inf", Float.POSITIVE_INFINITY}, + {"-inf", Float.NEGATIVE_INFINITY} + }; + } + + /** + * Generates test data for doubles. + */ + @DataProvider(name = "double_test_data") + public Object[][] doubleTestData() { + return new Object [][] { + {"100.0", 100.0d}, + {"Infinity", Double.POSITIVE_INFINITY}, + {"+Infinity", Double.POSITIVE_INFINITY}, + {"-Infinity", Double.NEGATIVE_INFINITY}, + {"nan", Double.NaN}, + {"inf", Double.POSITIVE_INFINITY}, + {"+inf", Double.POSITIVE_INFINITY}, + {"-inf", Double.NEGATIVE_INFINITY} + }; + } @Test public void testParseInt() throws Exception { @@ -161,4 +195,65 @@ public void testParseBoolean() throws SQLException { assertFalse(ClickHouseValueParser.parseBoolean(ByteFragment.fromString(" true"), columnInfo)); } + @Test (dataProvider = "float_test_data") + public void testParseFloat(String byteFragmentString, Float expectedValue) throws SQLException { + ClickHouseColumnInfo columnInfo = ClickHouseColumnInfo.parse("Float32", "columnName", null); + float floatDelta = 0.001f; + if (expectedValue.isNaN()) { + assertTrue(Float.isNaN(ClickHouseValueParser.parseFloat( + ByteFragment.fromString(byteFragmentString), columnInfo) + )); + } else { + assertEquals(ClickHouseValueParser.parseFloat( + ByteFragment.fromString(byteFragmentString), columnInfo), expectedValue, floatDelta + ); + } + } + + @Test (dataProvider = "double_test_data") + public void testParseDouble(String byteFragmentString, Double expectedValue) throws SQLException { + ClickHouseColumnInfo columnInfo = ClickHouseColumnInfo.parse("Float64", "columnName", null); + double doubleDelta = 0.001; + if (expectedValue.isNaN()) { + assertTrue(Double.isNaN(ClickHouseValueParser.parseDouble( + ByteFragment.fromString(byteFragmentString), columnInfo) + )); + } else { + assertEquals(ClickHouseValueParser.parseDouble( + ByteFragment.fromString(byteFragmentString), columnInfo), expectedValue, doubleDelta + ); + } + } + + @Test (dataProvider = "float_test_data") + public void testGetParserFloat(String byteFragmentString, Float expectedValue) throws SQLException { + ClickHouseColumnInfo columnInfo = ClickHouseColumnInfo.parse("Float32", "columnName", null); + float floatDelta = 0.001f; + + if (expectedValue.isNaN()) { + assertTrue(Float.isNaN(ClickHouseValueParser.getParser(Float.class).parse( + ByteFragment.fromString(byteFragmentString), columnInfo, null) + )); + } else { + assertEquals(ClickHouseValueParser.getParser(Float.class).parse( + ByteFragment.fromString(byteFragmentString), columnInfo, null), expectedValue, floatDelta + ); + } + } + + @Test (dataProvider = "double_test_data") + public void testGetParserDouble(String byteFragmentString, Double expectedValue) throws SQLException { + ClickHouseColumnInfo columnInfo = ClickHouseColumnInfo.parse("Float64", "columnName", null); + double doubleDelta = 0.001d; + + if (expectedValue.isNaN()) { + assertTrue(Double.isNaN(ClickHouseValueParser.getParser(Double.class).parse( + ByteFragment.fromString(byteFragmentString), columnInfo, null) + )); + } else { + assertEquals(ClickHouseValueParser.getParser(Double.class).parse( + ByteFragment.fromString(byteFragmentString), columnInfo, null), expectedValue, doubleDelta + ); + } + } }