chore(bigquery-jdbc): refactor PreparedStatement and CallableStatement#13849
chore(bigquery-jdbc): refactor PreparedStatement and CallableStatement#13849Neenu1995 wants to merge 8 commits into
Conversation
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request refactors parameter handling in BigQueryCallableStatement and BigQueryPreparedStatement by standardizing parameter names, adding checkClosed() validation, and introducing helper methods like getRawParameter. Feedback on these changes highlights a few inconsistencies: BigQueryCallableStatement.setDate uses convertDateToCalendar instead of convertDateWithCalendar, the overloaded getObject methods are missing checkClosed() checks, and temporal types are handled inconsistently between BigQueryPreparedStatement (bound as strings) and BigQueryCallableStatement (bound as raw temporal objects).
Neenu1995
changed the title
| @Override | ||
| public boolean getBoolean(int parameterIndex) throws SQLException { | ||
| Object param = this.parameterHandler.getParameter(parameterIndex); | ||
| checkClosed(); |
There was a problem hiding this comment.
can move checkClosed() to getRawParameter() since it is used in a lot of getters
| private final BigQueryJdbcCustomLogger LOG = new BigQueryJdbcCustomLogger(this.toString()); | ||
| private boolean lastReadWasNull = false; | ||
|
|
||
| private Object getRawParameter(int parameterIndex) { |
There was a problem hiding this comment.
We already have getObject that is doing exactly this, I'd suggest leverage it instead of extra overrides
| public void setTime(int parameterIndex, Time value) throws SQLException { | ||
| checkClosed(); | ||
| this.parameterHandler.setParameter(parameterIndex, x.toString(), String.class); | ||
| if (value == null) { |
There was a problem hiding this comment.
Why special-case for null? Can't setParameter handle it?
having is as special case creates potential mismatch between type used here & type used via setParameter for non-null
| if (type.isInstance(value)) { | ||
| return (T) value; | ||
| } | ||
| return null; |
There was a problem hiding this comment.
Seems like it should throw an exception for type mismatch?
2 participants
wasNull(): Routed all parameter getter paths inBigQueryCallableStatementthrough centralizedgetRawParameterhelpers, ensuring consistentwasNull()state tracking across all fast-path and fallback reads.checkClosed()checks to all implemented methods inBigQueryCallableStatementandBigQueryPreparedStatement.LOG.finerentry logging.getRef,getSQLXML) to directly throwBigQueryJdbcSqlFeatureNotSupportedException.