Add read-only timestamp type support for vertica #24264
Conversation
| @@ -118,6 +123,8 @@ public class VerticaClient | |||
| .appendValueReduced(ChronoField.YEAR, 4, 7, 1000) | |||
| .appendPattern("-MM-dd[ G]") | |||
| .toFormatter(); | |||
| private static final DateTimeFormatter FORMATTER_ZERO_SCALE = DateTimeFormatter.ofPattern("yyyy-MM-dd HH:mm:ss"); | |||
| private static final DateTimeFormatter FORMATTER_SIX_SCALE = DateTimeFormatter.ofPattern("yyyy-MM-dd HH:mm:ss.SSSSSS"); | |||
There was a problem hiding this comment.
Is it possible to use optional fraction like OracleClient.TIMESTAMP_NANO_OPTIONAL_FORMATTER?
| @@ -435,4 +444,36 @@ public OptionalInt getMaxColumnNameLength(ConnectorSession session) | |||
| { | |||
| return this.getMaxColumnNameLengthFromDatabaseMetaData(session); | |||
| } | |||
|
|
|||
| private static LongWriteFunction timestampWriteFunction(TimestampType timestampType) | |||
There was a problem hiding this comment.
nit: Move this method under timestampColumnMappingUsingString. We usually put a helper method after the caller.
| @@ -228,6 +235,8 @@ public Optional<ColumnMapping> toColumnMapping(ConnectorSession session, Connect | |||
| DATE, | |||
| (resultSet, index) -> LocalDate.parse(resultSet.getString(index), DATE_READ_FORMATTER).toEpochDay(), | |||
| dateWriteFunctionUsingString())); | |||
| case Types.TIMESTAMP: | |||
| .build(); | ||
|
|
||
| SqlDataTypeTest.create() | ||
| // before epoch |
There was a problem hiding this comment.
Can we add the following roundtrips?
- min and max timestamp values in Vertica https://docs.vertica.com/23.3.x/en/sql-reference/data-types/datetime-data-types/timestamptimestamptz/#limits
- julian->gregorian switch 1582 Oct 5-14
| @@ -228,6 +235,8 @@ public Optional<ColumnMapping> toColumnMapping(ConnectorSession session, Connect | |||
| DATE, | |||
| (resultSet, index) -> LocalDate.parse(resultSet.getString(index), DATE_READ_FORMATTER).toEpochDay(), | |||
| dateWriteFunctionUsingString())); | |||
| case Types.TIMESTAMP: | |||
| return Optional.of(timestampColumnMappingUsingString(createTimestampType(typeHandle.requiredDecimalDigits()))); | |||
There was a problem hiding this comment.
Vertica jdbc driver doesn't differentiate this types and use java.sql.Timestamp
I think we can recognize the type with JdbcTypeHandle#jdbcTypeName. Vertica returns Timestamp for timestamp type and TimestampTz for timestamp with time zone.
There was a problem hiding this comment.
I think I need to rephrase this sentence, I mean - that Vertica jdbc driver use java.sql.Timestamp for both of this types, despite other drivers use java.time.LocalDateTime for timestamp without Time Zone
Description
Vertica database has type - timestamp without time zone, but Vertica jdbc driver use java.sql.Timestamp for handling this type instead of java.time.LocalDateTime as other jdbc drivers.
So we cannot use this type for any write operations and pushdowns, because values will be automatically converted (because of java.sql.Timestamp) and
we can not guarantee correct results especially with DST rules.
However there is a way to overcome this issue, when we read the data. We can omit wrappers of java.sql.Timestampt by reading values directly as String from ResultSet. In this case we can correctly show this values in Trino.
Additional context and related issues
Release notes
( ) This is not user-visible or is docs only, and no release notes are required.
( ) Release notes are required. Please propose a release note for me.
( ) Release notes are required, with the following suggested text: