feat(spanner-jdbc): add support for INTERVAL data type - #14144
Conversation
There was a problem hiding this comment.
Code Review
This pull request adds support for the Spanner INTERVAL data type to the Spanner JDBC driver, including mapping to a custom SQL type, supporting conversions to and from Java's Duration and Period classes, updating metadata queries, and adding comprehensive tests. The review feedback highlights three critical issues: a catch-all ELSE 1111 fallback in the PostgreSQL metadata query that could break other unhandled types, an error-prone null check on the first element of a list in JdbcArray.java that could lead to runtime exceptions, and unhandled ArithmeticExceptions in JdbcTypeConverter.java when converting large intervals to Duration.
0b6e638 to
3af669d
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces support for the Spanner INTERVAL data type in the JDBC driver, including mapping to and from Java's Duration and Period classes, updating metadata, and adding comprehensive tests. The review feedback suggests addressing a potential INVALID_ARGUMENT exception when binding Spanner Value instances of type INTERVAL, replacing Java Streams with a simple loop in array conversions to avoid overhead and import issues, and explicitly handling integer overflow with a descriptive error message when converting Period to Interval.
3af669d to
187c7d7
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request adds support for the Spanner INTERVAL data type to the JDBC driver, enabling seamless integration and conversion between Spanner's INTERVAL and Java's java.time.Duration and java.time.Period classes. It updates metadata queries, parameter binding, result set retrieval, and type conversions, accompanied by comprehensive unit and mock server tests. The feedback recommends using try-with-resources for the Statement object in IntervalMockServerTest to prevent potential resource leaks.
187c7d7 to
e14b07c
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request introduces support for the Spanner INTERVAL data type in the Spanner JDBC driver, enabling mapping to and from Google Cloud Spanner's Interval, java.time.Duration, and java.time.Period types. The changes span across various JDBC wrapper, array, metadata, parameter, and result set classes, along with SQL metadata files and comprehensive test suites. The review feedback suggests improving JDBC compliance and error handling by throwing a checked SQLException instead of a runtime IllegalArgumentException when a Period's total months overflow the integer range in JdbcTypeConverter.toInterval(Period), along with updating the corresponding unit test.
Adds support for the Spanner INTERVAL data type to the Spanner JDBC driver. - Introduce IntervalType implementing java.sql.SQLType with vendor type number 100016. - Map INTERVAL in JdbcDataType, AbstractJdbcWrapper, and metadata queries (both GoogleSQL and PostgreSQL dialects). - Add getInterval(int/String) to JdbcResultSet with support for Interval and ISO 8601 strings. - Support java.time.Duration and java.time.Period in JdbcTypeConverter, JdbcResultSet (getObject), JdbcParameterStore (setObject/setArray), and JdbcArray, consistent with pgjdbc behavior. - Support typed null and parameterized statement bindings for Interval, Duration, and Period.
e14b07c to
61d4d20
Compare
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request adds comprehensive support for the Spanner INTERVAL data type to the Spanner JDBC driver. It introduces a new IntervalType class and updates core JDBC components—including wrappers, arrays, data types, parameter stores, result sets, and type converters—to handle Interval values and their arrays. Additionally, it supports conversions to and from java.time.Duration and java.time.Period (consistent with PostgreSQL JDBC behavior) and updates database metadata queries. Since there are no review comments provided, I have no additional feedback to offer.
Adds support for the Spanner INTERVAL data type to the Spanner JDBC driver.