Skip to content

Commit 55aa784

Browse files
keshavdandevakeshavdandeva
committed
address pr feedback and create helper method
1 parent 1dbf482 commit 55aa784

3 files changed

Lines changed: 91 additions & 38 deletions

File tree

‎java-bigquery-jdbc/src/main/java/com/google/cloud/bigquery/jdbc/BigQueryTemporalUtility.java‎

Lines changed: 34 additions & 29 deletions
Original file line numberDiff line numberDiff line change
@@ -96,16 +96,12 @@ public static Time boxTime(String val, ZoneId zoneId) {
9696
// Legacy JDBC 3.0 API (Calendar provided):
9797
// Use legacy Calendar manipulation to intentionally replicate old JVM historical DST quirks
9898
// for January 1, 1970, ensuring strict backwards compatibility for legacy ORMs.
99-
Calendar targetCal = Calendar.getInstance(TimeZone.getTimeZone(zoneId));
100-
targetCal.set(Calendar.YEAR, 1970);
101-
targetCal.set(Calendar.MONTH, Calendar.JANUARY);
102-
targetCal.set(Calendar.DAY_OF_MONTH, 1);
103-
targetCal.set(Calendar.HOUR_OF_DAY, localTime.getHour());
104-
targetCal.set(Calendar.MINUTE, localTime.getMinute());
105-
targetCal.set(Calendar.SECOND, localTime.getSecond());
106-
targetCal.set(Calendar.MILLISECOND, localTime.getNano() / 1_000_000);
107-
108-
return new Time(targetCal.getTimeInMillis());
99+
return createTimeInCalendar(
100+
Calendar.getInstance(TimeZone.getTimeZone(zoneId)),
101+
localTime.getHour(),
102+
localTime.getMinute(),
103+
localTime.getSecond(),
104+
localTime.getNano() / 1_000_000);
109105
}
110106

111107
/**
@@ -460,23 +456,12 @@ static Time convertTimeWithCalendar(Time time, Calendar cal) {
460456
if (time == null || cal == null) {
461457
return time;
462458
}
463-
ZoneId systemZone = ZoneId.systemDefault();
464-
ZoneId targetZone = cal.getTimeZone().toZoneId();
459+
TimeZone systemZone = TimeZone.getDefault();
460+
TimeZone targetZone = cal.getTimeZone();
465461
if (systemZone.equals(targetZone)) {
466462
return time;
467463
}
468-
Calendar defaultCal = Calendar.getInstance();
469-
defaultCal.setTime(time);
470-
471-
Calendar targetCal = getSafeCalendar(cal);
472-
targetCal.set(Calendar.YEAR, 1970);
473-
targetCal.set(Calendar.MONTH, Calendar.JANUARY);
474-
targetCal.set(Calendar.DAY_OF_MONTH, 1);
475-
targetCal.set(Calendar.HOUR_OF_DAY, defaultCal.get(Calendar.HOUR_OF_DAY));
476-
targetCal.set(Calendar.MINUTE, defaultCal.get(Calendar.MINUTE));
477-
targetCal.set(Calendar.SECOND, defaultCal.get(Calendar.SECOND));
478-
targetCal.set(Calendar.MILLISECOND, defaultCal.get(Calendar.MILLISECOND));
479-
return new Time(targetCal.getTimeInMillis());
464+
return shiftTimeBetweenCalendars(time, Calendar.getInstance(), getSafeCalendar(cal));
480465
}
481466

482467
/**
@@ -493,13 +478,33 @@ static Time convertTimeToCalendar(Time time, Calendar cal) {
493478
if (time == null || cal == null) {
494479
return time;
495480
}
496-
ZoneId systemZone = ZoneId.systemDefault();
497-
ZoneId targetZone = cal.getTimeZone().toZoneId();
481+
TimeZone systemZone = TimeZone.getDefault();
482+
TimeZone targetZone = cal.getTimeZone();
498483
if (systemZone.equals(targetZone)) {
499484
return time;
500485
}
501-
LocalTime localTime = Instant.ofEpochMilli(time.getTime()).atZone(targetZone).toLocalTime();
502-
ZonedDateTime zdt = localTime.atDate(LocalDate.of(1970, 1, 1)).atZone(systemZone);
503-
return new Time(zdt.toInstant().toEpochMilli());
486+
return shiftTimeBetweenCalendars(time, getSafeCalendar(cal), Calendar.getInstance());
487+
}
488+
489+
private static Time shiftTimeBetweenCalendars(Time time, Calendar sourceCal, Calendar destCal) {
490+
sourceCal.setTime(time);
491+
return createTimeInCalendar(
492+
destCal,
493+
sourceCal.get(Calendar.HOUR_OF_DAY),
494+
sourceCal.get(Calendar.MINUTE),
495+
sourceCal.get(Calendar.SECOND),
496+
sourceCal.get(Calendar.MILLISECOND));
497+
}
498+
499+
private static Time createTimeInCalendar(
500+
Calendar destCal, int hour, int minute, int second, int millisecond) {
501+
destCal.set(Calendar.YEAR, 1970);
502+
destCal.set(Calendar.MONTH, Calendar.JANUARY);
503+
destCal.set(Calendar.DAY_OF_MONTH, 1);
504+
destCal.set(Calendar.HOUR_OF_DAY, hour);
505+
destCal.set(Calendar.MINUTE, minute);
506+
destCal.set(Calendar.SECOND, second);
507+
destCal.set(Calendar.MILLISECOND, millisecond);
508+
return new Time(destCal.getTimeInMillis());
504509
}
505510
}

‎java-bigquery-jdbc/src/test/java/com/google/cloud/bigquery/jdbc/it/ITBigQueryJDBCTest.java‎

Lines changed: 57 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
package com.google.cloud.bigquery.jdbc.it;
1818

1919
import static com.google.common.truth.Truth.assertThat;
20+
import static org.junit.jupiter.api.Assertions.assertAll;
2021
import static org.junit.jupiter.api.Assertions.assertEquals;
2122
import static org.junit.jupiter.api.Assertions.assertFalse;
2223
import static org.junit.jupiter.api.Assertions.assertNotNull;
@@ -57,6 +58,7 @@
5758
import java.time.Instant;
5859
import java.time.LocalTime;
5960
import java.time.ZoneId;
61+
import java.time.ZoneOffset;
6062
import java.time.format.DateTimeFormatter;
6163
import java.util.Calendar;
6264
import java.util.Properties;
@@ -1739,12 +1741,16 @@ public void testPreparedStatementDateTimeValues() throws SQLException {
17391741

17401742
bigQueryStatement.execute(String.format(createTableQuery, DATASET, TABLE_NAME1));
17411743

1744+
Instant moment = Instant.parse("2025-12-03T12:34:56.123Z");
1745+
Time noon = Time.valueOf(LocalTime.NOON);
1746+
Date date = Date.valueOf("2025-12-03");
1747+
17421748
PreparedStatement insertPs = bigQueryConnection.prepareStatement(insertQuery);
17431749
insertPs.setString(1, "dishwasher");
17441750
insertPs.setInt(2, 1);
1745-
insertPs.setTimestamp(3, new Timestamp(System.currentTimeMillis()));
1746-
insertPs.setTime(4, Time.valueOf(LocalTime.NOON));
1747-
insertPs.setDate(5, Date.valueOf("2025-12-3"));
1751+
insertPs.setTimestamp(3, Timestamp.from(moment));
1752+
insertPs.setTime(4, noon);
1753+
insertPs.setDate(5, date);
17481754

17491755
int insertStatus = insertPs.executeUpdate();
17501756
assertEquals(1, insertStatus);
@@ -1753,22 +1759,65 @@ public void testPreparedStatementDateTimeValues() throws SQLException {
17531759
Calendar utcCal = Calendar.getInstance(TimeZone.getTimeZone("UTC"));
17541760
insertPs.setString(1, "refrigerator");
17551761
insertPs.setInt(2, 2);
1756-
insertPs.setTimestamp(3, new Timestamp(System.currentTimeMillis()), utcCal);
1757-
insertPs.setTime(4, Time.valueOf(LocalTime.NOON), utcCal);
1758-
insertPs.setDate(5, Date.valueOf("2025-12-03"), utcCal);
1762+
insertPs.setTimestamp(3, Timestamp.from(moment), utcCal);
1763+
insertPs.setTime(4, noon, utcCal);
1764+
insertPs.setDate(5, date, utcCal);
17591765

17601766
int insertStatus2 = insertPs.executeUpdate();
17611767
assertEquals(1, insertStatus2);
17621768

1769+
// Read the stored values server-side so no driver temporal conversion is involved.
17631770
ResultSet rs =
17641771
bigQueryStatement.executeQuery(
1765-
String.format("SELECT COUNT(*) AS row_count\n" + "FROM %s.%s", DATASET, TABLE_NAME1));
1766-
rs.next();
1772+
String.format(
1773+
"SELECT IntegerField, UNIX_MILLIS(TimestampField), CAST(TimeField AS STRING),"
1774+
+ " CAST(DateField AS STRING) FROM %s.%s ORDER BY IntegerField",
1775+
DATASET, TABLE_NAME1));
1776+
1777+
assertTrue(rs.next());
1778+
assertEquals(1, rs.getInt(1));
1779+
long plainTimestamp = rs.getLong(2);
1780+
String plainTime = rs.getString(3);
1781+
String plainDate = rs.getString(4);
1782+
assertTrue(rs.next());
17671783
assertEquals(2, rs.getInt(1));
1784+
long calTimestamp = rs.getLong(2);
1785+
String calTime = rs.getString(3);
1786+
String calDate = rs.getString(4);
1787+
assertFalse(rs.next());
17681788

1789+
// Drop before asserting so a failure does not leak the table.
17691790
String dropQuery = String.format("DROP TABLE %s.%s", DATASET, TABLE_NAME1);
17701791
int dropStatus = bigQueryStatement.executeUpdate(dropQuery);
17711792
assertEquals(0, dropStatus);
1793+
1794+
assertAll(
1795+
// Row 1: plain setters.
1796+
() ->
1797+
assertEquals(
1798+
moment.toEpochMilli(), plainTimestamp, "setTimestamp must store the moment"),
1799+
() -> assertEquals("12:00:00", plainTime, "setTime must send its digits"),
1800+
() -> assertEquals("2025-12-03", plainDate, "setDate must send its digits"),
1801+
// Row 2: Calendar setters. The Calendar cannot change a TIMESTAMP moment; for TIME and DATE
1802+
// it is the zone the value is rendered in.
1803+
() ->
1804+
assertEquals(
1805+
moment.toEpochMilli(), calTimestamp, "setTimestamp(ts, cal) must store the moment"),
1806+
() ->
1807+
assertEquals(
1808+
Instant.ofEpochMilli(noon.getTime())
1809+
.atZone(ZoneOffset.UTC)
1810+
.format(DateTimeFormatter.ofPattern("HH:mm:ss")),
1811+
calTime,
1812+
"setTime(t, cal) must send the time of t in the Calendar's zone"),
1813+
() ->
1814+
assertEquals(
1815+
Instant.ofEpochMilli(date.getTime())
1816+
.atZone(ZoneOffset.UTC)
1817+
.toLocalDate()
1818+
.toString(),
1819+
calDate,
1820+
"setDate(d, cal) must send the date of d in the Calendar's zone"));
17721821
}
17731822

17741823
@Test

‎java-bigquery-jdbc/src/test/java/com/google/cloud/bigquery/jdbc/it/ITTimestampTimeZoneTest.java‎

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -219,7 +219,6 @@ public void testSetObjectZonedDateTime_storesSameMoment(String zone) throws SQLE
219219
*/
220220
@ParameterizedTest(name = "JVM zone {0}")
221221
@MethodSource("jvmZones")
222-
@Disabled
223222
public void testSetTimeWithCalendar_sendsTimeInCalendarZone(String zone) throws SQLException {
224223
useJvmZone(zone);
225224
Time time = Time.valueOf("12:34:56");

0 commit comments

Comments
 (0)