refactor(jdbc): migrate write path and metadata to TypeRegistry - #14061
refactor(jdbc): migrate write path and metadata to TypeRegistry#14061Neenu1995 wants to merge 4 commits into
Conversation
There was a problem hiding this comment.
Code Review
This pull request refactors the BigQuery JDBC driver by migrating type mapping logic from BigQueryJdbcTypeMappings to BigQueryTypeRegistry across several classes, including BigQueryCallableStatement, BigQueryParameterMetaData, and BigQueryResultSetMetadata. The review feedback highlights several critical improvement opportunities: correcting misplaced Javadoc comments in BigQueryTypeRegistry, resolving dead code caused by a redundant null check on a primitive return value in BigQueryParameterMetaData, and addressing lossy type conversions where converting to a JDBC type first before mapping to a Java class loses precision for types like GEOGRAPHY or JSON. It is recommended to introduce a direct toJavaClass(StandardSQLTypeName) method to preserve type safety and prevent potential null pointer exceptions.
|
/gemini review |
There was a problem hiding this comment.
Code Review
This pull request refactors the BigQuery JDBC driver's type mapping mechanism by migrating from BigQueryJdbcTypeMappings to a centralized BigQueryTypeRegistry. This change affects several classes, including BigQueryCallableStatement, BigQueryParameterMetaData, and BigQueryResultSetMetadata. The review feedback highlights several areas for improvement: first, expanding the JDBC_TO_JAVA_CLASS_MAP to include missing standard JDBC types (such as DECIMAL and CHAR) to prevent runtime exceptions; and second, removing redundant null checks and boxing operations across the codebase, as the newly introduced registry methods either return primitive types or are guaranteed to return non-null values.
b/538177258
This PR executes Part 1 of Phase 4 of the Type Registry consolidation.
It migrates all parameter binding (Write Path) and metadata sizing away from the legacy `BigQueryJdbcTypeMappings` and onto the unified `BigQueryTypeRegistry`.
Changes: