[jdbc-v2] bug fix for UInt32 UInt64 and Added support for Nested Type #2334

gfunc · 2025-04-24T06:02:17Z

Summary

fixed bug for ResultSet.getObject() for type UInt32 UInt64
added support for not flattened Nested type in BinaryStreamReader
~~added support for set session settings and set profile~~

Closes

Checklist

Delete items not relevant to your PR:

Closes [jdbc-v2] Exception from ResultSet.getObject function for Uint32 Uint64 type #2333
Unit and integration tests covering the common scenarios were added

…tened Nested type

Copilot

Pull Request Overview

This pull request addresses a bug fix for the handling of UInt32/UInt64 types and adds support for non‐flattened Nested types.

Updated SQL type mapping for UInt32 and removed the mapping for UInt64 in JdbcUtils to fix type handling issues.
Expanded unit tests in DataTypeTests to cover getObject for various numeric types, including the new nested types.
Added support for Nested types in BinaryStreamReader by introducing the readNested method and a corresponding case in readValue.

Reviewed Changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.

File	Description
jdbc-v2/src/test/java/com/clickhouse/jdbc/DataTypeTests.java	Added getObject tests for integers, decimals, dates, arrays, booleans, and nested types.
jdbc-v2/src/main/java/com/clickhouse/jdbc/internal/JdbcUtils.java	Adjusted type mapping for UInt32 and removed UInt64 mapping for improved type representation.
client-v2/src/main/java/com/clickhouse/client/api/data_formats/internal/BinaryStreamReader.java	Introduced new support for non-flattened Nested types by adding a Nested case in readValue and the readNested method.

Comments suppressed due to low confidence (2)

jdbc-v2/src/main/java/com/clickhouse/jdbc/internal/JdbcUtils.java:47

Changing the mapping for UInt32 to BIGINT appears correct; however, the removal of the UInt64 mapping may cause unexpected behavior if explicit type handling for UInt64 is required. Verify that the fallback or alternative mechanism for processing UInt64 values is intentional and fully tested.

map.put(ClickHouseDataType.UInt32, JDBCType.BIGINT);

client-v2/src/main/java/com/clickhouse/client/api/data_formats/internal/BinaryStreamReader.java:237

Ensure that using convertArray with readNested correctly handles Nested type conversion. Adding additional unit tests for various nested data structures may help verify that this new logic covers all expected scenarios.

case Nested:

gfunc · 2025-04-24T08:01:16Z

Fixing test errors

gfunc · 2025-04-24T14:58:54Z

create table with setting flatten_nested is not supported in ClickHouse version 24.3 and 24.8. Thus the tests for non-flatten Nested type reported that flatten_nested is not known to MergeTree engine, hance the " table does not exists" error.

To avoid this, I used query SET flatten_nested = 0, but the behavior of the create table stayed the same, further I found out it is because of the handling of SET in v2 is quite limited and does not support changing session settings for queries using the same connection.

It is worth mentioning that I change the behavior of USE statement as well. I passed the database assigned by USE statement to connection rather than only keeping it within Statement. Since all the session settings are stored in connection, and to my knowledge USE statement should take effect session wide, it would be reasonable to pass the updated database to connection and treat it as the new default.

Copilot

Pull Request Overview

This PR fixes a bug in ResultSet.getObject() for UInt32/UInt64 types and adds support for nested types in BinaryStreamReader, along with enhancements to session settings and profile handling.

Fixes data type mapping for UInt32 and removes redundant mapping for UInt64.
Adds new tests to validate getObject() behavior for various numeric, date, and nested types.
Introduces support for setting session-level profile and server settings via SQL and HTTP API.

Reviewed Changes

Copilot reviewed 9 out of 9 changed files in this pull request and generated no comments.

Show a summary per file

File	Description
jdbc-v2/src/test/java/com/clickhouse/jdbc/metadata/DatabaseMetaDataTest.java	Updated column definitions and ordering to account for the new UInt128/huge_integer type.
jdbc-v2/src/test/java/com/clickhouse/jdbc/StatementTest.java	Added tests for SET session settings and profile support.
jdbc-v2/src/test/java/com/clickhouse/jdbc/DataTypeTests.java	Extended tests for getObject() with various data types, including high-precision numerics.
jdbc-v2/src/main/java/com/clickhouse/jdbc/internal/JdbcUtils.java	Modified mapping for UInt32 from INTEGER to BIGINT.
jdbc-v2/src/main/java/com/clickhouse/jdbc/StatementImpl.java	Updated SQL parsing logic to handle profile and session settings.
client-v2/src/main/java/com/clickhouse/client/api/internal/HttpAPIClientHelper.java	Added support for sending profile parameter in HTTP requests.
client-v2/src/main/java/com/clickhouse/client/api/data_formats/internal/BinaryStreamReader.java	Extended handling for Nested data types with new readNested implementation.
client-v2/src/main/java/com/clickhouse/client/api/ClientConfigProperties.java	Introduced a new PROFILE property.
client-v2/src/main/java/com/clickhouse/client/api/Client.java	Added setProfile() to update client configuration.

Comments suppressed due to low confidence (3)

jdbc-v2/src/main/java/com/clickhouse/jdbc/StatementImpl.java:380

[nitpick] Consider refactoring the SQL token parsing for SET statements to avoid relying solely on fixed token positions; a more robust approach could prevent potential misinterpretations when SQL syntax varies.

            } else if (JdbcUtils.containsIgnoresCase(tokens, "="){

jdbc-v2/src/main/java/com/clickhouse/jdbc/internal/JdbcUtils.java:47

Ensure that mapping UInt32 to JDBCType.BIGINT is fully consistent with the expected data range and any relevant documentation, as this change may affect how large values are retrieved.

map.put(ClickHouseDataType.UInt32, JDBCType.BIGINT);

client-v2/src/main/java/com/clickhouse/client/api/data_formats/internal/BinaryStreamReader.java:237

Verify that the new 'Nested' case in the readValue method correctly handles type conversion via convertArray(readNested(actualColumn), typeHint) and consider adding explicit checks or logging for type mismatches.

case Nested:

gfunc · 2025-04-24T16:25:41Z

It is worth mentioning that I change the behavior of USE statement as well. I passed the database assigned by USE statement to connection rather than only keeping it within Statement. Since all the session settings are stored in connection, and to my knowledge USE statement should take effect session wide, it would be reasonable to pass the updated database to connection and treat it as the new default.

correction: previous USE statement should be broken because of this code in executeQuery().

clickhouse-java/jdbc-v2/src/main/java/com/clickhouse/jdbc/StatementImpl.java

Line 187 in 5ae9b83

mergedSettings.setDatabase(connection.getSchema());

already fixed and added test to cover executeQuery

chernser · 2025-04-24T20:26:24Z

jdbc-v2/src/main/java/com/clickhouse/jdbc/StatementImpl.java

                    }
                }
+
+            } else if (JdbcUtils.containsIgnoresCase(tokens, "=")){


this will be true for any string containing "="
it may be would work for simple case.
just to note.

btw, there may be a multiline string so next lines should be analyzed, too.

chernser · 2025-04-24T20:28:55Z

jdbc-v2/src/main/java/com/clickhouse/jdbc/StatementImpl.java

+                String key = tokens.get(1);
+                String value = tokens.get(3).replace("'", "");
+                //SET profile
+                if (key.equals("profile")) {


equalsIgnoreCase ?

Settings and profile seem to be case sensitive, and ClickHouse will return error UNKNOWN_SETTING if the key is not recognized

chernser · 2025-04-24T20:34:11Z

Good day, @gfunc !

Would you please explain your fix for the UInt32/UInt64?
We are going to change statement parsing - would you please just check server version in tests to skip them?

Thanks you!

gfunc · 2025-04-25T02:42:58Z

Good day! @chernser

Since the recent releases changed how the v2 driver chooses JDBC type and Java class for ClickHouse data types, it seems it will force the same Java class for the same JDBC type.

My fix for UInt32/Uint64 is simple:

return JDBC Type BigInt and Java type Long for UInt32, therefore skipped the conversion from Long defined BinaryStreamReader to Integer in function JdbcUtils.convert
remove UInt64 from CLICKHOUSE_TO_SQL_TYPE_MAP in JdbcUtils and, like Int128, JDBC Type Other and Java Type BigInteger defined in BinaryStreamReader will be returned directly

As for changes related to the SET statement, I will try to skip the test for non-flattened nested type for ClickHouses version 24.3 and 24.8 for now and maybe raise another PR later.

chernser · 2025-04-25T03:09:40Z

jdbc-v2/src/main/java/com/clickhouse/jdbc/internal/JdbcUtils.java

-        map.put(ClickHouseDataType.UInt32, JDBCType.INTEGER);
+        map.put(ClickHouseDataType.UInt32, JDBCType.BIGINT);
        map.put(ClickHouseDataType.Int64, JDBCType.BIGINT);
-        map.put(ClickHouseDataType.UInt64, JDBCType.BIGINT);


This means that UInt64 is not mapped to anything, right? I think it is not correct.
UInt64 is not Other just because it is represent by BigInteger. This mapping is mainly for metadata.

You are right in some way.
But UInt32 and UInt64 are both OTHER because JDBC type is valid only for signed numbers.
Would you please remove UInt8, UInt16, UInt32 and UInt64 from this mapping? We should return whatever BinaryStreamReader returns.

Thanks!

all Uint had been removed

chernser · 2025-04-25T03:18:29Z

@gfunc
Thank you for the explanation!
But we have this mapping https://clickhouse.com/docs/sql-reference/data-types/int-uint#integer-aliases and we have to keep UInt64 as BIGINT otherwise it will screw type matching.
Give me sometime to think how is it better to handle. Definitely UInt32 in java is Long and UInt64 is BigInteger (as BinaryStreamReader returns)

chernser · 2025-04-25T03:34:19Z

@gfunc
Please see my comments. Thank you for pointing out the problem!

gfunc · 2025-04-25T07:41:18Z

Hi @chernser
I had removed SET related code in this PR and removed all Uint in CLICKHOUSE_TO_SQL_TYPE_MAP .
And workflow ran with only errors relateed to create database error for CH cloud.

gfunc added 3 commits April 24, 2025 05:39

debug UInt32 and UInt64 conversion error and add support for not flat…

4fca682

…tened Nested type

Merge remote-tracking branch 'origin/main' into main

d9c1d5e

Merge remote-tracking branch 'origin/main'

e2d1478

mshustov requested a review from Copilot April 24, 2025 07:40

Copilot AI reviewed Apr 24, 2025

View reviewed changes

chernser requested review from Paultagoras, chernser and mzitnik April 24, 2025 14:19

add support for SET stmt and debug

983a527

gfunc requested a review from Copilot April 24, 2025 15:23

Copilot AI reviewed Apr 24, 2025

View reviewed changes

fix USE statement

8d77e8f

chernser reviewed Apr 24, 2025

View reviewed changes

Merge remote-tracking branch 'origin/main'

b9d1ca0

chernser reviewed Apr 25, 2025

View reviewed changes

gfunc added 2 commits April 25, 2025 04:22

remove SET statement support

c3bec5f

remove UInt from JDBC type mapping

9f09e7d

chernser approved these changes Apr 25, 2025

View reviewed changes

chernser merged commit 478a643 into ClickHouse:main Apr 25, 2025
19 of 23 checks passed

gfunc mentioned this pull request May 8, 2025

change connection schema after USE is executed #2138

Merged

2 tasks

[jdbc-v2] bug fix for UInt32 UInt64 and Added support for Nested Type #2334

[jdbc-v2] bug fix for UInt32 UInt64 and Added support for Nested Type #2334

Uh oh!

Conversation

gfunc commented Apr 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Summary

Checklist

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

gfunc commented Apr 24, 2025

Uh oh!

gfunc commented Apr 24, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

gfunc commented Apr 24, 2025

Uh oh!

chernser Apr 24, 2025

Choose a reason for hiding this comment

Uh oh!

chernser Apr 24, 2025

Choose a reason for hiding this comment

Uh oh!

chernser Apr 24, 2025

Choose a reason for hiding this comment

Uh oh!

gfunc Apr 25, 2025

Choose a reason for hiding this comment

Uh oh!

chernser commented Apr 24, 2025

Uh oh!

gfunc commented Apr 25, 2025

Uh oh!

chernser Apr 25, 2025

Choose a reason for hiding this comment

Uh oh!

chernser Apr 25, 2025

Choose a reason for hiding this comment

Uh oh!

gfunc Apr 25, 2025

Choose a reason for hiding this comment

Uh oh!

chernser commented Apr 25, 2025

Uh oh!

chernser commented Apr 25, 2025

Uh oh!

gfunc commented Apr 25, 2025

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

gfunc commented Apr 24, 2025 •

edited

Loading

gfunc commented Apr 24, 2025 •

edited

Loading