fix(bigquery-jdbc): normalize timestamp string representation and improve temporal coercions - #14037
Open
keshavdandeva wants to merge 1 commit into
Open
fix(bigquery-jdbc): normalize timestamp string representation and improve temporal coercions#14037keshavdandeva wants to merge 1 commit into
keshavdandeva wants to merge 1 commit into
Conversation
…rove temporal coercions
Contributor
There was a problem hiding this comment.
Code Review
This pull request enhances the BigQuery JDBC driver's handling of TIMESTAMP values, specifically improving the parsing and formatting of numeric epoch decimal strings and supporting sub-microsecond precision. The review feedback highlights three key issues: a bug in BigQueryTypeCoercionUtility where negative epoch decimal strings incorrectly trigger the ISO/SQL format branch due to a leading hyphen; a bug in BigQueryTemporalUtility.boxTimestamp where Instant.parse fails due to a missing timezone offset and the fallback fails on trailing timezone suffixes; and a potential NumberFormatException in BigQueryJsonResultSet if the timestamp value is already formatted as an ISO/SQL string.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
b/544843125
This PR standardizes
TIMESTAMPstring representation across both Arrow and JSON result sets and introduces a unified mathematical epoch parser for temporal coercions.Problem Statement
getString(): Previously,getString()on the JSON REST path returned raw numeric epoch decimal strings (e.g."1408452095.22"or scientific notation"1.6905474E9"), diverging from the Arrow engine which returns formatted timestamp strings.FieldValueToTimestampcoerced timestamps throughgetTimestampValue(), truncating precision to microseconds and causing a single-second offset for pre-1970 negative epoch timestamps due to un-floored division.Changes Made
TIMESTAMP String Normalization:
BigQueryJsonResultSet.getString()to consistently formatTIMESTAMPcolumns into standard UTC timestamp strings ("yyyy-MM-dd HH:mm:ss.ffffff"), matching the Arrow engine while safely delegatingRECORDandREPEATEDfields.Unified Mathematical Temporal Parser:
BigQueryTemporalUtility.parseEpochDecimalToInstant()usingBigDecimalmathematical flooring (RoundingMode.DOWN). This losslessly handles standard epoch decimals, scientific notation (e.g."1.6905474E9"), whole integer epochs, and pre-1970 negative epoch decimals without sign-inversion or off-by-one errors.formatTimestampStringFromMicroseconds()using primitive integer arithmetic (Math.floorDiv/Math.floorMod) for high-throughput streaming on Arrow paths.Improved Temporal Coercion:
FieldValueToTimestampinBigQueryTypeCoercionUtilityto delegate directly toparseEpochDecimalToInstant(), preserving up to nanosecond precision forgetTimestamp()andgetObject().Resource Management:
ArrowUtilities.serializeVectorSchemaRoot()to properly closeArrowRecordBatchand prevent buffer leaks in tests.Testing
BigQueryTemporalUtilityTest: Added tests for standard epoch decimals, scientific notation, pre-1970 negative timestamps ("-1.5","-0.123456"), and deterministic truncation (RoundingMode.DOWN).FieldValueTypeBigQueryCoercionUtilityTest: Added tests for nanosecond precision preservation, scientific notation, pre-1970 negative epochs, and sub-second truncation.BigQueryJsonResultSetTest: Verified thatgetString()returns standardized UTC formatted timestamp strings for scalar timestamps and handlesRECORD/REPEATEDfields.BigQueryJsonArrayOfPrimitivesTest: Updated test fixture to exact 6-digit timestamp.