diff --git a/CHANGELOG.md b/CHANGELOG.md index 1e558e56a..db652d6a6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -171,6 +171,12 @@ ### Bug Fixes +- **[client-v2]** Fixed reading a `Nullable` column in the `Native` format failing with `Failed to read block ... + End of stream reached before reading all data`, or returning values of the wrong rows. The reader consumed a null + marker before every value, which is the RowBinary layout. `Native` is columnar: it stores a null map of one byte + per row before the values of the whole column, and a null row still has a placeholder value. The markers were + therefore taken from the value bytes and the column was read out of alignment. The null map is now read as a + block. (https://github.com/ClickHouse/clickhouse-java/issues/3137) - **[client-v2]** Fixed geo columns (`Point`, `Ring`, `LineString`, `MultiPoint`, `Polygon`, `MultiLineString`, `MultiPolygon`) being misread from the `Native` format. The reader decoded a geo column row by row with the RowBinary decoders, while `Native` writes it column-major: a `Point` block came back with its coordinates diff --git a/client-v2/src/main/java/com/clickhouse/client/api/data_formats/NativeFormatReader.java b/client-v2/src/main/java/com/clickhouse/client/api/data_formats/NativeFormatReader.java index 0f17e419b..6f4fe2cf8 100644 --- a/client-v2/src/main/java/com/clickhouse/client/api/data_formats/NativeFormatReader.java +++ b/client-v2/src/main/java/com/clickhouse/client/api/data_formats/NativeFormatReader.java @@ -127,6 +127,29 @@ private boolean readBlock() throws IOException { values.add(binaryStreamReader.readArrayItem(column.getNestedColumns().get(0), len)); prevOffset = offsets[j]; } + } else if (column.isNullable() && !column.isLowCardinality()) { + // Native encodes a Nullable column as a null map of nRows bytes followed by the values of + // all rows; a row marked as null still has a placeholder value in the values section. + // RowBinary, in contrast, prefixes every value with its own marker - that marker is what + // readValue() consumes, so it must not be read here. + values = new ArrayList<>(nRows); + boolean[] nulls = new boolean[nRows]; + for (int j = 0; j < nRows; j++) { + nulls[j] = binaryStreamReader.readByte() == 1; + } + // Nothing (the type of a bare NULL) has no value bytes in RowBinary, but Native still writes + // one placeholder byte per row, so it has to be consumed here. + boolean nothing = column.getDataType() == ClickHouseDataType.Nothing; + for (int j = 0; j < nRows; j++) { + Object value; + if (nothing) { + binaryStreamReader.readByte(); + value = null; + } else { + value = binaryStreamReader.readValueWithoutNullMarker(column); + } + values.add(nulls[j] ? null : value); + } } else { values = new ArrayList<>(nRows); for (int j = 0; j < nRows; j++) { diff --git a/client-v2/src/main/java/com/clickhouse/client/api/data_formats/internal/BinaryStreamReader.java b/client-v2/src/main/java/com/clickhouse/client/api/data_formats/internal/BinaryStreamReader.java index de4504856..018b0aeb6 100644 --- a/client-v2/src/main/java/com/clickhouse/client/api/data_formats/internal/BinaryStreamReader.java +++ b/client-v2/src/main/java/com/clickhouse/client/api/data_formats/internal/BinaryStreamReader.java @@ -117,9 +117,28 @@ public T readValue(ClickHouseColumn column, Class typeHint) throws IOExce return readValue(column, typeHint, binaryStringSupport); } - @SuppressWarnings("unchecked") + /** + * Reads a value of a {@code Nullable} column whose null marker is stored apart from the value. The Native + * format keeps the null flags of a {@code Nullable} column in a columnar null map that precedes the values + * of the whole block, so the marker must not be consumed here; in RowBinary every value carries its own + * marker and {@link #readValue(ClickHouseColumn)} consumes it. + * @param column - column information + * @return value + * @param - target type of the value + * @throws IOException when IO error occurs + */ + public T readValueWithoutNullMarker(ClickHouseColumn column) throws IOException { + return readValue(column, null, binaryStringSupport, false); + } + private T readValue(ClickHouseColumn column, Class typeHint, boolean stringAsBytes) throws IOException { - if (column.isNullable()) { + return readValue(column, typeHint, stringAsBytes, true); + } + + @SuppressWarnings("unchecked") + private T readValue(ClickHouseColumn column, Class typeHint, boolean stringAsBytes, + boolean readNullMarker) throws IOException { + if (readNullMarker && column.isNullable()) { int isNull = readByteOrEOF(input); if (isNull == 1) { // is Null? return null; diff --git a/client-v2/src/test/java/com/clickhouse/client/query/QueryTests.java b/client-v2/src/test/java/com/clickhouse/client/query/QueryTests.java index d26f59764..0610d7a0a 100644 --- a/client-v2/src/test/java/com/clickhouse/client/query/QueryTests.java +++ b/client-v2/src/test/java/com/clickhouse/client/query/QueryTests.java @@ -528,6 +528,75 @@ public void testReadingMultiRowArrays(ClickHouseFormat format, String sql, List< } } + @DataProvider(name = "nullableColumnCases") + Object[][] getNullableColumnCases() { + String nonNullStrings = "SELECT id, val, tag FROM values(" + + "'id UInt32, val Nullable(String), tag Int32', " + + "(1, 'alpha', 100), (2, 'beta', 200), (3, 'gamma', 300)) ORDER BY id"; + String mixedStrings = "SELECT id, val, tag FROM values(" + + "'id UInt32, val Nullable(String), tag Int32', " + + "(1, 'alpha', 100), (2, NULL, 200), (3, 'gamma', 300)) ORDER BY id"; + String allNullStrings = "SELECT id, val, tag FROM values(" + + "'id UInt32, val Nullable(String), tag Int32', " + + "(1, NULL, 100), (2, NULL, 200), (3, NULL, 300)) ORDER BY id"; + String mixedInts = "SELECT id, val, tag FROM values(" + + "'id UInt32, val Nullable(Int32), tag Int32', " + + "(1, 11, 100), (2, NULL, 200), (3, 33, 300)) ORDER BY id"; + String mixedDecimals = "SELECT id, val, tag FROM values(" + + "'id UInt32, val Nullable(Decimal(9, 2)), tag Int32', " + + "(1, 1.25, 100), (2, NULL, 200), (3, 3.75, 300)) ORDER BY id"; + String mixedUuids = "SELECT id, val, tag FROM values(" + + "'id UInt32, val Nullable(UUID), tag Int32', " + + "(1, '00000000-0000-0000-0000-000000000001', 100), (2, NULL, 200), " + + "(3, '00000000-0000-0000-0000-000000000003', 300)) ORDER BY id"; + String bareNulls = "SELECT id, NULL AS val, tag FROM values(" + + "'id UInt32, tag Int32', (1, 100), (2, 200), (3, 300)) ORDER BY id"; + String plainStrings = "SELECT id, val, tag FROM values(" + + "'id UInt32, val String, tag Int32', " + + "(1, 'alpha', 100), (2, '', 200), (3, 'gamma', 300)) ORDER BY id"; + + List strings = Arrays.asList("alpha", "beta", "gamma"); + List mixedStringValues = Arrays.asList("alpha", null, "gamma"); + List nulls = Arrays.asList(null, null, null); + List mixedIntValues = Arrays.asList(11, null, 33); + List mixedDecimalValues = Arrays.asList(new BigDecimal("1.25"), null, new BigDecimal("3.75")); + List mixedUuidValues = Arrays.asList( + UUID.fromString("00000000-0000-0000-0000-000000000001"), null, + UUID.fromString("00000000-0000-0000-0000-000000000003")); + List plainStringValues = Arrays.asList("alpha", "", "gamma"); + + return new Object[][]{ + {ClickHouseFormat.Native, nonNullStrings, strings}, + {ClickHouseFormat.Native, mixedStrings, mixedStringValues}, + {ClickHouseFormat.Native, allNullStrings, nulls}, + {ClickHouseFormat.Native, mixedInts, mixedIntValues}, + {ClickHouseFormat.Native, mixedDecimals, mixedDecimalValues}, + {ClickHouseFormat.Native, mixedUuids, mixedUuidValues}, + {ClickHouseFormat.Native, bareNulls, nulls}, + {ClickHouseFormat.Native, plainStrings, plainStringValues}, + {ClickHouseFormat.RowBinaryWithNamesAndTypes, mixedStrings, mixedStringValues}, + {ClickHouseFormat.RowBinaryWithNamesAndTypes, mixedInts, mixedIntValues}, + {ClickHouseFormat.RowBinaryWithNamesAndTypes, mixedUuids, mixedUuidValues}, + }; + } + + @Test(groups = {"integration"}, dataProvider = "nullableColumnCases") + public void testReadingNullableColumns(ClickHouseFormat format, String sql, List expectedValues) + throws Exception { + QuerySettings settings = new QuerySettings().setFormat(format); + try (QueryResponse response = client.query(sql, settings).get()) { + ClickHouseBinaryFormatReader reader = client.newBinaryFormatReader(response); + for (int i = 0; i < expectedValues.size(); i++) { + Map record = reader.next(); + Assert.assertNotNull(record, "Expected a row at index " + i); + Assert.assertEquals(record.get("id"), (long) (i + 1)); + Assert.assertEquals(record.get("val"), expectedValues.get(i)); + Assert.assertEquals(record.get("tag"), (i + 1) * 100); + } + Assert.assertNull(reader.next()); + } + } + @Test(groups = {"integration"}) public void testBinaryStreamReader() throws Exception { final String table = "dynamic_schema_test_table";