-
Notifications
You must be signed in to change notification settings - Fork 618
[client-v2,jdbc-v2] Fix reading Date/Time values #2727
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
Conversation
…be stored as Integer
…is reduces number of conversions and makes logic simple
… dates are used for simpler reference
Client V2 CoverageCoverage Report
Class Coverage
|
JDBC V2 CoverageCoverage Report
Class Coverage
|
JDBC V1 CoverageCoverage Report
Class Coverage
|
Client V1 CoverageCoverage Report
Class Coverage
|
...rc/main/java/com/clickhouse/client/api/data_formats/internal/AbstractBinaryFormatReader.java
Outdated
Show resolved
Hide resolved
client-v2/src/main/java/com/clickhouse/client/api/data_formats/internal/MapBackedRecord.java
Outdated
Show resolved
Hide resolved
...rc/main/java/com/clickhouse/client/api/data_formats/internal/AbstractBinaryFormatReader.java
Outdated
Show resolved
Hide resolved
...rc/main/java/com/clickhouse/client/api/data_formats/internal/AbstractBinaryFormatReader.java
Outdated
Show resolved
Hide resolved
...rc/main/java/com/clickhouse/client/api/data_formats/internal/AbstractBinaryFormatReader.java
Show resolved
Hide resolved
…ime value and actual time from it
...rc/main/java/com/clickhouse/client/api/data_formats/internal/AbstractBinaryFormatReader.java
Outdated
Show resolved
Hide resolved
...rc/main/java/com/clickhouse/client/api/data_formats/internal/AbstractBinaryFormatReader.java
Show resolved
Hide resolved
...rc/main/java/com/clickhouse/client/api/data_formats/internal/AbstractBinaryFormatReader.java
Show resolved
Hide resolved
...rc/main/java/com/clickhouse/client/api/data_formats/internal/AbstractBinaryFormatReader.java
Outdated
Show resolved
Hide resolved
...rc/main/java/com/clickhouse/client/api/data_formats/internal/AbstractBinaryFormatReader.java
Outdated
Show resolved
Hide resolved
...rc/main/java/com/clickhouse/client/api/data_formats/internal/AbstractBinaryFormatReader.java
Show resolved
Hide resolved
client-v2/src/main/java/com/clickhouse/client/api/data_formats/internal/MapBackedRecord.java
Show resolved
Hide resolved
client-v2/src/main/java/com/clickhouse/client/api/data_formats/internal/MapBackedRecord.java
Show resolved
Hide resolved
...rc/main/java/com/clickhouse/client/api/data_formats/internal/AbstractBinaryFormatReader.java
Outdated
Show resolved
Hide resolved
...rc/main/java/com/clickhouse/client/api/data_formats/internal/AbstractBinaryFormatReader.java
Show resolved
Hide resolved
...rc/main/java/com/clickhouse/client/api/data_formats/internal/AbstractBinaryFormatReader.java
Show resolved
Hide resolved
clickhouse-data/src/main/java/com/clickhouse/data/ClickHouseColumn.java
Outdated
Show resolved
Hide resolved
client-v2/src/main/java/com/clickhouse/client/api/data_formats/internal/MapBackedRecord.java
Show resolved
Hide resolved
client-v2/src/main/java/com/clickhouse/client/api/data_formats/internal/MapBackedRecord.java
Show resolved
Hide resolved
client-v2/src/main/java/com/clickhouse/client/api/data_formats/internal/BinaryStreamReader.java
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Cursor Bugbot has reviewed your changes and found 3 potential issues.
|




Summary
Closes
Checklist
Delete items not relevant to your PR:
Note
Medium Risk
Changes core date/time decoding and conversion paths (including JDBC
ResultSetbehavior) and introduces a breaking change whereDateis no longer exposed asZonedDateTime, so downstream callers may see type/format differences.Overview
Fixes client-side decoding/conversion for
Date/Timetypes (includingDynamic/Variant) and aligns JDBC getters accordingly.Date/Date32are now read asLocalDatewith no timezone adjustment, whileTime/Time64are read as UTC-basedLocalDateTime(and exposed via newgetLocalTime()APIs forLocalTimeaccess).Adds
ClickHouseColumn.getValueDataType()to represent the effective type forSimpleAggregateFunction, and updates binary readers/string conversion logic to branch on that effective type; also improves null-handling and conversion consistency for enums, geo, inet, and arrays/lists. Extensive new/updated integration tests cover time range/precision, timezone invariance for dates, andDynamic/Variantdate-time decoding, plus new JDBC-focused date/time tests (includingDurationmapping for time values).Written by Cursor Bugbot for commit 909bdf3. This will update automatically on new commits. Configure here.