-
Notifications
You must be signed in to change notification settings - Fork 1.2k
fix(bigquery-jdbc): normalize timestamp string representation and improve temporal coercions #14037
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.
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
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -16,6 +16,9 @@ | |
|
|
||
| package com.google.cloud.bigquery.jdbc; | ||
|
|
||
| import com.google.common.base.Strings; | ||
| import java.math.BigDecimal; | ||
| import java.math.RoundingMode; | ||
| import java.sql.Date; | ||
| import java.sql.Time; | ||
| import java.sql.Timestamp; | ||
|
|
@@ -24,6 +27,8 @@ | |
| import java.time.LocalDateTime; | ||
| import java.time.LocalTime; | ||
| import java.time.ZoneId; | ||
| import java.time.ZoneOffset; | ||
| import java.time.format.DateTimeFormatter; | ||
| import java.util.Calendar; | ||
|
|
||
| /** | ||
|
|
@@ -32,6 +37,9 @@ | |
| */ | ||
| final class BigQueryTemporalUtility { | ||
|
|
||
| private static final DateTimeFormatter UTC_FORMATTER = | ||
| DateTimeFormatter.ofPattern("yyyy-MM-dd HH:mm:ss").withZone(ZoneOffset.UTC); | ||
|
|
||
| private BigQueryTemporalUtility() {} | ||
|
|
||
| /** | ||
|
|
@@ -89,6 +97,28 @@ public static Time boxTime(String val, ZoneId zoneId) { | |
| return new Time(targetCal.getTimeInMillis()); | ||
| } | ||
|
|
||
| /** | ||
| * Truncates a BigQuery timestamp string to 9 fractional digits (nanoseconds) because | ||
| * Instant.parse throws DateTimeParseException for >9 digits, and java.sql.Timestamp maxes out at | ||
| * nanos anyway. | ||
| */ | ||
| private static String truncateToNanoseconds(String iso) { | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What happens if there is timezone data? Seems like this code assumes timezone is not specified, should we validate it? |
||
| int dotIdx = iso.indexOf('.'); | ||
| // Fast path: if there is no dot or at most 9 fractional digits after the dot, return as-is. | ||
| if (dotIdx == -1 || iso.length() - dotIdx <= 10) { | ||
| return iso; | ||
| } | ||
|
|
||
| int fractionEnd = dotIdx + 1; | ||
| while (fractionEnd < iso.length() && Character.isDigit(iso.charAt(fractionEnd))) { | ||
| fractionEnd++; | ||
| } | ||
| if (fractionEnd - dotIdx - 1 > 9) { | ||
| return iso.substring(0, dotIdx + 10) + iso.substring(fractionEnd); | ||
| } | ||
| return iso; | ||
| } | ||
|
|
||
| /** | ||
| * Converts a BigQuery absolute TIMESTAMP string into a legacy Timestamp. Because it is absolute, | ||
| * the Calendar timezone is explicitly ignored per JDBC 4.2 spec. | ||
|
|
@@ -105,11 +135,109 @@ public static Timestamp boxTimestamp(String val) { | |
| iso = iso.substring(0, 10) + 'T' + iso.substring(11); | ||
| } | ||
|
|
||
| iso = truncateToNanoseconds(iso); | ||
|
|
||
| try { | ||
| return Timestamp.from(Instant.parse(iso)); | ||
| } catch (java.time.format.DateTimeParseException e) { | ||
| // Fallback for non-standard formats | ||
| return Timestamp.valueOf(val); | ||
| return Timestamp.valueOf(truncateToNanoseconds(val)); | ||
| } | ||
|
keshavdandeva marked this conversation as resolved.
|
||
| } | ||
|
|
||
| /** | ||
| * Parses a numeric epoch decimal string (e.g. from BigQuery REST JSON) into a JSR-310 {@link | ||
| * Instant}. Sub-nanosecond precision is deterministically truncated (floor/down) rather than | ||
| * rounded to avoid boundary rollovers. | ||
| */ | ||
| public static Instant parseEpochDecimalToInstant(String epochDecimal) { | ||
| if (epochDecimal == null) { | ||
| return null; | ||
| } | ||
| BigDecimal bd = new BigDecimal(epochDecimal); | ||
| long seconds = bd.setScale(0, RoundingMode.FLOOR).longValue(); | ||
| long nanos = | ||
| bd.subtract(BigDecimal.valueOf(seconds)) | ||
| .movePointRight(9) | ||
| .setScale(0, RoundingMode.DOWN) | ||
| .longValue(); | ||
| return Instant.ofEpochSecond(seconds, nanos); | ||
| } | ||
|
|
||
| /** | ||
| * Formats a numeric epoch decimal string into standard SQL timestamp string format ("yyyy-MM-dd | ||
| * HH:mm:ss.ffffff"). Sub-microsecond precision is deterministically truncated (down) to prevent | ||
| * timestamp boundary rollovers. | ||
| */ | ||
| public static String formatTimestampString(String epochDecimal) { | ||
| return formatTimestampString(epochDecimal, false); | ||
| } | ||
|
|
||
| /** | ||
| * Formats a numeric epoch decimal string into standard SQL timestamp string format ("yyyy-MM-dd | ||
| * HH:mm:ss.ffffff[ffffff]"). Sub-microsecond / sub-picosecond precision is deterministically | ||
| * truncated (down) to prevent timestamp boundary rollovers. | ||
| */ | ||
| public static String formatTimestampString(String epochDecimal, boolean enableTimestampPicos) { | ||
| if (epochDecimal == null) { | ||
| return null; | ||
| } | ||
|
|
||
| BigDecimal bd = new BigDecimal(epochDecimal); | ||
| long seconds = bd.setScale(0, RoundingMode.FLOOR).longValue(); | ||
| BigDecimal fractionalSeconds = bd.subtract(BigDecimal.valueOf(seconds)); | ||
|
|
||
| int originalScale = bd.scale() > 0 ? bd.scale() : 0; | ||
| int scale = enableTimestampPicos ? Math.max(6, Math.min(12, originalScale)) : 6; | ||
|
|
||
| String fraction = | ||
| fractionalSeconds.setScale(scale, RoundingMode.DOWN).toPlainString().substring(2); | ||
|
|
||
| Instant instant = Instant.ofEpochSecond(seconds); | ||
| return UTC_FORMATTER.format(instant) + "." + fraction; | ||
| } | ||
|
|
||
| public static String formatTimestampStringFromMicroseconds(long microseconds) { | ||
| long seconds = Math.floorDiv(microseconds, 1000000L); | ||
| long micros = Math.floorMod(microseconds, 1000000L); | ||
|
|
||
| String fraction = Strings.padStart(Long.toString(micros), 6, '0'); | ||
| Instant instant = Instant.ofEpochSecond(seconds); | ||
| return UTC_FORMATTER.format(instant) + "." + fraction; | ||
| } | ||
|
|
||
| public static String formatTimestampStringFromIso( | ||
| String isoString, boolean enableTimestampPicos) { | ||
| if (isoString == null) { | ||
| return null; | ||
| } | ||
|
|
||
| String s = isoString; | ||
| if (s.endsWith(" UTC")) { | ||
| s = s.substring(0, s.length() - 4); | ||
| } else if (s.endsWith("Z")) { | ||
| s = s.substring(0, s.length() - 1); | ||
| } | ||
|
|
||
| if (s.length() > 10 && s.charAt(10) == 'T') { | ||
| s = s.substring(0, 10) + ' ' + s.substring(11); | ||
| } | ||
|
|
||
| int dotIdx = s.indexOf('.'); | ||
| if (dotIdx == -1) { | ||
| return s + ".000000"; | ||
| } | ||
|
|
||
| String base = s.substring(0, dotIdx); | ||
| String fraction = s.substring(dotIdx + 1); | ||
|
|
||
| int maxScale = enableTimestampPicos ? 12 : 6; | ||
| if (fraction.length() > maxScale) { | ||
| fraction = fraction.substring(0, maxScale); | ||
| } else if (fraction.length() < 6) { | ||
| fraction = Strings.padEnd(fraction, 6, '0'); | ||
| } | ||
|
|
||
| return base + "." + fraction; | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -127,10 +127,10 @@ public static Collection<Object[]> data() { | |
| TIMESTAMP, | ||
| arraySchemaAndValue( | ||
| TIMESTAMP, | ||
| "1680174859.8202269", | ||
| "1680261259.8202269", | ||
| "1680347659.8202269", | ||
| "1680434059.8202269"), | ||
| "1680174859.820227", | ||
| "1680261259.820227", | ||
| "1680347659.820227", | ||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. In another test you validate that it is always truncated down, not up. But that test contradicts it |
||
| "1680434059.820227"), | ||
| new Timestamp[] { | ||
| Timestamp.valueOf(aTimeStamp), // 2023-03-30 16:44:19.82 | ||
| Timestamp.valueOf(aTimeStamp.plusDays(1)), | ||
|
|
||
Uh oh!
There was an error while loading. Please reload this page.