Skip to content

Commit

Permalink
Feature/fix date conversion with tz (#99)
Browse files Browse the repository at this point in the history
  • Loading branch information
aymeric-dispa authored Oct 7, 2022
1 parent 20b8560 commit 4eb07c1
Show file tree
Hide file tree
Showing 6 changed files with 160 additions and 134 deletions.
2 changes: 1 addition & 1 deletion build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ plugins {
}

group 'com.firebolt'
version = '2.2.2'
version = '2.2.3-SNAPSHOT'

repositories {
mavenCentral()
Expand Down
38 changes: 26 additions & 12 deletions src/integrationTest/java/integration/tests/TimestampTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -3,16 +3,17 @@
import static org.junit.jupiter.api.Assertions.assertEquals;

import java.sql.*;
import java.time.LocalDate;
import java.time.LocalDateTime;
import java.time.LocalTime;
import java.time.ZonedDateTime;
import java.util.TimeZone;

import org.junit.jupiter.api.Test;
import org.junitpioneer.jupiter.DefaultTimeZone;

import integration.IntegrationTest;
import lombok.extern.slf4j.Slf4j;

@Slf4j
@DefaultTimeZone("UTC")
class TimestampTest extends IntegrationTest {

@Test
Expand All @@ -22,9 +23,14 @@ void shouldGetTimeObjectsInEstTimezone() throws SQLException {
ResultSet resultSet = statement
.executeQuery("SELECT TO_TIMESTAMP_EXT('1975/01/01 23:01:01', 6, 'EST');")) {
resultSet.next();
Timestamp expectedTimestamp = Timestamp.valueOf(LocalDateTime.of(1975, 1, 2, 4, 1, 1));
Time expectedTime = Time.valueOf(LocalTime.of(4, 1, 1));
Date expectedDate = Date.valueOf(LocalDate.of(1975, 1, 2));

ZonedDateTime zonedDateTime = ZonedDateTime.of(1975, 1, 2, 4, 1, 1, 0,
TimeZone.getTimeZone("UTC").toZoneId());

Timestamp expectedTimestamp = Timestamp.valueOf(zonedDateTime.toLocalDateTime());
Time expectedTime = Time.valueOf(zonedDateTime.toLocalTime());
Date expectedDate = Date.valueOf(zonedDateTime.toLocalDate());

assertEquals(expectedTimestamp, resultSet.getTimestamp(1));
assertEquals(expectedTimestamp, resultSet.getObject(1));
assertEquals(expectedTime, resultSet.getTime(1));
Expand All @@ -38,9 +44,13 @@ void shouldGetTimeObjectsInDefaultUTCTimezone() throws SQLException {
Statement statement = connection.createStatement();
ResultSet resultSet = statement.executeQuery("SELECT TO_TIMESTAMP_EXT('1975/01/01 23:01:01', 6);")) {
resultSet.next();
Timestamp expectedTimestamp = Timestamp.valueOf(LocalDateTime.of(1975, 1, 1, 23, 1, 1));
Time expectedTime = Time.valueOf(LocalTime.of(23, 1, 1));
Date expectedDate = Date.valueOf(LocalDate.of(1975, 1, 1));
ZonedDateTime zonedDateTime = ZonedDateTime.of(1975, 1, 1, 23, 1, 1, 0,
TimeZone.getTimeZone("UTC").toZoneId());

Timestamp expectedTimestamp = Timestamp.valueOf(zonedDateTime.toLocalDateTime());
Time expectedTime = Time.valueOf(zonedDateTime.toLocalTime());
Date expectedDate = Date.valueOf(zonedDateTime.toLocalDate());

assertEquals(expectedTimestamp, resultSet.getTimestamp(1));
assertEquals(expectedTimestamp, resultSet.getObject(1));
assertEquals(expectedTime, resultSet.getTime(1));
Expand All @@ -55,9 +65,13 @@ void shouldGetParsedTimeStampExtTimeObjects() throws SQLException {
ResultSet resultSet = statement
.executeQuery("SELECT CAST('1111-11-11 ' || '12:00:03' AS timestamp_ext);")) {
resultSet.next();
Timestamp expectedTimestamp = Timestamp.valueOf(LocalDateTime.of(1111, 11, 11, 12, 0, 3));
Time expectedTime = Time.valueOf(LocalTime.of(12, 0, 3));
Date expectedDate = Date.valueOf(LocalDate.of(1111, 11, 11));
ZonedDateTime zonedDateTime = ZonedDateTime.of(1111, 11, 11, 12, 0, 3, 0,
TimeZone.getTimeZone("UTC").toZoneId());

Timestamp expectedTimestamp = Timestamp.valueOf(zonedDateTime.toLocalDateTime());
Time expectedTime = Time.valueOf(zonedDateTime.toLocalTime());
Date expectedDate = Date.valueOf(zonedDateTime.toLocalDate());

assertEquals(expectedTimestamp, resultSet.getTimestamp(1));
assertEquals(expectedTimestamp, resultSet.getObject(1));
assertEquals(expectedTime, resultSet.getTime(1));
Expand Down
43 changes: 24 additions & 19 deletions src/main/java/com/firebolt/jdbc/statement/FireboltStatement.java
Original file line number Diff line number Diff line change
Expand Up @@ -56,47 +56,50 @@ public ResultSet executeQuery(String sql) throws SQLException {

protected ResultSet executeQuery(List<StatementInfoWrapper> statementInfoList) throws SQLException {
StatementInfoWrapper query = getOneQueryStatementInfo(statementInfoList);
this.execute(Collections.singletonList(query), null);
Optional<ResultSet> resultSet = this.execute(Collections.singletonList(query), null);
synchronized (this) {
if (firstUnclosedStatementResult == null) {
if (!resultSet.isPresent()) {
throw new FireboltException("Could not return ResultSet - the result object is null");
} else {
return firstUnclosedStatementResult.getResultSet();
return resultSet.get();
}
}
}

private boolean execute(String sql, Map<String, String> params) throws SQLException {
return this.execute(StatementUtil.parseToStatementInfoWrappers(sql), params);
return this.execute(StatementUtil.parseToStatementInfoWrappers(sql), params).isPresent();
}

private boolean execute(List<StatementInfoWrapper> statements, Map<String, String> params) throws SQLException {
Boolean isFirstStatementAQuery = null;
private Optional<ResultSet> execute(List<StatementInfoWrapper> statements, Map<String, String> params)
throws SQLException {
Optional<ResultSet> resultSet = Optional.empty();
this.closeAllResults();
Set<String> queryIds = statements.stream().map(StatementInfoWrapper::getId)
.collect(Collectors.toCollection(HashSet::new));
try {
synchronized (statementsToExecuteIds) {
statementsToExecuteIds.addAll(queryIds);
}
for (StatementInfoWrapper statementInfoWrapper : statements) {
boolean isQuery = execute(statementInfoWrapper, params, true);
isFirstStatementAQuery = isFirstStatementAQuery == null ? isQuery : isFirstStatementAQuery;
for (int i = 0; i < statements.size(); i++) {
if (i == 0) {
resultSet = execute(statements.get(i), params, true);
} else {
execute(statements.get(i), params, true);
}
}
} finally {
synchronized (statementsToExecuteIds) {
statementsToExecuteIds.removeAll(queryIds);
}
}
return isFirstStatementAQuery != null && isFirstStatementAQuery;
return resultSet;
}

private boolean execute(StatementInfoWrapper statementInfoWrapper, Map<String, String> params,
private Optional<ResultSet> execute(StatementInfoWrapper statementInfoWrapper, Map<String, String> params,
boolean verifyNotCancelled) throws SQLException {
boolean isQuery = statementInfoWrapper.getType() == StatementType.QUERY;
ResultSet resultSet = null;
if (!verifyNotCancelled || isStatementNotCancelled(statementInfoWrapper)) {
runningStatementId = statementInfoWrapper.getId();
ResultSet rs = null;
synchronized (this) {
this.validateStatementIsNotClosed();
}
Expand All @@ -112,7 +115,8 @@ private boolean execute(StatementInfoWrapper statementInfoWrapper, Map<String, S
inputStream = statementService.execute(statementInfoWrapper, this.sessionProperties,
statementParams);
if (statementInfoWrapper.getType() == StatementType.QUERY) {
rs = getResultSet(inputStream, (QueryRawStatement) statementInfoWrapper.getInitialStatement());
resultSet = getResultSet(inputStream,
(QueryRawStatement) statementInfoWrapper.getInitialStatement());
currentUpdateCount = -1; // Always -1 when returning a ResultSet
} else {
currentUpdateCount = 0;
Expand All @@ -129,16 +133,17 @@ private boolean execute(StatementInfoWrapper statementInfoWrapper, Map<String, S
}
synchronized (this) {
if (this.firstUnclosedStatementResult == null) {
this.firstUnclosedStatementResult = this.currentStatementResult = new StatementResultWrapper(rs,
statementInfoWrapper);
this.firstUnclosedStatementResult = this.currentStatementResult = new StatementResultWrapper(
resultSet, statementInfoWrapper);
} else {
this.firstUnclosedStatementResult.append(new StatementResultWrapper(rs, statementInfoWrapper));
this.firstUnclosedStatementResult
.append(new StatementResultWrapper(resultSet, statementInfoWrapper));
}
}
} else {
log.warn("Did not run cancelled query with id {}", statementInfoWrapper.getId());
}
return isQuery;
return Optional.ofNullable(resultSet);
}

private boolean isStatementNotCancelled(StatementInfoWrapper statementInfoWrapper) {
Expand Down Expand Up @@ -365,7 +370,7 @@ public boolean execute(String sql) throws SQLException {
}

public boolean execute(List<StatementInfoWrapper> statementInfoList) throws SQLException {
return this.execute(statementInfoList, null);
return this.execute(statementInfoList, null).isPresent();
}

@Override
Expand Down
36 changes: 14 additions & 22 deletions src/main/java/com/firebolt/jdbc/type/date/SqlDateUtil.java
Original file line number Diff line number Diff line change
Expand Up @@ -23,45 +23,37 @@
@Slf4j
public class SqlDateUtil {

private DateTimeFormatter dateFormatter = DateTimeFormatter.ofPattern("yyyy-MM-dd");
private static final ZoneId UTC_ZONE_ID = ZoneId.of("UTC");
private static final TimeZone DEFAULT_TZ = TimeZone.getDefault();

private static final DateTimeFormatter dateFormatter = DateTimeFormatter.ofPattern("yyyy-MM-dd");

public static final Function<Date, String> transformFromDateToSQLStringFunction = value -> String.format("'%s'",
dateFormatter.format(value.toLocalDate()));
DateTimeFormatter dateTimeFormatter = new DateTimeFormatterBuilder().appendPattern("yyyy-MM-dd [HH:mm[:ss]]")
.appendFraction(ChronoField.NANO_OF_SECOND, 0, 9, true).toFormatter();
public static final BiFunction<String, TimeZone, Timestamp> transformToTimestampFunction = (value,
fromTimeZone) -> {
Optional<ZonedDateTime> zdt = parse(value, fromTimeZone);
return zdt.map(t -> Timestamp.valueOf(t.toLocalDateTime())).orElse(null);
};
fromTimeZone) -> parse(value, fromTimeZone).map(t -> Timestamp.valueOf(t.toLocalDateTime())).orElse(null);

public static final BiFunction<String, TimeZone, Date> transformToDateFunction = (value, fromTimeZone) -> {
Optional<ZonedDateTime> zdt = parse(value, fromTimeZone);
return zdt.map(t -> Date.valueOf(t.toLocalDate())).orElse(null);
};
public static final BiFunction<String, TimeZone, Date> transformToDateFunction = (value,
fromTimeZone) -> parse(value, fromTimeZone).map(t -> Date.valueOf(t.toLocalDate())).orElse(null);

public static final BiFunction<String, TimeZone, Time> transformToTimeFunction = (value, fromTimeZone) -> {
Optional<ZonedDateTime> zdt = parse(value, fromTimeZone);
return zdt.map(t -> Time.valueOf(LocalTime.of(t.getHour(), t.getMinute(), t.getSecond(), t.getNano())))
.orElse(null);
};
public static final BiFunction<String, TimeZone, Time> transformToTimeFunction = (value,
fromTimeZone) -> parse(value, fromTimeZone).map(t -> Time.valueOf(t.toLocalTime())).orElse(null);
public static final Function<Timestamp, String> transformFromTimestampToSQLStringFunction = value -> String
.format("'%s'", dateTimeFormatter.format(value.toLocalDateTime()));

private static Optional<ZonedDateTime> parse(String value, @Nullable TimeZone fromTimeZone) {
if (StringUtils.isEmpty(value)) {
return Optional.empty();
}
ZoneId zoneId = fromTimeZone == null ? UTC_ZONE_ID : fromTimeZone.toZoneId();
ZoneId zoneId = fromTimeZone == null ? DEFAULT_TZ.toZoneId() : fromTimeZone.toZoneId();
try {
return Optional
.of(LocalDateTime.parse(value, dateTimeFormatter).atZone(zoneId).withZoneSameInstant(UTC_ZONE_ID));
return Optional.of(LocalDateTime.parse(value, dateTimeFormatter).atZone(zoneId)
.withZoneSameInstant(DEFAULT_TZ.toZoneId()));
} catch (DateTimeException dateTimeException) {
LocalDate date = LocalDate.from(dateFormatter.parse(value));
return Optional.of(LocalDateTime.of(date.getYear(), date.getMonth(), date.getDayOfMonth(), 0, 0)
.atZone(zoneId).withZoneSameInstant(UTC_ZONE_ID));
.atZone(zoneId).withZoneSameInstant(DEFAULT_TZ.toZoneId()));
}
}

public static final Function<Timestamp, String> transformFromTimestampToSQLStringFunction = value -> String
.format("'%s'", dateTimeFormatter.format(value.toLocalDateTime()));
}
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
package com.firebolt.jdbc.resultset;

import static java.sql.ResultSet.TYPE_FORWARD_ONLY;
import static java.sql.Time.valueOf;
import static org.junit.jupiter.api.Assertions.*;
import static org.mockito.Mockito.*;

Expand All @@ -12,8 +11,7 @@
import java.text.ParseException;
import java.text.SimpleDateFormat;
import java.time.LocalDate;
import java.time.LocalDateTime;
import java.time.LocalTime;
import java.time.ZonedDateTime;
import java.util.Calendar;
import java.util.TimeZone;

Expand All @@ -33,6 +31,7 @@
@DefaultTimeZone("UTC")
class FireboltResultSetTest {

private static final TimeZone UTC_TZ = TimeZone.getTimeZone("UTC");
private InputStream inputStream;
private ResultSet resultSet;

Expand Down Expand Up @@ -321,7 +320,7 @@ void shouldReturnBoolean() throws SQLException {

@Test
void shouldReturnTime() throws SQLException {
Time expectedTime = Time.valueOf(LocalTime.of(13, 1, 2));
Time expectedTime = Time.valueOf(ZonedDateTime.of(2022, 5, 10, 13, 1, 2, 0, UTC_TZ.toZoneId()).toLocalTime());
inputStream = getInputStreamWithArray();
resultSet = new FireboltResultSet(inputStream, "array_test_table", "array_test_db", 65535);
resultSet.next();
Expand Down Expand Up @@ -460,9 +459,15 @@ void shouldGetTimeWithTimezoneFromCalendar() throws SQLException {
fireboltStatement, true);
resultSet.next();

assertEquals(valueOf(LocalTime.of(18, 1, 2)), resultSet.getTime("a_timestamp", EST_CALENDAR));
assertEquals(valueOf(LocalTime.of(13, 1, 2)), resultSet.getTime("a_timestamp", UTC_CALENDAR));
assertEquals(valueOf(LocalTime.of(13, 1, 2)), resultSet.getTime("a_timestamp", null));
Time firstExpectedTime = Time
.valueOf(ZonedDateTime.of(2022, 5, 10, 18, 1, 2, 0, UTC_TZ.toZoneId()).toLocalTime());

Time secondExpectedTime = Time
.valueOf(ZonedDateTime.of(2022, 5, 10, 13, 1, 2, 0, UTC_TZ.toZoneId()).toLocalTime());

assertEquals(firstExpectedTime, resultSet.getTime("a_timestamp", EST_CALENDAR));
assertEquals(secondExpectedTime, resultSet.getTime("a_timestamp", UTC_CALENDAR));
assertEquals(secondExpectedTime, resultSet.getTime("a_timestamp", null));

}
}
Expand All @@ -475,16 +480,18 @@ void shouldGetTimestampWithTimezoneFromCalendar() throws SQLException {
resultSet = new FireboltResultSet(inputStream, "array_test_table", "array_test_db", 65535, false,
fireboltStatement, true);
resultSet.next();

assertEquals(Timestamp.valueOf(LocalDateTime.of(2022, 5, 10, 18, 1, 2)),
resultSet.getTimestamp("a_timestamp", EST_CALENDAR));
assertEquals(Timestamp.valueOf(LocalDateTime.of(2022, 5, 10, 13, 1, 2)),
resultSet.getTimestamp("a_timestamp", UTC_CALENDAR));
assertEquals(Timestamp.valueOf(LocalDateTime.of(2022, 5, 10, 13, 1, 2)),
resultSet.getTimestamp("a_timestamp", null));
Timestamp firstTimeStampFromEST = Timestamp
.valueOf(ZonedDateTime.of(2022, 5, 10, 18, 1, 2, 0, UTC_TZ.toZoneId()).toLocalDateTime());
Timestamp firstTimeStampFromUTC = Timestamp
.valueOf(ZonedDateTime.of(2022, 5, 10, 13, 1, 2, 0, UTC_TZ.toZoneId()).toLocalDateTime());
Timestamp secondTimeStampFromUTC = Timestamp
.valueOf(ZonedDateTime.of(2022, 5, 11, 4, 1, 2, 0, UTC_TZ.toZoneId()).toLocalDateTime());

assertEquals(firstTimeStampFromEST, resultSet.getTimestamp("a_timestamp", EST_CALENDAR));
assertEquals(firstTimeStampFromUTC, resultSet.getTimestamp("a_timestamp", UTC_CALENDAR));
assertEquals(firstTimeStampFromUTC, resultSet.getTimestamp("a_timestamp", null));
resultSet.next();
assertEquals(Timestamp.valueOf(LocalDateTime.of(2022, 5, 11, 4, 1, 2)),
resultSet.getTimestamp("a_timestamp", EST_CALENDAR));
assertEquals(secondTimeStampFromUTC, resultSet.getTimestamp("a_timestamp", EST_CALENDAR));
}
}

Expand All @@ -496,8 +503,14 @@ void shouldGetTimeObjectsWithTimeZoneFromResponse() throws SQLException {
resultSet = new FireboltResultSet(inputStream, "array_test_table", "array_test_db", 65535, false,
fireboltStatement, true);
resultSet.next();
Timestamp expectedTimestamp = Timestamp.valueOf(LocalDateTime.of(2022, 5, 10, 18, 1, 2));
Time expectedTime = valueOf(LocalTime.of(18, 1, 2));
ZonedDateTime zonedDateTime = ZonedDateTime.of(2022, 5, 10, 18, 1, 2, 0, UTC_TZ.toZoneId());

Timestamp expectedTimestamp = Timestamp.valueOf(zonedDateTime.toLocalDateTime());

Timestamp.valueOf(zonedDateTime.toLocalDateTime());
Time expectedTime = Time.valueOf(zonedDateTime.toLocalTime());
Date expectedDate = Date.valueOf(
ZonedDateTime.of(2022, 5, 11, 4, 1, 2, 0, TimeZone.getTimeZone("UTC").toZoneId()).toLocalDate());

// The timezone returned by the db is always used regardless of the timezone
// passed as an argument
Expand All @@ -509,8 +522,7 @@ void shouldGetTimeObjectsWithTimeZoneFromResponse() throws SQLException {
assertEquals(expectedTimestamp, resultSet.getTimestamp("a_timestamp_with_tz", UTC_CALENDAR));
assertEquals(expectedTimestamp, resultSet.getTimestamp("a_timestamp_with_tz", null));
resultSet.next();
assertEquals(Date.valueOf(LocalDate.of(2022, 5, 11)),
resultSet.getDate("a_timestamp_with_tz", UTC_CALENDAR));
assertEquals(expectedDate, resultSet.getDate("a_timestamp_with_tz", UTC_CALENDAR));
}
}

Expand All @@ -522,10 +534,17 @@ void shouldGetDateWithTimezoneFromCalendar() throws SQLException {
resultSet = new FireboltResultSet(inputStream, "array_test_table", "array_test_db", 65535, false,
fireboltStatement, true);
resultSet.next();
assertEquals(Date.valueOf(LocalDate.of(2022, 5, 10)), resultSet.getDate("a_timestamp", EST_CALENDAR));
Date firstExpectedDateFromEST = Date.valueOf(
ZonedDateTime.of(2022, 5, 10, 18, 1, 2, 0, TimeZone.getTimeZone("UTC").toZoneId()).toLocalDate());
Date secondExpectedDateFromEST = Date.valueOf(
ZonedDateTime.of(2022, 5, 11, 4, 1, 2, 0, TimeZone.getTimeZone("UTC").toZoneId()).toLocalDate());
Date secondExpectedDateFromUTC = Date.valueOf(
ZonedDateTime.of(2022, 5, 10, 23, 1, 2, 0, TimeZone.getTimeZone("UTC").toZoneId()).toLocalDate());

assertEquals(firstExpectedDateFromEST, resultSet.getDate("a_timestamp", EST_CALENDAR));
resultSet.next();
assertEquals(Date.valueOf(LocalDate.of(2022, 5, 11)), resultSet.getDate("a_timestamp", EST_CALENDAR));
assertEquals(Date.valueOf(LocalDate.of(2022, 5, 10)), resultSet.getDate("a_timestamp", UTC_CALENDAR));
assertEquals(secondExpectedDateFromEST, resultSet.getDate("a_timestamp", EST_CALENDAR));
assertEquals(secondExpectedDateFromUTC, resultSet.getDate("a_timestamp", UTC_CALENDAR));
}
}

Expand Down
Loading

0 comments on commit 4eb07c1

Please sign in to comment.