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

Move wasNull to subclass from BaseTypeHandler #1242 #1244

Merged
merged 1 commit into from Apr 25, 2018

Conversation

Projects
None yet
3 participants
@kazuki43zoo
Copy link
Member

kazuki43zoo commented Apr 5, 2018

I've fixed #1242 based on the comment of #1243 by @harawata .

About backword compatility

In this change, type handlers provided by the mybatis core project keeps backward compatibility. But If you used the method that returned type is primitive(e.g. ResultSet#getInt) in your type handler, there is need a notice that it may be lost backward compatibility.
If you used the method that returned type is primitive, please consider to change as calling wasNull in your type handler.

@harawata

This comment has been minimized.

Copy link
Member

harawata commented Apr 5, 2018

You work fast 🙀
And it looks great. Thank you, @kazuki43zoo !

@mnesarco @h3adache Please comment if you guys noticed anything!

@kazuki43zoo kazuki43zoo force-pushed the kazuki43zoo:gh-1242_move-wasNull-to-subclass branch from 44cec15 to 9d6bd53 Apr 5, 2018

@kazuki43zoo kazuki43zoo added this to the 3.5.0 milestone Apr 6, 2018

@kazuki43zoo kazuki43zoo self-assigned this Apr 6, 2018

@@ -38,19 +38,19 @@ public void setNonNullParameter(PreparedStatement ps, int i, Object parameter, J
@Override
public Object getNullableResult(ResultSet rs, String columnName) throws SQLException {
Array array = rs.getArray(columnName);
return array == null ? null : array.getArray();
return (array == null || rs.wasNull()) ? null : array.getArray();

This comment has been minimized.

@h3adache

h3adache Apr 8, 2018

Member

A lot of your changes are because of this logic.
I'm curious what driver returns a non null for an object type but returns true for rs.wasNull()
I've only see it happen for primitive types but never for object types

This comment has been minimized.

@kazuki43zoo

kazuki43zoo Apr 8, 2018

Author Member

@h3adache , Thanks for your comment. You're right! I've confirmed the JDBC 4.2 specification, it says about NULL values as follow:

15.2.3.3 Retrieving NULL values
The method wasNull can be called to determine if the last value retrieved was a
SQL NULL in the database.
When the column value in the database is SQL NULL, it may be returned to the Java
application as null, 0, or false, depending on the type of the column value.
Column values that map to Java Object types are returned as a Java null; those
that map to numeric types are returned as 0; those that map to a Java boolean are
returned as false. Therefore, it may be necessary to call the wasNull method to
determine whether the last value retrieved was a SQL NULL.

I will modify it at tonight(JST).

@h3adache

This comment has been minimized.

Copy link
Member

h3adache commented Apr 9, 2018

Sorry I got pulled away before I finished the code review. Everything else looks good.
Thanks for the work @kazuki43zoo!

@kazuki43zoo kazuki43zoo force-pushed the kazuki43zoo:gh-1242_move-wasNull-to-subclass branch from 9d6bd53 to 28ddddd Apr 9, 2018

@kazuki43zoo kazuki43zoo force-pushed the kazuki43zoo:gh-1242_move-wasNull-to-subclass branch from 28ddddd to 2b68768 Apr 9, 2018

@kazuki43zoo

This comment has been minimized.

Copy link
Member Author

kazuki43zoo commented Apr 9, 2018

I've fixed @h3adache 's comment. (Revert some classes to original version)

@kazuki43zoo kazuki43zoo merged commit 7282f76 into mybatis:master Apr 25, 2018

1 check passed

continuous-integration/travis-ci/pr The Travis CI build passed
Details

@kazuki43zoo kazuki43zoo deleted the kazuki43zoo:gh-1242_move-wasNull-to-subclass branch Apr 25, 2018

@kazuki43zoo

This comment has been minimized.

Copy link
Member Author

kazuki43zoo commented Apr 25, 2018

I've merged this. If there is a problem, please give a comment.

@kazuki43zoo

This comment has been minimized.

Copy link
Member Author

kazuki43zoo commented Apr 25, 2018

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment