SerializerUtils support OffsetDateTime and Instant #2116

abcfy2 · 2025-01-26T03:08:59Z

Summary

Use cases:

public record MyData (OffsetDateTime ts1, Instant ts2) {
}

client.insert(List.of(new MyData(OffsetDateTime.now(), Instant.now())));

Checklist

Delete items not relevant to your PR:

Closes [client-v2] Support ZonedDateTime and Instant in POJO fields for SerDe #2117
Unit and integration tests covering the common scenarios were added
A human-readable description of the changes was provided to include in CHANGELOG
For significant changes, documentation in https://github.com/ClickHouse/clickhouse-docs was updated with further explanations or tutorials

chernser · 2025-01-27T17:02:00Z

client-v2/src/main/java/com/clickhouse/client/api/data_formats/internal/SerializerUtils.java

-        if (value instanceof LocalDateTime) {
-            LocalDateTime dt = (LocalDateTime) value;
+        long ts;
+        if (value instanceof LocalDateTime dt) {


dt variable is available only in14th or later java. We are keeping our code 1.8 compatible still.
Please revert this back.
Try to compile code with mvn -Dj8 clean install to verify such things.

chernser · 2025-01-27T17:05:53Z

@abcfy2 thank you for the contribution!
Please also extend com.clickhouse.client.query.QueryTests#testDateTimeDataTypes to test new types support
and here is another test for POJO com.clickhouse.client.insert.InsertTests#insertPOJOAndReadBack

chernser · 2025-01-27T17:07:49Z

@abcfy2 would you please run tests locally?
mvn -Dj8 clean verify in the root project
You just need a running docker or rancher

abcfy2 · 2025-01-28T15:45:49Z

Hi @chernser Updated, and fixed a small BUG in ClickHouseUtils.java.

But why I get this output in test. I don't know why:

[WARNING] /home/fengyu/projects/clickhouse-java/clickhouse-data/src/test/java/com/clickhouse/data/value/ClickHouseIntegerValueTest.java:[137,17] redundant cast to int
[INFO] 
[INFO] --- surefire:3.2.5:test (default-test) @ clickhouse-data ---
[INFO] Using auto detected provider org.apache.maven.surefire.testng.TestNGProvider
[INFO] 
[INFO] -------------------------------------------------------
[INFO]  T E S T S
[INFO] -------------------------------------------------------
[INFO] Running TestSuite
[ERROR] Tests run: 1648, Failures: 1, Errors: 0, Skipped: 113, Time elapsed: 22.94 s <<< FAILURE! -- in TestSuite
[ERROR] com.clickhouse.data.UnloadableClassLoaderTest.testLoadAndUnload -- Time elapsed: 0.008 s <<< FAILURE!
java.lang.VerifyError: 
JVMCFRE036 target not instruction in jump bytecode; class=unknown/81860ca7x35c2x408bxb914xb9d355550a9a, method=mapTo(Lcom/clickhouse/data/ClickHouseRecord;Ljava/lang/Class;Ljava/lang/Object;)Ljava/lang/Object;, pc=24
Exception Details:
  Location:
    unknown/81860ca7x35c2x408bxb914xb9d355550a9a.mapTo(Lcom/clickhouse/data/ClickHouseRecord;Ljava/lang/Class;Ljava/lang/Object;)Ljava/lang/Object; @24: JBificmpge
  Reason:
    Target in jump bytecode doesn't exist.
	at java.lang.ClassLoader.defineClassImpl(Native Method)
	at java.lang.ClassLoader.defineClassInternal(ClassLoader.java:399)
	at java.lang.ClassLoader.defineClass(ClassLoader.java:360)
	at java.lang.ClassLoader.defineClass(ClassLoader.java:315)
	at com.clickhouse.data.UnloadableClassLoader.findClass(UnloadableClassLoader.java:84)
	at com.clickhouse.data.UnloadableClassLoader.loadClass(UnloadableClassLoader.java:94)
	at com.clickhouse.data.UnloadableClassLoaderTest.testLoadAndUnload(UnloadableClassLoaderTest.java:102)
	at sun.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
	at sun.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62)
	at sun.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
	at java.lang.reflect.Method.invoke(Method.java:503)
	at org.testng.internal.invokers.MethodInvocationHelper.invokeMethod(MethodInvocationHelper.java:136)
	at org.testng.internal.invokers.TestInvoker.invokeMethod(TestInvoker.java:658)
	at org.testng.internal.invokers.TestInvoker.invokeTestMethod(TestInvoker.java:219)
	at org.testng.internal.invokers.MethodRunner.runInSequence(MethodRunner.java:50)
	at org.testng.internal.invokers.TestInvoker$MethodInvocationAgent.invoke(TestInvoker.java:923)
	at org.testng.internal.invokers.TestInvoker.invokeTestMethods(TestInvoker.java:192)
	at org.testng.internal.invokers.TestMethodWorker.invokeTestMethods(TestMethodWorker.java:146)
	at org.testng.internal.invokers.TestMethodWorker.run(TestMethodWorker.java:128)
	at java.util.ArrayList.forEach(ArrayList.java:1259)
	at org.testng.TestRunner.privateRun(TestRunner.java:808)
	at org.testng.TestRunner.run(TestRunner.java:603)
	at org.testng.SuiteRunner.runTest(SuiteRunner.java:429)
	at org.testng.SuiteRunner.runSequentially(SuiteRunner.java:423)
	at org.testng.SuiteRunner.privateRun(SuiteRunner.java:383)
	at org.testng.SuiteRunner.run(SuiteRunner.java:326)
	at org.testng.SuiteRunnerWorker.runSuite(SuiteRunnerWorker.java:52)
	at org.testng.SuiteRunnerWorker.run(SuiteRunnerWorker.java:95)
	at org.testng.TestNG.runSuitesSequentially(TestNG.java:1249)
	at org.testng.TestNG.runSuitesLocally(TestNG.java:1169)
	at org.testng.TestNG.runSuites(TestNG.java:1092)
	at org.testng.TestNG.run(TestNG.java:1060)
	at org.apache.maven.surefire.testng.TestNGExecutor.run(TestNGExecutor.java:155)
	at org.apache.maven.surefire.testng.TestNGDirectoryTestSuite.executeMulti(TestNGDirectoryTestSuite.java:169)
	at org.apache.maven.surefire.testng.TestNGDirectoryTestSuite.execute(TestNGDirectoryTestSuite.java:88)
	at org.apache.maven.surefire.testng.TestNGProvider.invoke(TestNGProvider.java:137)
	at org.apache.maven.surefire.booter.ForkedBooter.runSuitesInProcess(ForkedBooter.java:385)
	at org.apache.maven.surefire.booter.ForkedBooter.execute(ForkedBooter.java:162)
	at org.apache.maven.surefire.booter.ForkedBooter.run(ForkedBooter.java:507)
	at org.apache.maven.surefire.booter.ForkedBooter.main(ForkedBooter.java:495)

[INFO] 
[INFO] Results:
[INFO] 
[ERROR] Failures: 
[ERROR]   UnloadableClassLoaderTest.testLoadAndUnload:102 » Verify JVMCFRE036 target not instruction in jump bytecode; class=unknown/81860ca7x35c2x408bxb914xb9d355550a9a, method=mapTo(Lcom/clickhouse/data/ClickHouseRecord;Ljava/lang/Class;Ljava/lang/Object;)Ljava/lang/Object;, pc=24
Exception Details:
  Location:
    unknown/81860ca7x35c2x408bxb914xb9d355550a9a.mapTo(Lcom/clickhouse/data/ClickHouseRecord;Ljava/lang/Class;Ljava/lang/Object;)Ljava/lang/Object; @24: JBificmpge
  Reason:
    Target in jump bytecode doesn't exist.
[INFO] 
[ERROR] Tests run: 1648, Failures: 1, Errors: 0, Skipped: 113
[INFO] 
[INFO] ------------------------------------------------------------------------
[INFO] Reactor Summary for clickhouse-java 0.8.0-SNAPSHOT:
[INFO] 
[INFO] clickhouse-java .................................... SUCCESS [  0.650 s]
[INFO] ClickHouse Data Processing Utilities ............... FAILURE [ 28.175 s]
[INFO] ClickHouse Java Client ............................. SKIPPED
[INFO] ClickHouse HTTP Client ............................. SKIPPED
[INFO] ClickHouse Client API .............................. SKIPPED
[INFO] JDBC Driver V2 ..................................... SKIPPED
[INFO] ClickHouse JDBC Driver ............................. SKIPPED
[INFO] ClickHouse R2DBC Driver ............................ SKIPPED
[INFO] ------------------------------------------------------------------------
[INFO] BUILD FAILURE
[INFO] ------------------------------------------------------------------------
[INFO] Total time:  28.971 s
[INFO] Finished at: 2025-01-28T23:45:31+08:00
[INFO] ------------------------------------------------------------------------
[ERROR] Failed to execute goal org.apache.maven.plugins:maven-surefire-plugin:3.2.5:test (default-test) on project clickhouse-data: There are test failures.
[ERROR] 
[ERROR] Please refer to /home/fengyu/projects/clickhouse-java/clickhouse-data/target/surefire-reports for the individual test results.
[ERROR] Please refer to dump files (if any exist) [date].dump, [date]-jvmRun[N].dump and [date].dumpstream.
[ERROR] -> [Help 1]
[ERROR] 
[ERROR] To see the full stack trace of the errors, re-run Maven with the -e switch.
[ERROR] Re-run Maven using the -X switch to enable full debug logging.
[ERROR] 
[ERROR] For more information about the errors and possible solutions, please read the following articles:
[ERROR] [Help 1] http://cwiki.apache.org/confluence/display/MAVEN/MojoFailureException
[ERROR] 
[ERROR] After correcting the problems, you can resume the build with the command
[ERROR]   mvn <args> -rf :clickhouse-data

abcfy2 · 2025-01-28T15:45:59Z

And test will be added later.

chernser · 2025-01-29T07:08:10Z

Thanks, @abcfy2 !
Strange error. What OS and JDK do you use?

chernser · 2025-01-30T22:00:11Z

client-v2/src/main/java/com/clickhouse/client/api/data_formats/internal/SerializerUtils.java

        if (value instanceof LocalDateTime) {
            LocalDateTime dt = (LocalDateTime) value;
            ts = dt.atZone(targetTz).toEpochSecond();
        } else if (value instanceof ZonedDateTime) {
            ZonedDateTime dt = (ZonedDateTime) value;
-            ts = dt.withZoneSameInstant(targetTz).toEpochSecond();
+            ts = dt.toEpochSecond();


seems missing .withZoneSameInstante(targetTz) ?

Hi @chernser withZoneSameInstant is useless, see comments in withZoneSameInstante:

/** * Returns a copy of this date-time with a different time-zone, * retaining the instant. * ..... * /

So this function change the time zone only, not change the real datetime.

We don't really need to use this method, because ZonedDateTime itself contains the time zone, so using it to convert to epoch time is always in line with expectations

A simple test:

ZonedDateTime now = ZonedDateTime.now(); // now --> 2025-01-31T19:14:34.168397146+08:00[Asia/Shanghai] ZonedDateTime changedTimeZone = now.withZoneSameInstant(ZoneId.of("America/Chicago")); // changedTimeZone --> 2025-01-31T05:14:34.168397146-06:00[America/Chicago] System.out.println(now.toEpochSecond()); // -> 1738322074 System.out.println(changedTimeZone.toEpochSecond()); // -> 1738322074

chernser · 2025-01-30T22:00:56Z

client-v2/src/main/java/com/clickhouse/client/api/data_formats/internal/SerializerUtils.java

        } else if (value instanceof Timestamp) {
            Timestamp t = (Timestamp) value;
            ts = t.toLocalDateTime().atZone(targetTz).toEpochSecond();
+        } else if(value instanceof OffsetDateTime) {
+            OffsetDateTime dt = (OffsetDateTime) value;
+            ts = dt.toEpochSecond();


I think we should convert to a target timezone.

Hi @chernser . This is the same reason as above ZonedDateTime, OffsetDateTime contains ZoneOffset, which will point to the corrent datetime or instant all over the world.

Epoch seconds itself implies that the time zone is UTC, so the epoch seconds calculated in any time zone are always the same.

chernser · 2025-01-30T22:01:31Z

client-v2/src/main/java/com/clickhouse/client/api/data_formats/internal/SerializerUtils.java

+            ts = dt.toEpochSecond();
+        } else if (value instanceof Instant) {
+            Instant dt = (Instant) value;
+            ts = dt.getEpochSecond();


what about timezone?

Hi @chernser . Instant also implicitly contains time zone, it always be UTC.

Instant now = Instant.now(); // now --> 2025-01-31T11:21:58.887640603Z ZonedDateTime zdt = now.atZone(ZoneId.systemDefault()); // zdt --> 2025-01-31T19:21:58.887640603+08:00[Asia/Shanghai] // Yes, correct datetime in my time zone

So to summary, as long as the datetime type contains the time zone information (like ZonedDateTime, OffsetDateTime, Instant), we don't need to consider the time zone issue anymore.

This is why so many ORM frameworks (Like mybatis, hibernate) recommends using OffsetDateTime as the mapping type for database datetime.

abcfy2 · 2025-01-31T11:45:54Z

Thanks, @abcfy2 ! Strange error. What OS and JDK do you use?

I find it. Seems the test framework does not support OpenJ9 JVM (I use semeru java before):

$ java -version
openjdk version "21.0.5" 2024-10-15 LTS
IBM Semeru Runtime Open Edition 21.0.5.11 (build 21.0.5+11-LTS)
Eclipse OpenJ9 VM 21.0.5.11 (build openj9-0.48.0, JRE 21 Linux amd64-64-Bit Compressed References 20241015_307 (JIT enabled, AOT enabled)
OpenJ9   - 1d5831436e
OMR      - d10a4d553
JCL      - b1b311c53fe based on jdk-21.0.5+11)

But I switch to use a hotspots JVM (Temurin), it's working:

$ java -version
openjdk version "21.0.6" 2025-01-21 LTS
OpenJDK Runtime Environment Temurin-21.0.6+7 (build 21.0.6+7-LTS)
OpenJDK 64-Bit Server VM Temurin-21.0.6+7 (build 21.0.6+7-LTS, mixed mode, sharing)

abcfy2 · 2025-01-31T13:50:49Z

Hi @chernser . Please review again. The tests have been added.

com.clickhouse.client.query.QueryTests#testDateTimeDataTypes not be extended because this does not test for POJO.

Please review again. Thanks.

mshustov requested a review from chernser January 26, 2025 09:18

chernser reviewed Jan 27, 2025

View reviewed changes

abcfy2 force-pushed the main branch from 09bc662 to a5b77cb Compare January 28, 2025 15:41

chernser reviewed Jan 30, 2025

View reviewed changes

abcfy2 added 2 commits January 31, 2025 21:49

SerializerUtils support OffsetDateTime and Instant

c37aac1

add test for new java.time type support

f5ecac8

abcfy2 force-pushed the main branch from a5b77cb to f5ecac8 Compare January 31, 2025 13:49

mshustov requested a review from chernser January 31, 2025 14:20

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

SerializerUtils support OffsetDateTime and Instant #2116

SerializerUtils support OffsetDateTime and Instant #2116

abcfy2 commented Jan 26, 2025 •

edited by chernser

Loading

chernser Jan 27, 2025

chernser commented Jan 27, 2025

chernser commented Jan 27, 2025

abcfy2 commented Jan 28, 2025

abcfy2 commented Jan 28, 2025

chernser commented Jan 29, 2025 •

edited

Loading

chernser Jan 30, 2025

abcfy2 Jan 31, 2025 •

edited

Loading

chernser Jan 30, 2025

abcfy2 Jan 31, 2025 •

edited

Loading

chernser Jan 30, 2025

abcfy2 Jan 31, 2025

abcfy2 Jan 31, 2025

abcfy2 commented Jan 31, 2025

abcfy2 commented Jan 31, 2025

SerializerUtils support OffsetDateTime and Instant #2116

Are you sure you want to change the base?

SerializerUtils support OffsetDateTime and Instant #2116

Conversation

abcfy2 commented Jan 26, 2025 • edited by chernser Loading

Summary

Checklist

chernser Jan 27, 2025

Choose a reason for hiding this comment

chernser commented Jan 27, 2025

chernser commented Jan 27, 2025

abcfy2 commented Jan 28, 2025

abcfy2 commented Jan 28, 2025

chernser commented Jan 29, 2025 • edited Loading

chernser Jan 30, 2025

Choose a reason for hiding this comment

abcfy2 Jan 31, 2025 • edited Loading

Choose a reason for hiding this comment

chernser Jan 30, 2025

Choose a reason for hiding this comment

abcfy2 Jan 31, 2025 • edited Loading

Choose a reason for hiding this comment

chernser Jan 30, 2025

Choose a reason for hiding this comment

abcfy2 Jan 31, 2025

Choose a reason for hiding this comment

abcfy2 Jan 31, 2025

Choose a reason for hiding this comment

abcfy2 commented Jan 31, 2025

abcfy2 commented Jan 31, 2025

abcfy2 commented Jan 26, 2025 •

edited by chernser

Loading

chernser commented Jan 29, 2025 •

edited

Loading

abcfy2 Jan 31, 2025 •

edited

Loading

abcfy2 Jan 31, 2025 •

edited

Loading