This is an automated email from the ASF dual-hosted git repository. amanin pushed a commit to branch fix/sql-temporal in repository https://gitbox.apache.org/repos/asf/sis.git
commit 213249a4d74af9cdc22ffdb691359674cf26da3d Author: Alexis Manin <[email protected]> AuthorDate: Mon May 23 19:27:02 2022 +0200 fix(Storage): properly handle SQL timestamps to avoid ambiguity Timestamps without time zone are now represented as proper local date time Timestamps with timezone, that can be mapped to an absolute point in time are mapped to java time Instant. Note that for Timestamp with timezone, a hack has been added to manage HSQLDB special case. --- .../apache/sis/internal/sql/feature/Database.java | 4 +- .../sis/internal/sql/feature/ValueGetter.java | 56 ++++++++++++---------- 2 files changed, 34 insertions(+), 26 deletions(-) diff --git a/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/Database.java b/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/Database.java index 3abf5140ed..5a7944cf09 100644 --- a/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/Database.java +++ b/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/Database.java @@ -568,9 +568,9 @@ public class Database<G> extends Syntax { case Types.LONGVARCHAR: return ValueGetter.AsString.INSTANCE; case Types.DATE: return ValueGetter.AsDate.INSTANCE; case Types.TIME: return ValueGetter.AsLocalTime.INSTANCE; - case Types.TIMESTAMP: return ValueGetter.AsInstant.INSTANCE; + case Types.TIMESTAMP: return ValueGetter.AsLocalDateTime.INSTANCE; case Types.TIME_WITH_TIMEZONE: return ValueGetter.AsOffsetTime.INSTANCE; - case Types.TIMESTAMP_WITH_TIMEZONE: return ValueGetter.AsOffsetDateTime.INSTANCE; + case Types.TIMESTAMP_WITH_TIMEZONE: return ValueGetter.AsInstant.INSTANCE; case Types.BLOB: return ValueGetter.AsBytes.INSTANCE; case Types.OTHER: case Types.JAVA_OBJECT: return getDefaultMapping(); diff --git a/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/ValueGetter.java b/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/ValueGetter.java index fd30c9aa52..97807d0f79 100644 --- a/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/ValueGetter.java +++ b/storage/sis-sqlstore/src/main/java/org/apache/sis/internal/sql/feature/ValueGetter.java @@ -16,6 +16,8 @@ */ package org.apache.sis.internal.sql.feature; +import java.time.LocalDate; +import java.time.LocalDateTime; import java.util.Calendar; import java.util.Collection; import java.sql.Array; @@ -284,18 +286,18 @@ public abstract class ValueGetter<T> { /** * A getter of {@link Date} values from the current row of a {@link ResultSet}. - * This getter delegates to {@link ResultSet#getDate(int)} and returns that value with no change. - * - * @todo Delegate to {@link ResultSet#getDate(int, Calendar)} instead. + * This getter delegates to {@link ResultSet#getDate(int)} then convert that value to {@link LocalDate java time API}. */ - static final class AsDate extends ValueGetter<Date> { + static final class AsDate extends ValueGetter<LocalDate> { /** The unique instance of this accessor. */ public static final AsDate INSTANCE = new AsDate(); - private AsDate() {super(Date.class);} + private AsDate() {super(LocalDate.class);} /** Fetches the value from the specified column in the given result set. */ - @Override public Date getValue(InfoStatements stmts, ResultSet source, int columnIndex) throws SQLException { - return source.getDate(columnIndex); + @Override public LocalDate getValue(InfoStatements stmts, ResultSet source, int columnIndex) throws SQLException { + final Date rawValue = source.getDate(columnIndex); + if (rawValue == null) return null; + else return rawValue.toLocalDate(); } } @@ -321,19 +323,17 @@ public abstract class ValueGetter<T> { /** * A getter of {@link Instant} values from the current row of a {@link ResultSet}. * This getter delegates to {@link ResultSet#getTimestamp(int)}, then converts the - * object by a call to {@link Timestamp#toInstant()}. - * - * @todo Delegate to {@link ResultSet#getTimestamp(int, Calendar)} instead. + * object by a call to {@link Timestamp#toLocalDateTime()}. */ - static final class AsInstant extends ValueGetter<Instant> { + static final class AsLocalDateTime extends ValueGetter<LocalDateTime> { /** The unique instance of this accessor. */ - public static final AsInstant INSTANCE = new AsInstant(); - private AsInstant() {super(Instant.class);} + public static final AsLocalDateTime INSTANCE = new AsLocalDateTime(); + private AsLocalDateTime() {super(LocalDateTime.class);} /** Fetches the value from the specified column in the given result set. */ - @Override public Instant getValue(InfoStatements stmts, ResultSet source, int columnIndex) throws SQLException { + @Override public LocalDateTime getValue(InfoStatements stmts, ResultSet source, int columnIndex) throws SQLException { final Timestamp time = source.getTimestamp(columnIndex); - return (time != null) ? time.toInstant() : null; + return (time != null) ? time.toLocalDateTime() : null; } } @@ -342,19 +342,27 @@ public abstract class ValueGetter<T> { * This getter delegates to {@link ResultSet#getTimestamp(int)}, converts the object by a * call to {@link Timestamp#toInstant()} then apply the time zone offset. * - * @todo Delegate to {@link ResultSet#getTimestamp(int, Calendar)} instead. + * Note: This converter is only meant for "Timestamp with time zone" data types, that describe a fixed/absolute point in time. + * Semantically, an SQL "timestamp with time zone" should better match {@link OffsetDateTime}. + * However, none of the tested JDBC drivers (PostgreSQL and HSQLDB) return the zone offset given at insertion. + * They both return the timestamp UTC, meaning that in any cases, the original zone offset information in lost. + * Therefore, it is more straightforward to return an UTC Timestamp, i.e {@link Instant} anyway. + * + * WARNING: timeStamps returned by HSQLDB looks flawed. To bypass the problem, we add a special case to check if the + * driver directly returns a java.time data type. If not, then we fallback to {@link Timestamp SQL Timestamps}. */ - static final class AsOffsetDateTime extends ValueGetter<OffsetDateTime> { + static final class AsInstant extends ValueGetter<Instant> { /** The unique instance of this accessor. */ - public static final AsOffsetDateTime INSTANCE = new AsOffsetDateTime(); - private AsOffsetDateTime() {super(OffsetDateTime.class);} + public static final AsInstant INSTANCE = new AsInstant(); + private AsInstant() {super(Instant.class);} /** Fetches the value from the specified column in the given result set. */ - @Override public OffsetDateTime getValue(InfoStatements stmts, ResultSet source, int columnIndex) throws SQLException { - final Timestamp time = source.getTimestamp(columnIndex); - if (time == null) return null; - final int offsetMinute = time.getTimezoneOffset(); - return time.toInstant().atOffset(ZoneOffset.ofHoursMinutes(offsetMinute / 60, offsetMinute % 60)); + @Override public Instant getValue(InfoStatements stmts, ResultSet source, int columnIndex) throws SQLException { + final Object rawValue = source.getObject(columnIndex); + if (rawValue == null) return null; + else if (rawValue instanceof Instant) return (Instant) rawValue; + else if (rawValue instanceof OffsetDateTime) return (((OffsetDateTime) rawValue).toInstant()); + else return source.getTimestamp(columnIndex).toInstant(); } }
