Repository navigation
fix(bigquery-jdbc): honor Calendar timezone and prevent ClassCastException in ResultSet temporal getters - #14657
Open
keshavdandeva wants to merge 2 commits into
Open
keshavdandeva wants to merge 2 commits into
keshavdandeva wants to merge 2 commits into
Conversation
…xception` in ResultSet temporal getters
Contributor
There was a problem hiding this comment.
Code Review
This pull request refactors temporal type handling and timezone conversions across BigQuery JDBC result sets and the type registry, introducing ZoneId-aware boxing and conversion methods. Feedback on these changes highlights a potential JDBC specification compliance issue where Calendar timezones are ignored for TIMESTAMP columns, as well as backward compatibility concerns regarding new coercion exceptions for INT64 columns in getTime and getTimestamp. Additionally, a bug was identified in BigQueryTypeRegistry where Class.isInstance fails on primitive types, which can be resolved by wrapping them.
This branch has not been deployed
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/565791346
Summary
Fixes two bugs in
ResultSet.getDate(col, Calendar),ResultSet.getTime(col, Calendar), andResultSet.getTimestamp(col, Calendar):Calendarignored onDATE,TIME, andDATETIMEcolumns:Calendargetter overloads calledgetObject(columnIndex)first withzone = null, which boxed raw values intoDate,Time, orTimestampanchored in the JVM default timezone beforeBigQueryTypeRegistry.convert(..., zone)ran.DATE(LocalDate/Integer) andDATETIME(LocalDateTime) branches inBigQueryTypeRegistryalso usedDate.valueOf(...)andTimestamp.valueOf(...), ignoringzone.ClassCastExceptionon cross-type conversions (e.g.,getDate/getTimeonDATETIMEorTIMESTAMPcolumns):BigQueryTypeRegistry.convert(input, bqType, targetClass, zoneId)dispatched solely bybqType, returning aTimestampforDATETIMEandTIMESTAMPcolumns even whentargetClasswasDate.classorTime.class.Changes
getObject(int columnIndex, ZoneId zone)toBigQueryBaseResultSet,BigQueryJsonResultSet, andBigQueryArrowResultSetso theCalendar'sZoneIdis applied during initial unboxing for zone-less types (DATE,TIME,DATETIME), while continuing to ignoreCalendaronTIMESTAMPcolumns per the JDBC specification.boxDate(LocalDate, ZoneId),boxDateTime(LocalDateTime, ZoneId), andboxTime(LocalTime, ZoneId)inBigQueryTemporalUtilityand wired them intoBigQueryTypeRegistry.BigQueryTypeRegistry.convert(Object, StandardSQLTypeName, Class<T>, ZoneId)to delegate to the target class descriptor when the column descriptor returns an intermediate type.BigQueryJsonResultSetTest,BigQueryArrowResultSetTest, andBigQueryTypeRegistryTest.