Skip to content

Support to assign single primitive parameter on SQL provider method #1595

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

Closed
wants to merge 8 commits into from

Conversation

masssh
Copy link
Contributor

@masssh masssh commented Jul 7, 2019

Fix a bug in ProviderSqlSource.
A method with single primitive argument results in BuilderException.

Example (kotlin):

@Mapper
interface AuthorDao {

    @SelectProvider(type = SqlProvider::class)
    fun fetchOneById(id: Long): AuthorRow

    class SqlProvider : ProviderMethodResolver {
        companion object {

            @JvmStatic
            fun fetchOneById(id: Long): String {
                return """
                    SELECT * FROM author WHERE id = $id
                """.trimIndent()
            }

        }
    }

}

If AuthorDao#fetchOneById() would be called like authorDao.fetchOneById(10), following exception will occur.

Caused by: org.apache.ibatis.builder.BuilderException: Error invoking SqlProvider method (com.khloe.web.dao.AuthorDao$SqlProvider.fetchOneById). Cannot invoke a method that holds named argument(@Param) using a specifying parameterObject. In this case, please specify a 'java.util.Map' object.
	at org.apache.ibatis.builder.annotation.ProviderSqlSource.createSqlSource(ProviderSqlSource.java:132) ~[mybatis-3.5.1.jar:3.5.1]
	at org.apache.ibatis.builder.annotation.ProviderSqlSource.getBoundSql(ProviderSqlSource.java:109) ~[mybatis-3.5.1.jar:3.5.1]
	at org.apache.ibatis.mapping.MappedStatement.getBoundSql(MappedStatement.java:297) ~[mybatis-3.5.1.jar:3.5.1]
	at org.apache.ibatis.executor.CachingExecutor.query(CachingExecutor.java:81) ~[mybatis-3.5.1.jar:3.5.1]
	at org.apache.ibatis.session.defaults.DefaultSqlSession.selectList(DefaultSqlSession.java:147) ~[mybatis-3.5.1.jar:3.5.1]
	at org.apache.ibatis.session.defaults.DefaultSqlSession.selectList(DefaultSqlSession.java:140) ~[mybatis-3.5.1.jar:3.5.1]
	at org.apache.ibatis.session.defaults.DefaultSqlSession.selectOne(DefaultSqlSession.java:76) ~[mybatis-3.5.1.jar:3.5.1]
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method) ~[na:na]
	at java.base/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:62) ~[na:na]
	at java.base/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43) ~[na:na]
	at java.base/java.lang.reflect.Method.invoke(Method.java:566) ~[na:na]
	at org.mybatis.spring.SqlSessionTemplate$SqlSessionInterceptor.invoke(SqlSessionTemplate.java:433) ~[mybatis-spring-2.0.1.jar:2.0.1]
	... 120 common frames omitted

Copy link
Member

@kazuki43zoo kazuki43zoo left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi @masssh ,
Thank you for your reporting and PR!! I've add some comments. Please check it.

@harawata
Please comment if there is problem!

@kazuki43zoo kazuki43zoo added the bug label Jul 8, 2019
@kazuki43zoo kazuki43zoo self-assigned this Jul 8, 2019
@kazuki43zoo kazuki43zoo added this to the 3.5.2 milestone Jul 8, 2019
@kazuki43zoo kazuki43zoo changed the title Add support for single primitive parameter assignment for ProviderSqlSource Support to assign single primitive parameter on SQL provider method Jul 8, 2019
@kazuki43zoo kazuki43zoo requested a review from harawata July 8, 2019 22:52
@harawata
Copy link
Member

harawata commented Jul 9, 2019

Thanks for the PR, @masssh !

As Mehtod#invoke() throws a decent exception when wrong arguments are passed, I think there is a way to solve this without declaring primitive=wrapper-type mapping.

@kazuki43zoo ,
The below is a rough rewrite of the part invoking provider method in createSqlSource().
Could you take a look and see if there is a possibility?

if (parameterObject instanceof ParamMap) {
  if (bindParameterCount == 1 && Map.class.equals(providerMethodParameterTypes[0])) {
    sql = invokeProviderMethod(parameterObject); // need to add provider context?
  } else {
    @SuppressWarnings("unchecked")
    Map<String, Object> params = (Map<String, Object>) parameterObject;
    sql = invokeProviderMethod(extractProviderMethodArguments(params, providerMethodArgumentNames));
  }
} else if (providerMethodParameterTypes.length == 0) {
  sql = invokeProviderMethod();
} else if (providerMethodParameterTypes.length == 1) {
  if (providerContext == null) {
    sql = invokeProviderMethod(parameterObject);
  } else {
    sql = invokeProviderMethod(providerContext);
  }
} else if (providerMethodParameterTypes.length == 2) {
  sql = invokeProviderMethod(extractProviderMethodArguments(parameterObject));
} else {
  // provider method has too many args

@kazuki43zoo
Copy link
Member

Hi @harawata , Thanks for suggestion!! I will confirm your suggestion at tonight :D

@masssh
Copy link
Contributor Author

masssh commented Jul 9, 2019

Hi, Thanks for your reviewing, @kazuki43zoo , @harawata !!
For now, I just wrote some patch to improve my code ( not contain the suggestion by @harawata yet... )
Please check it! :)

@kazuki43zoo
Copy link
Member

Hi @masssh, thanks for your rework!! I will consider to apply @harawata 's suggestion. Please wait a just moment.

kazuki43zoo added a commit to kazuki43zoo/mybatis-3 that referenced this pull request Jul 13, 2019
kazuki43zoo added a commit that referenced this pull request Jul 13, 2019
 Support to assign single primitive parameter on SQL provider method
@kazuki43zoo kazuki43zoo removed the bug label Jul 13, 2019
@kazuki43zoo kazuki43zoo removed this from the 3.5.2 milestone Jul 13, 2019
@kazuki43zoo kazuki43zoo removed their assignment Jul 13, 2019
@kazuki43zoo
Copy link
Member

@masssh I've fixed via #1604 at now. Thanks for your contributing!!

pulllock pushed a commit to pulllock/mybatis-3 that referenced this pull request Oct 19, 2023
pulllock pushed a commit to pulllock/mybatis-3 that referenced this pull request Oct 19, 2023
 Support to assign single primitive parameter on SQL provider method
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants