-
Notifications
You must be signed in to change notification settings - Fork 8.8k
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
feature: support mysql update join sql #4914
feature: support mysql update join sql #4914
Conversation
支持语句:update employees inner join merit on employees.performance = merit.performance set salary = salary + salary * count !!!暂不支持Oracle和PG数据库update join语法 |
…' into feature_support_mysql_updatejoin
Codecov Report
@@ Coverage Diff @@
## develop #4914 +/- ##
=============================================
+ Coverage 48.62% 48.80% +0.17%
- Complexity 4052 4077 +25
=============================================
Files 732 733 +1
Lines 25773 25942 +169
Branches 3173 3197 +24
=============================================
+ Hits 12532 12660 +128
- Misses 11909 11933 +24
- Partials 1332 1349 +17
|
rm-datasource/src/main/java/io/seata/rm/datasource/exec/BaseTransactionalExecutor.java
Outdated
Show resolved
Hide resolved
rm-datasource/src/main/java/io/seata/rm/datasource/exec/BaseTransactionalExecutor.java
Outdated
Show resolved
Hide resolved
This pull request introduces 2 alerts when merging 2c73223 into d0f324c - view on LGTM.com new alerts:
|
This pull request introduces 2 alerts when merging f9314c6 into d0f324c - view on LGTM.com new alerts:
|
rm-datasource/src/main/java/io/seata/rm/datasource/exec/mysql/MySQLUpdateJoinExecutor.java
Show resolved
Hide resolved
rm-datasource/src/test/java/io/seata/rm/datasource/exec/UpdateJoinExecutorTest.java
Outdated
Show resolved
Hide resolved
@@ -134,28 +118,36 @@ private String buildAfterImageSQL(TableMeta tableMeta, TableRecords beforeImage) | |||
String whereSql = SqlGenerateUtils.buildWhereConditionByPKs(tableMeta.getPrimaryKeyOnlyName(), beforeImage.pkRows().size(), getDbType()); | |||
String suffix = " FROM " + getFromTableInSQL() + " WHERE " + whereSql; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FROM WHERE 是否可以考虑定义成静态常量来使用?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
FROM WHERE 是否可以考虑定义成静态常量来使用?
这个pr先不要定义了,刘洋有个pr专门搞这个的 @doubleDimple
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
这块已有的常量比如where可以直接使用,没有的就先不要专门弄了
rm-datasource/src/main/java/io/seata/rm/datasource/exec/mysql/MySQLUpdateJoinExecutor.java
Show resolved
Hide resolved
rm-datasource/src/main/java/io/seata/rm/datasource/exec/UpdateExecutor.java
Show resolved
Hide resolved
druidDataSource.setDriverClassName(mysql_driverClassName); | ||
druidDataSource.init(); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
本地执行下./mvnw clean install -DskipTests=true -Dcheckstyle.skip=false -Dlicense.skip=false
命令,再执行下代码提交,不然checks不通过
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
UpdateJoinExecutorTest.testUpdateJoinUndoLog:64 » NullPointer
This pull request introduces 2 alerts when merging c27ee3e into dac8aa8 - view on LGTM.com new alerts:
|
This pull request introduces 2 alerts when merging 02cb4fa into dac8aa8 - view on LGTM.com new alerts:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM Please register pr and author information
rm-datasource/src/main/java/io/seata/rm/datasource/exec/BaseTransactionalExecutor.java
Show resolved
Hide resolved
rm-datasource/src/main/java/io/seata/rm/datasource/exec/BaseTransactionalExecutor.java
Outdated
Show resolved
Hide resolved
SQLUpdateRecognizer recognizer = (SQLUpdateRecognizer) sqlRecognizer; | ||
String tableNames = recognizer.getTableName(); | ||
if (StringUtils.isEmpty(tableNames)) { | ||
return null; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
throw Exceptions with an explicit meaning, otherwise it will result in NPE with an unclear meaning
rm-datasource/src/main/java/io/seata/rm/datasource/exec/mysql/MySQLUpdateJoinExecutor.java
Show resolved
Hide resolved
|
||
@Override | ||
public boolean visit(SQLSubqueryTableSource x) { | ||
//just like: select * from (select * from t) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
select SQL statement that seata does not intercept.
* distributed under the License is distributed on an "AS IS" BASIS, | ||
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
* See the License for the specific language governing permissions and | ||
* limitations under the License. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please do not reformat the code that have nothing to do with this pr.
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
seata mysql8,test库中的test1,test2两张表执行如下语句无法回滚。
|
Ⅰ. Describe what this PR did
support mysql update join sql
Ⅱ. Does this pull request fix one issue?
fixes #4118
fixes #4890
Ⅲ. Why don't you add test cases (unit test/integration test)?
Ⅳ. Describe how to verify it
MysqlUpdateJoinTest有测试用例
Ⅴ. Special notes for reviews