Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

fix: ResultSet#get(...) methods should auto convert values #143

Merged
merged 6 commits into from Jun 9, 2020

Conversation

olavloite
Copy link
Collaborator

@olavloite olavloite commented May 31, 2020

Methods like ResultSet#getString(int) and ResultSet#getInt(int) would only allow data to be fetched if the underlying data type matched the method that was invoked, i.e. calling ResultSet#getString(int) for a column containing an INT64 value would always fail. According to the JDBC specification it should however be allowed to call certain get... methods that automatically convert the underlying value to the requested type. The supported conversions will in some cases always be supported (e.g. INT64 to STRING), and in some cases the method may throw an exception if the actual value cannot be converted to the requested type (e.g. STRING to INT64).

This PR adds support for all implicit conversions for ResultSet#get... methods that according to the JDBC specification should be allowed. This also enables the use of the JDBC driver with frameworks that rely on these automatic conversions, such as MyBatis.

@googlebot googlebot added the cla: yes This human has signed the Contributor License Agreement. label May 31, 2020
@codecov
Copy link

codecov bot commented May 31, 2020

Codecov Report

Merging #143 into master will increase coverage by 0.24%.
The diff coverage is 83.70%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #143      +/-   ##
============================================
+ Coverage     71.95%   72.20%   +0.24%     
- Complexity     1027     1095      +68     
============================================
  Files            24       24              
  Lines          3156     3292     +136     
  Branches        463      495      +32     
============================================
+ Hits           2271     2377     +106     
- Misses          706      722      +16     
- Partials        179      193      +14     
Impacted Files Coverage Δ Complexity Δ
...m/google/cloud/spanner/jdbc/JdbcTypeConverter.java 73.79% <50.00%> (-1.07%) 91.00 <3.00> (+3.00) ⬇️
...a/com/google/cloud/spanner/jdbc/JdbcResultSet.java 78.81% <81.14%> (-3.04%) 186.00 <10.00> (+57.00) ⬇️
...google/cloud/spanner/jdbc/AbstractJdbcWrapper.java 77.31% <100.00%> (+13.31%) 51.00 <8.00> (+8.00)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 2a7c53e...e4fd1f9. Read the comment docs.

@olavloite olavloite requested a review from skuruppu June 1, 2020 11:10
@olavloite olavloite merged commit bc7d5bd into master Jun 9, 2020
@olavloite olavloite deleted the resultset-get-should-autoconvert branch June 9, 2020 14:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
cla: yes This human has signed the Contributor License Agreement.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants