Prepare for JDBC 4.2 #406 #418

enqueue · 2020-01-11T11:28:10Z

Enhance parsing and retrieving of date time values, especially using java.time objects (issue #406 )

Remove support for Java 1.6, add Java 11 checks (this will also allow us to use newer library versions soon, and perhaps get rid of some altogether)
Update maven plugins
Fix some Javadocs to make it work with Java 11 build
Implement "old" methods in PreparedStatement
Support java.time Objects
Breaking change: SQL type FLOAT is strange, Float32 is actually a SQL type REAL according to my understanding. I do not have access to any of the SQL standards, but see for example PostgreSQL manual, MS SQL Server manual, Oracle Datatypes
Deprecate non-JDBC methods
Remove ByteFragmentUtils
Add a test for JDBC Table B-3 conformance
Add a test for JDBC Table B-1 conformance
Added two tables to explain how the PreparedStatement and ResultSet implementations deal with different date time values (see docs/datetime.md).

I am sorry this PR is so large. I tried to create individual commits at least for some specific parts. If you have a good suggestion on how to split the PR, please let me know.

enqueue · 2020-02-25T13:06:51Z

@alex-krash did you have time to take a look at this PR? I am happy to discuss any issues. Have a good day!

alex-krash · 2020-03-03T10:03:48Z

@enqueue , sorry for delay. Will take a look now.
Looks like it's time to start new release (0.3), with breaking Java6 support, and incorporating your changes.

alex-krash · 2020-03-03T10:05:10Z

pom.xml

@@ -4,7 +4,7 @@

    <groupId>ru.yandex.clickhouse</groupId>
    <artifactId>clickhouse-jdbc</artifactId>
-    <version>0.2.4</version>
+    <version>0.2.4-enqueue</version>


Please, avoid changes like this, in order not to pollute master branch.

Sorry, this was a mistake

alex-krash · 2020-03-03T10:18:44Z

src/main/java/ru/yandex/clickhouse/response/parser/ClickHouseDateValueParser.java

+            case UInt32:
+            case UInt64:
+                try {
+                    long l = Long.parseLong(s);


We may have UInt64 overflow here

Why all these things in class ClickHouseDateValueParser? That's really confusing.

Thanks, I had not thought of this. New implementation truncates to milliseconds.

The code should not be confusing, let's see if we can improve it. ClickHouseDateValueParser provides basic functionality for all date/time parsing. This makes the individual implementations much easier to read. We could move the "utility" methods (lines 146 ff.) into the actual implementations. Do you think that this would help overall simplicity?

alex-krash · 2020-03-03T10:19:49Z

src/main/java/ru/yandex/clickhouse/response/parser/ClickHouseLocalTimeParser.java

+    LocalTime parseDate(String value, ClickHouseColumnInfo columnInfo,
+        TimeZone timeZone)
+    {
+        return LocalTime.MIDNIGHT;


Are you sure, that it is ok?

It is the most straightforward response in my opinion. The ClickHouse column is of type Date, e.g. "2020-03-07". If we adjusted midnight server time zone into JVM time zone, we would perhaps cause more confusion.

enqueue · 2020-03-08T16:00:43Z

Hi @alex-krash , thank you for taking the time to review this very large PR ❤️

Moving to Java 8 is an important step that will also help to keep libraries up to date. 0.2 releases containing the state of development as of now will still available for die-hard Java 6 users.

hopsoft · 2020-11-17T00:38:49Z

Any updates on when this PR might get merged and released?

pan3793 · 2020-11-21T09:20:25Z

docs/datetime.md

+For Date and DateTime fields, the JDBC driver has enough time zone related information available, so these methods would only be relevant for String or other typed fields. There might be valid use cases, but for now we think that adding such an option would make things even more complicated.
+
+Requested Type | Number | Date | DateTime | Other 
+---------------| ----------------------------------  


missing |

Thank you @pan3793 ! After almost a year, someone stumbles upon wrongly formatted tables in the documentation... 😃 I added a couple of pipes, and the table looks better now in the preview.

There are so many tricks about timezone, especially in legacy Java Date API. I also encountered some issues when migrated to java.time API from legacy Date API in another ClickHouse JDBC driver implement in native protocol. And would you mind me pick up some code and doc from this PR?

Yes, life would be boring without time zones and leap seconds. Feel free to use the code in this PR, but please let me know when you find any error, so I can fix it. Have a good weekend!

alongls · 2020-12-13T13:10:10Z

Hello
is there any update regarding the driver compatibility to the current release of Metabase ?
thanks.

enqueue · 2020-12-13T15:10:40Z

Hi @alongls

this PR is for improvements in the official ClickHouse JDBC driver. Please follow ClickHouse/metabase-clickhouse-driver#58 for an updated Metabase driver.

We use this branch for the driver, so it would be nice to see the PR reviewed and merged and use the official distribution, but perhaps it's too much work for maintainer due to size or other reasons.

zhicwu · 2021-02-18T14:31:59Z

Thanks @enqueue. This is really a large PR. I quickly walked through the changes today, but I still need a few more days to play with the code(and maybe benchmark too).

github-actions · 2021-02-19T14:30:53Z

🟢 verify using JDK [13] and ClickHouse [21.2]: success

github-actions · 2021-02-19T22:12:37Z

🔴 verify using JDK [15.0.2] and ClickHouse [21]: failure

github-actions · 2021-02-23T22:48:07Z

🟢 benchmark using JDK [8] and ClickHouse [latest]: success

Expand to see details...

Benchmark                                                                 (client)  (statement)   Mode  Cnt         Score         Error   Units
Insertion.insert10kStringRows                                      clickhouse-jdbc       normal  thrpt   20       796.889 ±       7.491   ops/s
Insertion.insert10kStringRows                                      clickhouse-jdbc     prepared  thrpt   20        25.315 ±       2.982   ops/s
Insertion.insert10kTimestampRows                                   clickhouse-jdbc       normal  thrpt   20       251.606 ±       3.919   ops/s
Insertion.insert10kTimestampRows                                   clickhouse-jdbc     prepared  thrpt   20        26.276 ±       2.793   ops/s
Insertion.insert10kUInt64Rows                                      clickhouse-jdbc       normal  thrpt   20       946.238 ±      19.478   ops/s
Insertion.insert10kUInt64Rows                                      clickhouse-jdbc     prepared  thrpt   20        24.932 ±       2.948   ops/s
Query.select10kStringRows                                          clickhouse-jdbc       normal  thrpt   20       457.826 ±      32.503   ops/s
Query.select10kStringRows                                          clickhouse-jdbc     prepared  thrpt   20       443.620 ±      32.391   ops/s
Query.select10kTimestampRows                                       clickhouse-jdbc       normal  thrpt   20        86.249 ±       4.698   ops/s
Query.select10kTimestampRows                                       clickhouse-jdbc     prepared  thrpt   20        90.834 ±       4.697   ops/s
Query.select10kUInt64Rows                                          clickhouse-jdbc       normal  thrpt   20       487.490 ±      21.605   ops/s
Query.select10kUInt64Rows                                          clickhouse-jdbc     prepared  thrpt   20       438.861 ±      31.040   ops/s

zhicwu · 2021-02-24T01:12:50Z

@enqueue, the build break is caused by below code snippets in TimeZoneTest:

Long offset = Long.valueOf(
    TimeUnit.MILLISECONDS.toHours(
        connectionServerTz.getTimeZone().getOffset(currentTime)) - 1);

I'm not sure why we need to deduct one here - it works if I remove that part. Could you take a look?

zhicwu · 2021-02-26T15:25:46Z

I'll take the risk to merge this large PR in order to unblock future development. I'll create separate PR to fix above issues as well as existing ones in ClickHousePreparedStatementTest and those introduced in PR #571.

zhicwu · 2021-02-27T15:45:11Z

@enqueue, I'm not sure if Date is handled properly, but now Time and Timestamp seem good. I think we probably need some more time to compare the tests against clickhouse-client and other drivers. As you may noticed, I removed the hard-coded timezone in pom.xml and enhanced build process to test a few selected timezones. In addition to that, you can now use below command to test more scenarios:

mvn -Duser.timezone=Etc/UTC -DclickhouseTimezone=Europe/Moscow clean verify

Anyway, I'll restructure directories tomorrow and start to add more features in the next following days. If you have time, you may want to review the changes I made in PR #580 and the 3 test cases I temporarily disabled(search @Test(enabled = false)).

enqueue force-pushed the jdbc_4.2 branch from 53cf984 to 309c116 Compare February 4, 2020 19:37

enqueue changed the title ~~Prepare for JDBC 4.2 #406 WIP~~ Prepare for JDBC 4.2 #406 Feb 7, 2020

enqueue requested a review from alex-krash February 7, 2020 13:54

alex-krash suggested changes Mar 3, 2020

View reviewed changes

enqueue mentioned this pull request Oct 9, 2020

Eror build driver not find ru.yandex.clickhouse:clickhouse-jdbc:jar:0.2.4-enqueue ClickHouse/metabase-clickhouse-driver#57

Closed

pan3793 reviewed Nov 21, 2020

View reviewed changes

pan3793 mentioned this pull request Dec 23, 2020

ClickHouseResultSet.getTimestamp(col, calendar) implementation proposal housepower/ClickHouse-Native-JDBC#265

Closed

enqueue mentioned this pull request Jan 8, 2021

Static null parameters for non-batch PreparedStatement #541

Merged

enqueue added 2 commits February 1, 2021 19:19

Prepare for JDBC 4.2 ClickHouse#406

c361379

Rebase on master

9cee6c3

enqueue force-pushed the jdbc_4.2 branch from 20e379f to 9cee6c3 Compare February 1, 2021 18:27

enqueue changed the base branch from master to develop February 1, 2021 18:28

enqueue added 8 commits February 1, 2021 19:30

It is time for Java 1.8!

96ff1b1

fix rebase

619e469

fix tests

568e0e1

Reduce merge conflict potential

4e57663

Merge branch 'develop' into jdbc_4.2

1eb0220

fix compile

853b9ee

Reconcile with develop branch

430deec

Merge branch 'develop' into jdbc_4.2

65f5063

zhicwu merged commit 1e6896a into ClickHouse:develop Feb 26, 2021

This was linked to issues Mar 2, 2021

clickhouse-jdbc not support localDateTime #583

Closed

JDBC 4.2 Support #406

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Prepare for JDBC 4.2 #406 #418

Prepare for JDBC 4.2 #406 #418

enqueue commented Jan 11, 2020 •

edited

Loading

enqueue commented Feb 25, 2020

alex-krash commented Mar 3, 2020

alex-krash Mar 3, 2020

enqueue Mar 8, 2020

alex-krash Mar 3, 2020

enqueue Mar 8, 2020

alex-krash Mar 3, 2020

enqueue Mar 8, 2020

enqueue commented Mar 8, 2020

hopsoft commented Nov 17, 2020

pan3793 Nov 21, 2020

enqueue Nov 21, 2020

pan3793 Nov 21, 2020

enqueue Nov 21, 2020

alongls commented Dec 13, 2020

enqueue commented Dec 13, 2020

zhicwu commented Feb 18, 2021

github-actions bot commented Feb 19, 2021

github-actions bot commented Feb 19, 2021

github-actions bot commented Feb 23, 2021

zhicwu commented Feb 24, 2021

zhicwu commented Feb 26, 2021

zhicwu commented Feb 27, 2021

Prepare for JDBC 4.2 #406 #418

Prepare for JDBC 4.2 #406 #418

Conversation

enqueue commented Jan 11, 2020 • edited Loading

enqueue commented Feb 25, 2020

alex-krash commented Mar 3, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

enqueue commented Mar 8, 2020

hopsoft commented Nov 17, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

alongls commented Dec 13, 2020

enqueue commented Dec 13, 2020

zhicwu commented Feb 18, 2021

github-actions bot commented Feb 19, 2021

github-actions bot commented Feb 19, 2021

github-actions bot commented Feb 23, 2021

zhicwu commented Feb 24, 2021

zhicwu commented Feb 26, 2021

zhicwu commented Feb 27, 2021

enqueue commented Jan 11, 2020 •

edited

Loading