Enhanced ProviderSqlSource to support dynamic sqlSource. #1111

abel533 · 2017-10-08T14:07:01Z

Major changes:

Process the SQL with the languageDriver, sql statement that supports the <script> pattern.
The XXXProvider annotation adds the cacheSqlSource property to avoid duplicate calculations.

A more powerful general-purpose Dao.

Test cases demonstrate a simple usage, with reference to the basic principles, and can achieve a powerful general-purpose Dao.

Test

Test method 1

@Lang(XMLLanguageDriver.class)
@InsertProvider(type = OurSqlBuilder.class, method = "buildInsertSelective", cacheSqlSource = true)
void insertSelective(T entity);

This method dynamically generates the following SQL script:

<script>
    insert into users
    <trim prefix="(" suffix=")" suffixOverrides=",">
        <if test="id != null">id,</if>
        <if test="name != null">name,</if>
    </trim>

    <trim prefix="VALUES (" suffix=")" suffixOverrides=",">
        <if test="id != null">#{id} ,</if>
        <if test="name != null">#{name} ,</if>
    </trim>
</script>

The executed SQL is as follows:

DEBUG [main] - ==>  Preparing: insert into users ( id ) VALUES ( ? ) 
DEBUG [main] - ==> Parameters: 999(Integer)
DEBUG [main] - <==    Updates: 1
DEBUG [main] - ==>  Preparing: select * from users where id = ? 
DEBUG [main] - ==> Parameters: 999(Integer)
TRACE [main] - <==    Columns: ID, NAME, LOGICAL_DELETE
TRACE [main] - <==        Row: 999, null, FALSE
DEBUG [main] - <==      Total: 1

Test method 2

@Lang(XMLLanguageDriver.class)
@UpdateProvider(type= OurSqlBuilder.class, method= "buildUpdateSelective", cacheSqlSource = true)
void updateSelective(T entity);

This method dynamically generates the following SQL script:

<script>
    update users
    <set>
        <if test="id != null">id = #{id} ,</if>
        <if test="name != null">name = #{name} ,</if>
    </set>
    <!-- For simplicity, there is no @Id annotation here, using default id directly -->
    where id = #{id}
</script>

The executed SQL is as follows:

DEBUG [main] - ==>  Preparing: update users SET id = ? where id = ? 
DEBUG [main] - ==> Parameters: 999(Integer), 999(Integer)
DEBUG [main] - <==    Updates: 1

Test method 3

@Lang(XMLLanguageDriver.class)
@SelectProvider(type = OurSqlBuilder.class, method = "buildGetByEntityQuery", cacheSqlSource = true)
List<T> getByEntity(T entity);

This method dynamically generates the following SQL script:

<script>
    select * from users
    <where>
        <if test="id != null">and id = #{id} </if>
        <if test="name != null">and name = #{name} </if>
    </where>
</script>

The executed SQL is as follows:

DEBUG [main] - ==>  Preparing: select * from users WHERE name = ? 
DEBUG [main] - ==> Parameters: User4(String)
TRACE [main] - <==    Columns: ID, NAME, LOGICAL_DELETE
TRACE [main] - <==        Row: 4, User4, TRUE
DEBUG [main] - <==      Total: 1
DEBUG [main] - ==>  Preparing: select * from users WHERE id = ? 
DEBUG [main] - ==> Parameters: 1(Integer)
TRACE [main] - <==    Columns: ID, NAME, LOGICAL_DELETE
TRACE [main] - <==        Row: 1, User1, FALSE
DEBUG [main] - <==      Total: 1
DEBUG [main] - ==>  Preparing: select * from users WHERE id = ?and name = ? 
DEBUG [main] - ==> Parameters: 1(Integer), User4(String)
DEBUG [main] - <==      Total: 0

abel533 · 2017-10-08T14:18:23Z

@harawata
This feature is very useful and it is easy for developers to create the underlying framework. I hope it will be reviewed as soon as possible.

Thank you.

abel533 · 2017-10-09T01:07:45Z

There is a concurrent problem with the following code.

@Override
public BoundSql getBoundSql(Object parameterObject) {
   SqlSource sqlSource;
   if (this.sqlSource != null) {
       sqlSource = this.sqlSource;
   } else {
       sqlSource = createSqlSource(parameterObject);
       if(this.cacheSqlSource){
           this.sqlSource = sqlSource;
       }
   }
   return sqlSource.getBoundSql(parameterObject);
}

Since it was previously executed repeatedly, there is no problem with the repeated execution.

There is no need to consider the lock.

kazuki43zoo · 2017-10-09T08:23:39Z

Hi @abel533 , Thanks for your contribution!

I have an interesting for this PR. However, I will afraid to break a backward compatibility for the current version.
In this changes(Change to use the LanguageDriver for creating SqlSource), I think there is a possibility that there will be a difference in behavior with the current version when using the LanguageDriver provided from other than MyBatis Core.
If there is a possibility that not keep a backward compatibility, I think better it should not be included at maintenance release for 3.4.x line.
What do you think about this topic?

Note: The Mybatis project provides two artifacts (velocity-scripting and freemarker-scripting) as related project.

kazuki43zoo

I've added some comments.

kazuki43zoo · 2017-10-09T08:27:26Z

src/main/java/org/apache/ibatis/builder/annotation/ProviderSqlSource.java

@@ -54,11 +56,19 @@ public ProviderSqlSource(Configuration configuration, Object provider) {
   * @since 3.4.5
   */
  public ProviderSqlSource(Configuration configuration, Object provider, Class<?> mapperType, Method mapperMethod) {
+    this(configuration, provider, mapperType, mapperMethod, new XMLLanguageDriver());


I think better using the Configuration#getDefaultScriptingLanguageInstance() instead of new XMLLanguageDriver().

kazuki43zoo · 2017-10-09T08:38:07Z

src/main/java/org/apache/ibatis/annotations/DeleteProvider.java

@@ -31,4 +31,6 @@
  Class<?> type();

  String method();
+
+  boolean cacheSqlSource() default false;


Please add API documentation(JavaDoc).
I think that we should explain an effect and caution for this attribute.

@kazuki43zoo This attribute is used to configure whether to cache SqlSource and avoid generating SqlSource repeatedly. is this name appropriate?

abel533 · 2017-10-09T08:50:13Z

By default, RawLanguageDriver and XMLLanguageDriver are treated in the same way as the original ProviderSqlSource without <script>.

In order to be compatible with the other two LanguageDriver, can I first judge languagedriver instanceof XMLLanguageDriver? If false, use the default XMLLanguageDriver.

abel533 · 2017-10-09T08:50:45Z

I will refer to the above comment to modify.

abel533 · 2017-10-09T09:00:25Z

src/main/java/org/apache/ibatis/builder/annotation/ProviderSqlSource.java

    String providerMethodName;
    try {
      this.configuration = configuration;
-      this.sqlSourceParser = new SqlSourceBuilder(configuration);
+      this.languageDriver = languageDriver;


@kazuki43zoo In order to be compatible with the other two LanguageDriver, can I judge languagedriver instanceof XMLLanguageDriver here? If false, use the default XMLLanguageDriver.

abel533 · 2017-10-09T12:59:55Z

@kazuki43zoo @harawata I have changed the source code.

I use rebase to merge the commit, so the review information lost.

Do I need rebase or commit directly ?

h3adache · 2017-10-10T12:21:33Z

src/main/java/org/apache/ibatis/builder/annotation/ProviderSqlSource.java

+      if(languageDriver instanceof XMLLanguageDriver){
+        this.languageDriver = languageDriver;
+      } else {
+        this.languageDriver = new XMLLanguageDriver();


Is there a reason to force languageDriver to XMLLanguageDriver?

@h3adache Velocity and Freemarker's languageDriver do not support StaticSqlSource.

When call the configuration.getdefaultscriptinglanguageinstance( ) method, it is possible to return velocity and freemarker's the languageDriver.

The main purpose is to be compatible with the original method of ProviderSqlSource.

I would prefer that you leave those to the implementations and not take the current implementations. This is a hidden behavior that might surprise people who try to use this.

@h3adache I submitted a new PR: #1120

abel533 · 2017-10-15T07:25:52Z

@kazuki43zoo @h3adache @harawata Any other questions?

I can change it.

abel533 force-pushed the master branch 2 times, most recently from 49d8cc5 to 737fc80 Compare October 8, 2017 14:15

abel533 mentioned this pull request Oct 9, 2017

通用 Mapper 4.0 计划 abel533/Mapper#100

Closed

kazuki43zoo requested changes Oct 9, 2017

View reviewed changes

kazuki43zoo requested a review from harawata October 9, 2017 08:41

abel533 commented Oct 9, 2017

View reviewed changes

Enhanced ProviderSqlSource to support dynamic sqlSource.

bcfe480

abel533 force-pushed the master branch from 737fc80 to bcfe480 Compare October 9, 2017 12:52

h3adache reviewed Oct 10, 2017

View reviewed changes

abel533 mentioned this pull request Oct 22, 2017

Support for customized ProviderSqlSource. #1120

Closed

abel533 mentioned this pull request Mar 24, 2018

Support for customized ProviderSqlSource. #1226

Closed

abel533 closed this Mar 25, 2018

abel533 mentioned this pull request Dec 3, 2018

Add LanguageDriver to ProviderSqlSource #1391

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Enhanced ProviderSqlSource to support dynamic sqlSource. #1111

Enhanced ProviderSqlSource to support dynamic sqlSource. #1111

abel533 commented Oct 8, 2017

abel533 commented Oct 8, 2017

abel533 commented Oct 9, 2017

kazuki43zoo commented Oct 9, 2017

kazuki43zoo left a comment

kazuki43zoo Oct 9, 2017

kazuki43zoo Oct 9, 2017

abel533 Oct 9, 2017 •

edited

Loading

abel533 commented Oct 9, 2017

abel533 commented Oct 9, 2017

abel533 Oct 9, 2017

abel533 commented Oct 9, 2017

h3adache Oct 10, 2017

abel533 Oct 10, 2017

h3adache Oct 16, 2017

abel533 Oct 22, 2017

abel533 commented Oct 15, 2017

Enhanced ProviderSqlSource to support dynamic sqlSource. #1111

Enhanced ProviderSqlSource to support dynamic sqlSource. #1111

Conversation

abel533 commented Oct 8, 2017

Major changes:

A more powerful general-purpose Dao.

Test

Test method 1

Test method 2

Test method 3

abel533 commented Oct 8, 2017

abel533 commented Oct 9, 2017

kazuki43zoo commented Oct 9, 2017

kazuki43zoo left a comment

Choose a reason for hiding this comment

kazuki43zoo Oct 9, 2017

Choose a reason for hiding this comment

kazuki43zoo Oct 9, 2017

Choose a reason for hiding this comment

abel533 Oct 9, 2017 • edited Loading

Choose a reason for hiding this comment

abel533 commented Oct 9, 2017

abel533 commented Oct 9, 2017

abel533 Oct 9, 2017

Choose a reason for hiding this comment

abel533 commented Oct 9, 2017

h3adache Oct 10, 2017

Choose a reason for hiding this comment

abel533 Oct 10, 2017

Choose a reason for hiding this comment

h3adache Oct 16, 2017

Choose a reason for hiding this comment

abel533 Oct 22, 2017

Choose a reason for hiding this comment

abel533 commented Oct 15, 2017

abel533 Oct 9, 2017 •

edited

Loading