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

Allow long type on size parameter when use multiple '@Param' #1031

Merged
merged 1 commit into from Jun 11, 2017

Conversation

Projects
None yet
2 participants
@kazuki43zoo
Member

kazuki43zoo commented Jun 11, 2017

I've fixed gh-1030.
In current implementation, mybatis use the field type of ParamMap#size(HashMap#size) instead of method argument type.

Please review this.

@harawata

This comment has been minimized.

Show comment
Hide comment
@harawata

harawata Jun 11, 2017

Member

There is a corner case that tha statement takes HashMap as a parameter.

@Insert("insert into param_test (id, size) values(#{id}, #{size})")
void insertHashMap(HashMap<String, Object> param);

I haven't had the time to test this, but I was going to fix it in SqlSourceBuilder actually.
Something like this.

         propertyType = parameterType;
       } else if (JdbcType.CURSOR.name().equals(propertiesMap.get("jdbcType"))) {
         propertyType = java.sql.ResultSet.class;
-      } else if (property != null) {
+      } else if (property == null || Map.class.isAssignableFrom(parameterType)) {
+        propertyType = Object.class;
+      } else {
         MetaClass metaClass = MetaClass.forClass(parameterType, configuration.getReflectorFactory());
         if (metaClass.hasGetter(property)) {
           propertyType = metaClass.getGetterType(property);
         } else {
           propertyType = Object.class;
         }
-      } else {
-        propertyType = Object.class;
       }
       ParameterMapping.Builder builder = new ParameterMapping.Builder(configuration, property, propertyType);
       Class<?> javaType = propertyType;
Member

harawata commented Jun 11, 2017

There is a corner case that tha statement takes HashMap as a parameter.

@Insert("insert into param_test (id, size) values(#{id}, #{size})")
void insertHashMap(HashMap<String, Object> param);

I haven't had the time to test this, but I was going to fix it in SqlSourceBuilder actually.
Something like this.

         propertyType = parameterType;
       } else if (JdbcType.CURSOR.name().equals(propertiesMap.get("jdbcType"))) {
         propertyType = java.sql.ResultSet.class;
-      } else if (property != null) {
+      } else if (property == null || Map.class.isAssignableFrom(parameterType)) {
+        propertyType = Object.class;
+      } else {
         MetaClass metaClass = MetaClass.forClass(parameterType, configuration.getReflectorFactory());
         if (metaClass.hasGetter(property)) {
           propertyType = metaClass.getGetterType(property);
         } else {
           propertyType = Object.class;
         }
-      } else {
-        propertyType = Object.class;
       }
       ParameterMapping.Builder builder = new ParameterMapping.Builder(configuration, property, propertyType);
       Class<?> javaType = propertyType;
@kazuki43zoo

This comment has been minimized.

Show comment
Hide comment
@kazuki43zoo

kazuki43zoo Jun 11, 2017

Member

@harawata Thanks. I agree with your comments. I will update this PR.

Member

kazuki43zoo commented Jun 11, 2017

@harawata Thanks. I agree with your comments. I will update this PR.

@kazuki43zoo kazuki43zoo changed the title from Allow long type on size parameter when use the `@Param` gh-1030 to Allow long type on size parameter when use multiple '@Param' Jun 11, 2017

@kazuki43zoo

This comment has been minimized.

Show comment
Hide comment
@kazuki43zoo

kazuki43zoo Jun 11, 2017

Member

I've updated this.

Please review again.

Member

kazuki43zoo commented Jun 11, 2017

I've updated this.

Please review again.

@harawata

This comment has been minimized.

Show comment
Hide comment
@harawata

harawata Jun 11, 2017

Member

Looks good. Thank you @kazuki43zoo !

Member

harawata commented Jun 11, 2017

Looks good. Thank you @kazuki43zoo !

@kazuki43zoo kazuki43zoo merged commit 3aae5f3 into mybatis:master Jun 11, 2017

1 check passed

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

@kazuki43zoo kazuki43zoo deleted the kazuki43zoo:gh-1030 branch Jun 11, 2017

@kazuki43zoo kazuki43zoo self-assigned this Jul 30, 2017

@kazuki43zoo kazuki43zoo added the bug label Jul 30, 2017

@kazuki43zoo kazuki43zoo added this to the 3.4.5 milestone Jul 30, 2017

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