Repository: alibaba/fescar
Pull request #229 (2019-01-23 02:35:29)

fixs #227 
<!-- Please make sure you have read and understood the contributing guidelines -->

### Ⅰ. Describe what this PR did


### Ⅱ. Does this pull request fix one issue?
<!-- If that, add "fixes #xxx" below in the next line, for example, fixes #97. -->

fixs #227 

### Ⅲ. Why don't you add test cases (unit test/integration test)? 


### Ⅳ. Describe how to verify it


### Ⅴ. Special notes for reviews



codecov-io: # [Codecov](https://codecov.io/gh/alibaba/fescar/pull/229?src=pr&el=h1) Report
> Merging [#229](https://codecov.io/gh/alibaba/fescar/pull/229?src=pr&el=desc) into [develop](https://codecov.io/gh/alibaba/fescar/commit/d1afdaa70afd9b649472c2f16e85a172c06aad9f?src=pr&el=desc) will **increase** coverage by `0.17%`.
> The diff coverage is `11.76%`.

[![Impacted file tree graph](https://codecov.io/gh/alibaba/fescar/pull/229/graphs/tree.svg?width=650&token=Hy9a6MCc4p&height=150&src=pr)](https://codecov.io/gh/alibaba/fescar/pull/229?src=pr&el=tree)

```diff
@@            Coverage Diff             @@
##           develop    #229      +/-   ##
==========================================
+ Coverage     8.21%   8.38%   +0.17%     
==========================================
  Files          184     184              
  Lines         6770    6772       +2     
  Branches       931     933       +2     
==========================================
+ Hits           556     568      +12     
+ Misses        6163    6147      -16     
- Partials        51      57       +6
```


| [Impacted Files](https://codecov.io/gh/alibaba/fescar/pull/229?src=pr&el=tree) | Coverage Δ | |
|---|---|---|
| [...aba/fescar/rm/datasource/exec/ExecuteTemplate.java](https://codecov.io/gh/alibaba/fescar/pull/229/diff?src=pr&el=tree#diff-cm0tZGF0YXNvdXJjZS9zcmMvbWFpbi9qYXZhL2NvbS9hbGliYWJhL2Zlc2Nhci9ybS9kYXRhc291cmNlL2V4ZWMvRXhlY3V0ZVRlbXBsYXRlLmphdmE=) | `0% <0%> (ø)` | :arrow_up: |
| [...ibaba/fescar/rm/datasource/exec/PlainExecutor.java](https://codecov.io/gh/alibaba/fescar/pull/229/diff?src=pr&el=tree#diff-cm0tZGF0YXNvdXJjZS9zcmMvbWFpbi9qYXZhL2NvbS9hbGliYWJhL2Zlc2Nhci9ybS9kYXRhc291cmNlL2V4ZWMvUGxhaW5FeGVjdXRvci5qYXZh) | `0% <0%> (ø)` | :arrow_up: |
| [...ba/fescar/rm/datasource/sql/SQLVisitorFactory.java](https://codecov.io/gh/alibaba/fescar/pull/229/diff?src=pr&el=tree#diff-cm0tZGF0YXNvdXJjZS9zcmMvbWFpbi9qYXZhL2NvbS9hbGliYWJhL2Zlc2Nhci9ybS9kYXRhc291cmNlL3NxbC9TUUxWaXNpdG9yRmFjdG9yeS5qYXZh) | `33.33% <66.66%> (+33.33%)` | :arrow_up: |
| [...urce/sql/druid/MySQLSelectForUpdateRecognizer.java](https://codecov.io/gh/alibaba/fescar/pull/229/diff?src=pr&el=tree#diff-cm0tZGF0YXNvdXJjZS9zcmMvbWFpbi9qYXZhL2NvbS9hbGliYWJhL2Zlc2Nhci9ybS9kYXRhc291cmNlL3NxbC9kcnVpZC9NeVNRTFNlbGVjdEZvclVwZGF0ZVJlY29nbml6ZXIuamF2YQ==) | `6.66% <0%> (+6.66%)` | :arrow_up: |
| [...fescar/rm/datasource/sql/druid/BaseRecognizer.java](https://codecov.io/gh/alibaba/fescar/pull/229/diff?src=pr&el=tree#diff-cm0tZGF0YXNvdXJjZS9zcmMvbWFpbi9qYXZhL2NvbS9hbGliYWJhL2Zlc2Nhci9ybS9kYXRhc291cmNlL3NxbC9kcnVpZC9CYXNlUmVjb2duaXplci5qYXZh) | `50% <0%> (+50%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/alibaba/fescar/pull/229?src=pr&el=continue).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/alibaba/fescar/pull/229?src=pr&el=footer). Last update [d1afdaa...3c5ef32](https://codecov.io/gh/alibaba/fescar/pull/229?src=pr&el=lastupdated). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments).


Patch:
diff --git a/rm-datasource/src/main/java/com/alibaba/fescar/rm/datasource/exec/ExecuteTemplate.java b/rm-datasource/src/main/java/com/alibaba/fescar/rm/datasource/exec/ExecuteTemplate.java
index 17eab9dea..46375e7f9 100644
--- a/rm-datasource/src/main/java/com/alibaba/fescar/rm/datasource/exec/ExecuteTemplate.java
+++ b/rm-datasource/src/main/java/com/alibaba/fescar/rm/datasource/exec/ExecuteTemplate.java
@@ -48,22 +48,26 @@ public class ExecuteTemplate {
                     statementProxy.getConnectionProxy().getDbType());
         }
         Executor<T> executor = null;
-        switch (sqlRecognizer.getSQLType()) {
-            case INSERT:
-                executor = new InsertExecutor<T, S>(statementProxy, statementCallback, sqlRecognizer);
-                break;
-            case UPDATE:
-                executor = new UpdateExecutor<T, S>(statementProxy, statementCallback, sqlRecognizer);
-                break;
-            case DELETE:
-                executor = new DeleteExecutor<T, S>(statementProxy, statementCallback, sqlRecognizer);
-                break;
-            case SELECT_FOR_UPDATE:
-                executor = new SelectForUpdateExecutor(statementProxy, statementCallback, sqlRecognizer);
-                break;
-            default:
-                executor = new PlainExecutor<T, S>(statementProxy, statementCallback, sqlRecognizer);
-                break;
+        if (sqlRecognizer == null) {
+            executor = new PlainExecutor<T, S>(statementProxy, statementCallback);
+        } else {
+            switch (sqlRecognizer.getSQLType()) {
+                case INSERT:
+                    executor = new InsertExecutor<T, S>(statementProxy, statementCallback, sqlRecognizer);
+                    break;
+                case UPDATE:
+                    executor = new UpdateExecutor<T, S>(statementProxy, statementCallback, sqlRecognizer);
+                    break;
+                case DELETE:
+                    executor = new DeleteExecutor<T, S>(statementProxy, statementCallback, sqlRecognizer);
+                    break;
+                case SELECT_FOR_UPDATE:
+                    executor = new SelectForUpdateExecutor(statementProxy, statementCallback, sqlRecognizer);
+                    break;
+                default:
+                    executor = new PlainExecutor<T, S>(statementProxy, statementCallback);
+                    break;
+            }
         }
         T rs = null;
         try {
diff --git a/rm-datasource/src/main/java/com/alibaba/fescar/rm/datasource/exec/PlainExecutor.java b/rm-datasource/src/main/java/com/alibaba/fescar/rm/datasource/exec/PlainExecutor.java
index 14a2f3455..db7f4c1ce 100644
--- a/rm-datasource/src/main/java/com/alibaba/fescar/rm/datasource/exec/PlainExecutor.java
+++ b/rm-datasource/src/main/java/com/alibaba/fescar/rm/datasource/exec/PlainExecutor.java
@@ -27,12 +27,9 @@ public class PlainExecutor<T, S extends Statement> implements Executor {
 
     private StatementCallback<T, S> statementCallback;
 
-    private SQLRecognizer sqlRecognizer;
-
-    public PlainExecutor(StatementProxy<S> statementProxy, StatementCallback<T, S> statementCallback, SQLRecognizer sqlRecognizer) {
+    public PlainExecutor(StatementProxy<S> statementProxy, StatementCallback<T, S> statementCallback) {
         this.statementProxy = statementProxy;
         this.statementCallback = statementCallback;
-        this.sqlRecognizer = sqlRecognizer;
     }
 
     @Override
diff --git a/rm-datasource/src/main/java/com/alibaba/fescar/rm/datasource/sql/SQLVisitorFactory.java b/rm-datasource/src/main/java/com/alibaba/fescar/rm/datasource/sql/SQLVisitorFactory.java
index 3976064c4..c36d4bb18 100644
--- a/rm-datasource/src/main/java/com/alibaba/fescar/rm/datasource/sql/SQLVisitorFactory.java
+++ b/rm-datasource/src/main/java/com/alibaba/fescar/rm/datasource/sql/SQLVisitorFactory.java
@@ -35,7 +35,7 @@ public class SQLVisitorFactory {
     public static SQLRecognizer get(String sql, String dbType) {
         List<SQLStatement> asts = SQLUtils.parseStatements(sql, dbType);
         if (asts == null || asts.size() != 1) {
-            throw new UnsupportedOperationException("xxx");
+            throw new UnsupportedOperationException("Unsupported SQL: " + sql);
         }
         SQLRecognizer recognizer = null;
         SQLStatement ast = asts.get(0);
@@ -47,11 +47,14 @@ public class SQLVisitorFactory {
             } else if (ast instanceof SQLDeleteStatement) {
                 recognizer = new MySQLDeleteRecognizer(sql, ast);
             } else if (ast instanceof SQLSelectStatement) {
-                recognizer = new MySQLSelectForUpdateRecognizer(sql, ast);
+                if (((SQLSelectStatement)ast).getSelect().getQueryBlock().isForUpdate()) {
+                    recognizer = new MySQLSelectForUpdateRecognizer(sql, ast);
+                }
             }
         } else {
             throw new UnsupportedOperationException("Just support MySQL by now!");
         }
+
         return recognizer;
     }
 }
diff --git a/spring/src/main/java/com/alibaba/fescar/spring/annotation/GlobalTransactionScanner.java b/spring/src/main/java/com/alibaba/fescar/spring/annotation/GlobalTransactionScanner.java
index 3b0451b69..24f9edecf 100644
--- a/spring/src/main/java/com/alibaba/fescar/spring/annotation/GlobalTransactionScanner.java
+++ b/spring/src/main/java/com/alibaba/fescar/spring/annotation/GlobalTransactionScanner.java
@@ -26,6 +26,7 @@ import com.alibaba.fescar.common.exception.NotSupportYetException;
 import com.alibaba.fescar.config.ConfigurationFactory;
 import com.alibaba.fescar.rm.RMClientAT;
 import com.alibaba.fescar.tm.TMClient;
+import com.alibaba.fescar.tm.api.DefaultFailureHandlerImpl;
 import com.alibaba.fescar.tm.api.FailureHandler;
 
 import org.apache.commons.lang.StringUtils;
@@ -56,8 +57,10 @@ public class GlobalTransactionScanner extends AbstractAutoProxyCreator implement
     private static final int MT_MODE = 2;
 
     private static final int ORDER_NUM = 1024;
+    private static final int DEFAULT_MODE = AT_MODE;
 
     private static final Set<String> PROXYED_SET = new HashSet<>();
+    private static final FailureHandler DEFAULT_FAIL_HANDLER = new DefaultFailureHandlerImpl();
 
     private GlobalTransactionalInterceptor interceptor;
 
@@ -67,8 +70,6 @@ public class GlobalTransactionScanner extends AbstractAutoProxyCreator implement
     private final boolean disableGlobalTransaction =
         ConfigurationFactory.getInstance().getBoolean("service.disableGlobalTransaction", false);
 
-    private static final int DEFAULT_MODE = AT_MODE;
-
     private final FailureHandler failureHandlerHook;
 
     /**
@@ -108,7 +109,7 @@ public class GlobalTransactionScanner extends AbstractAutoProxyCreator implement
      * @param mode           the mode
      */
     public GlobalTransactionScanner(String applicationId, String txServiceGroup, int mode) {
-        this(applicationId, txServiceGroup, mode, null);
+        this(applicationId, txServiceGroup, mode, DEFAULT_FAIL_HANDLER);
     }
 
     /**
diff --git a/spring/src/main/java/com/alibaba/fescar/spring/annotation/GlobalTransactionalInterceptor.java b/spring/src/main/java/com/alibaba/fescar/spring/annotation/GlobalTransactionalInterceptor.java
index e3a7cbbda..84864b611 100644
--- a/spring/src/main/java/com/alibaba/fescar/spring/annotation/GlobalTransactionalInterceptor.java
+++ b/spring/src/main/java/com/alibaba/fescar/spring/annotation/GlobalTransactionalInterceptor.java
@@ -33,13 +33,14 @@ import org.slf4j.LoggerFactory;
 public class GlobalTransactionalInterceptor implements MethodInterceptor {
 
     private static final Logger LOGGER = LoggerFactory.getLogger(GlobalTransactionalInterceptor.class);
+    private static final FailureHandler DEFAULT_FAIL_HANDLER = new DefaultFailureHandlerImpl();
 
     private final TransactionalTemplate transactionalTemplate = new TransactionalTemplate();
     private final FailureHandler failureHandler;
 
     public GlobalTransactionalInterceptor(FailureHandler failureHandler) {
         if (null == failureHandler) {
-            failureHandler = new DefaultFailureHandlerImpl();
+            failureHandler = DEFAULT_FAIL_HANDLER;
         }
         this.failureHandler = failureHandler;
     }


Test patch:
diff --git a/rm-datasource/src/test/java/com/alibaba/fescar/rm/datasource/SQLVisitorFactoryTest.java b/rm-datasource/src/test/java/com/alibaba/fescar/rm/datasource/SQLVisitorFactoryTest.java
new file mode 100644
index 000000000..e578f4377
--- /dev/null
+++ b/rm-datasource/src/test/java/com/alibaba/fescar/rm/datasource/SQLVisitorFactoryTest.java
@@ -0,0 +1,23 @@
+package com.alibaba.fescar.rm.datasource;
+
+import com.alibaba.druid.util.JdbcConstants;
+import com.alibaba.fescar.rm.datasource.sql.SQLRecognizer;
+import com.alibaba.fescar.rm.datasource.sql.SQLVisitorFactory;
+import com.alibaba.fescar.rm.datasource.sql.druid.MySQLSelectForUpdateRecognizer;
+import org.junit.Assert;
+import org.junit.Test;
+
+public class SQLVisitorFactoryTest {
+
+    @Test
+    public void testSqlRecognizing() {
+        String dbType = JdbcConstants.MYSQL;
+        String sql = "select a, b, c from t1 where id = 9";
+        SQLRecognizer recognizer = SQLVisitorFactory.get(sql, dbType);
+        Assert.assertNull(recognizer);
+
+        sql = sql + " for update";
+        recognizer = SQLVisitorFactory.get(sql, dbType);
+        Assert.assertTrue(recognizer instanceof MySQLSelectForUpdateRecognizer);
+    }
+}