Skip to content
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.

Sign up for GitHub

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

SerializerUtils support OffsetDateTime and Instant #2116

Merged
merged 3 commits into from
Feb 10, 2025
Merged

SerializerUtils support OffsetDateTime and Instant #2116

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
c37aac1
SerializerUtils support OffsetDateTime and Instant
abcfy2 Jan 26, 2025
f5ecac8
add test for new java.time type support
abcfy2 Jan 31, 2025
29f5ffd
fix InsertClientHttpCompressionTests.insertSimplePOJOs test failed
abcfy2 Feb 1, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion clickhouse-data/src/main/java/com/clickhouse/data/ClickHouseUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -362,7 +362,7 @@ public static List<Path> findFiles(String pattern, String... paths) throws IOExc
pattern.chars().anyMatch(
value -> {
if (value < ' ' || reservedCharsWindows.indexOf(value) != -1) {
throw new IllegalArgumentException("File path contains reserved character <%s>".formatted((char) value));
throw new IllegalArgumentException(String.format("File path contains reserved character <%s>", value));
}
return false;
}
Expand Down
1 change: 1 addition & 0 deletions client-v2/src/main/java/com/clickhouse/client/api/Client.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2000,6 +2000,7 @@ private <T> CompletableFuture<T> runAsyncOperation(Supplier<T> resultSupplier, M
return isAsync ? CompletableFuture.supplyAsync(resultSupplier, sharedOperationExecutor) : CompletableFuture.completedFuture(resultSupplier.get());
}

@Override
public String toString() {
return "Client{" +
"endpoints=" + endpoints +
Expand Down
10 changes: 5 additions & 5 deletions ...v2/src/main/java/com/clickhouse/client/api/data_formats/ClickHouseBinaryFormatReader.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,7 @@
import java.math.BigInteger;
import java.net.Inet4Address;
import java.net.Inet6Address;
import java.time.Duration;
import java.time.Instant;
import java.time.LocalDate;
import java.time.LocalDateTime;
import java.time.ZonedDateTime;
import java.time.*;
import java.util.List;
import java.util.Map;
import java.util.UUID;
Expand Down Expand Up @@ -535,6 +531,10 @@ public interface ClickHouseBinaryFormatReader extends AutoCloseable {

LocalDateTime getLocalDateTime(int index);

OffsetDateTime getOffsetDateTime(String colName);

OffsetDateTime getOffsetDateTime(int index);

TableSchema getSchema();

ClickHouseBitmap getClickHouseBitmap(String colName);
Expand Down
18 changes: 18 additions & 0 deletions ...nt-v2/src/main/java/com/clickhouse/client/api/data_formats/RowBinaryFormatSerializer.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@
import java.math.BigInteger;
import java.net.Inet4Address;
import java.net.Inet6Address;
import java.time.Instant;
import java.time.OffsetDateTime;
import java.time.ZoneId;
import java.time.ZonedDateTime;

Expand Down Expand Up @@ -145,6 +147,22 @@ public void writeDateTime64(ZonedDateTime value, int scale, ZoneId targetTz) thr
SerializerUtils.writeDateTime64(out, value, scale, targetTz);
}

public void writeDateTime32(OffsetDateTime value) throws IOException {
SerializerUtils.writeDateTime32(out, value, null);
}

public void writeDateTime64(OffsetDateTime value, int scale) throws IOException {
SerializerUtils.writeDateTime64(out, value, scale, null);
}

public void writeDateTime32(Instant value) throws IOException {
SerializerUtils.writeDateTime32(out, value, null);
}

public void writeDateTime64(Instant value, int scale) throws IOException {
SerializerUtils.writeDateTime64(out, value, scale, null);
}

public void writeEnum8(byte value) throws IOException {
BinaryStreamUtils.writeEnum8(out, value);
}
Expand Down
47 changes: 33 additions & 14 deletions ...main/java/com/clickhouse/client/api/data_formats/internal/AbstractBinaryFormatReader.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,12 +27,7 @@
import java.math.BigInteger;
import java.net.Inet4Address;
import java.net.Inet6Address;
import java.time.Duration;
import java.time.Instant;
import java.time.LocalDate;
import java.time.LocalDateTime;
import java.time.ZoneOffset;
import java.time.ZonedDateTime;
import java.time.*;
import java.time.format.DateTimeFormatter;
import java.time.temporal.ChronoUnit;
import java.util.*;
Expand Down Expand Up @@ -221,7 +216,7 @@ protected void endReached() {

protected void setSchema(TableSchema schema) {
this.schema = schema;
this.columns = schema.getColumns().toArray(new ClickHouseColumn[0]);
this.columns = schema.getColumns().toArray(ClickHouseColumn.EMPTY_ARRAY);
this.convertions = new Map[columns.length];

for (int i = 0; i < columns.length; i++) {
Expand Down Expand Up @@ -279,7 +274,7 @@ public String getString(String colName) {
ClickHouseDataType dataType = schema.getColumnByName(colName).getDataType();
ZonedDateTime zdt = (ZonedDateTime) value;
if (dataType == ClickHouseDataType.Date) {
return zdt.format(com.clickhouse.client.api.DataTypeUtils.DATE_FORMATTER).toString();
return zdt.format(com.clickhouse.client.api.DataTypeUtils.DATE_FORMATTER);
}
return value.toString();
} else {
Expand Down Expand Up @@ -369,11 +364,17 @@ public Instant getInstant(String colName) {
return data.atStartOfDay().toInstant(ZoneOffset.UTC);
case DateTime:
case DateTime64:
LocalDateTime dateTime = readValue(colName);
return dateTime.toInstant(column.getTimeZone().toZoneId().getRules().getOffset(dateTime));

Object colValue = readValue(colName);
if (colValue instanceof LocalDateTime) {
LocalDateTime dateTime = (LocalDateTime) colValue;
return dateTime.toInstant(column.getTimeZone().toZoneId().getRules().getOffset(dateTime));
} else {
ZonedDateTime dateTime = (ZonedDateTime) colValue;
return dateTime.toInstant();
}
default:
throw new ClientException("Column of type " + column.getDataType() + " cannot be converted to Instant");
}
throw new ClientException("Column of type " + column.getDataType() + " cannot be converted to Instant");
}

@Override
Expand All @@ -386,9 +387,9 @@ public ZonedDateTime getZonedDateTime(String colName) {
case Date:
case Date32:
return readValue(colName);
default:
throw new ClientException("Column of type " + column.getDataType() + " cannot be converted to Instant");
}

throw new ClientException("Column of type " + column.getDataType() + " cannot be converted to Instant");
}

@Override
Expand Down Expand Up @@ -725,6 +726,24 @@ public LocalDateTime getLocalDateTime(int index) {
return (LocalDateTime) value;
}

@Override
public OffsetDateTime getOffsetDateTime(String colName) {
Object value = readValue(colName);
if (value instanceof ZonedDateTime) {
return ((ZonedDateTime) value).toOffsetDateTime();
}
return (OffsetDateTime) value;
}

@Override
public OffsetDateTime getOffsetDateTime(int index) {
Object value = readValue(index);
if (value instanceof ZonedDateTime) {
return ((ZonedDateTime) value).toOffsetDateTime();
}
return (OffsetDateTime) value;
}

@Override
public ClickHouseBitmap getClickHouseBitmap(String colName) {
return readValue(colName);
Expand Down
10 changes: 10 additions & 0 deletions ...c/main/java/com/clickhouse/client/api/data_formats/internal/BinaryReaderBackedRecord.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -354,6 +354,16 @@ public LocalDateTime getLocalDateTime(int index) {
return reader.getLocalDateTime(index);
}

@Override
public OffsetDateTime getOffsetDateTime(String colName) {
return reader.getOffsetDateTime(colName);
}

@Override
public OffsetDateTime getOffsetDateTime(int index) {
return reader.getOffsetDateTime(index);
}

@Override
public Object getObject(String colName) {
return reader.readValue(colName);
Expand Down
18 changes: 18 additions & 0 deletions client-v2/src/main/java/com/clickhouse/client/api/data_formats/internal/MapBackedRecord.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -489,6 +489,24 @@ public LocalDateTime getLocalDateTime(int index) {
return (LocalDateTime) value;
}

@Override
public OffsetDateTime getOffsetDateTime(String colName) {
Object value = readValue(colName);
if (value instanceof ZonedDateTime) {
return ((ZonedDateTime) value).toOffsetDateTime();
}
return (OffsetDateTime) value;
}

@Override
public OffsetDateTime getOffsetDateTime(int index) {
Object value = readValue(index);
if (value instanceof ZonedDateTime) {
return ((ZonedDateTime) value).toOffsetDateTime();
}
return (OffsetDateTime) value;
}

@Override
public ClickHouseBitmap getClickHouseBitmap(String colName) {
return readValue(colName);
Expand Down
39 changes: 26 additions & 13 deletions client-v2/src/main/java/com/clickhouse/client/api/data_formats/internal/SerializerUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,7 @@
import java.net.Inet4Address;
import java.net.Inet6Address;
import java.sql.Timestamp;
import java.time.LocalDate;
import java.time.LocalDateTime;
import java.time.ZoneId;
import java.time.ZonedDateTime;
import java.time.*;
import java.util.Arrays;
import java.util.HashMap;
import java.util.HashSet;
Expand Down Expand Up @@ -639,16 +636,22 @@ public static void writeDateTime32(OutputStream output, Object value, ZoneId tar
}

public static void writeDateTime(OutputStream output, Object value, ZoneId targetTz) throws IOException {
long ts = 0;
long ts;
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();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

seems missing .withZoneSameInstante(targetTz) ?

Copy link
Contributor Author

@abcfy2 abcfy2 Jan 31, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

} 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();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should convert to a target timezone.

Copy link
Contributor Author

@abcfy2 abcfy2 Jan 31, 2025 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

} else if (value instanceof Instant) {
Instant dt = (Instant) value;
ts = dt.getEpochSecond();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

what about timezone?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

} else {
throw new IllegalArgumentException("Cannot convert " + value + " to DataTime");
}
Expand All @@ -661,20 +664,30 @@ public static void writeDateTime64(OutputStream output, Object value, int scale,
throw new IllegalArgumentException("Invalid scale value '" + scale + "'");
}

long ts = 0;
long nano = 0;
long ts;
long nano;
if (value instanceof LocalDateTime) {
ZonedDateTime dt = ((LocalDateTime) value).atZone(targetTz);
ts = dt.toEpochSecond();
nano = dt.getNano();
LocalDateTime dt = (LocalDateTime) value;
ZonedDateTime zdt = dt.atZone(targetTz);
ts = zdt.toEpochSecond();
nano = zdt.getNano();
} else if (value instanceof ZonedDateTime) {
ZonedDateTime dt = ((ZonedDateTime) value).withZoneSameInstant(targetTz);
ZonedDateTime dt = (ZonedDateTime) value;
ts = dt.toEpochSecond();
nano = dt.getNano();
} else if (value instanceof Timestamp) {
ZonedDateTime dt = ((Timestamp) value).toLocalDateTime().atZone(targetTz);
Timestamp dt = (Timestamp) value;
ZonedDateTime zdt = dt.toLocalDateTime().atZone(targetTz);
ts = zdt.toEpochSecond();
nano = zdt.getNano();
} else if (value instanceof OffsetDateTime) {
OffsetDateTime dt = (OffsetDateTime) value;
ts = dt.toEpochSecond();
nano = dt.getNano();
} else if (value instanceof Instant) {
Instant dt = (Instant) value;
ts = dt.getEpochSecond();
nano = dt.getNano();
} else {
throw new IllegalArgumentException("Cannot convert " + value + " to DataTime");
}
Expand Down
4 changes: 4 additions & 0 deletions client-v2/src/main/java/com/clickhouse/client/api/query/GenericRecord.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -487,6 +487,10 @@ public interface GenericRecord {

LocalDateTime getLocalDateTime(int index);

OffsetDateTime getOffsetDateTime(String colName);

OffsetDateTime getOffsetDateTime(int index);

Object getObject(String colName);

Object getObject(int index);
Expand Down
12 changes: 9 additions & 3 deletions client-v2/src/test/java/com/clickhouse/client/insert/InsertTests.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -43,6 +43,7 @@
import java.time.Instant;
import java.time.ZoneId;
import java.time.ZonedDateTime;
import java.time.temporal.ChronoField;
import java.util.ArrayList;
import java.util.Arrays;
import java.util.Collections;
Expand Down Expand Up @@ -134,7 +135,6 @@ public void insertSimplePOJOs() throws Exception {
assertEquals(response.getQueryId(), uuid);
}


@Test(groups = { "integration" }, enabled = true)
public void insertPOJOWithJSON() throws Exception {
if (isCloud()) {
Expand Down Expand Up @@ -207,6 +207,12 @@ public void insertPOJOAndReadBack() throws Exception {
Assert.assertEquals(reader.getDouble("float64"), pojo.getFloat64());
Assert.assertEquals(reader.getString("string"), pojo.getString());
Assert.assertEquals(reader.getString("fixedString"), pojo.getFixedString());
Assert.assertTrue(reader.getZonedDateTime("zonedDateTime").isEqual(pojo.getZonedDateTime().withNano(0)));
Assert.assertTrue(reader.getZonedDateTime("zonedDateTime64").isEqual(pojo.getZonedDateTime64()));
Assert.assertTrue(reader.getOffsetDateTime("offsetDateTime").isEqual(pojo.getOffsetDateTime().withNano(0)));
Assert.assertTrue(reader.getOffsetDateTime("offsetDateTime64").isEqual(pojo.getOffsetDateTime64()));
Assert.assertEquals(reader.getInstant("instant"), pojo.getInstant().with(ChronoField.MICRO_OF_SECOND, 0));
Assert.assertEquals(reader.getInstant("instant64"), pojo.getInstant64());
}
}

Expand Down Expand Up @@ -451,12 +457,12 @@ public void testWriter() throws Exception {
if (row[4] == null) {
formatWriter.writeDefault();
} else {
formatWriter.writeString((String) row[4]);
formatWriter.writeDateTime((ZonedDateTime) row[4], null);
}
if (row[5] == null) {
formatWriter.writeDefault();
} else {
formatWriter.writeInt8((byte) row[5]);
formatWriter.writeInt8(((Integer) row[5]).byteValue());
}
}
}, ClickHouseFormat.RowBinaryWithDefaults, new InsertSettings()).get()) {
Expand Down
Loading
Loading