Skip to content
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

bugfix: update executor store the actually modified columns but not only the columns in set condition #4253

Merged
merged 6 commits into from
Mar 7, 2022

Conversation

caohdgege
Copy link
Contributor

  • I have registered the PR changes.

Ⅰ. Describe what this PR did

Ⅱ. Does this pull request fix one issue?

fixes #3822
fixes #3036

Ⅲ. Why don't you add test cases (unit test/integration test)?

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

@caohdgege caohdgege added the Do Not Merge Do not merge into develop label Jan 1, 2022
@codecov-commenter
Copy link

codecov-commenter commented Jan 1, 2022

Codecov Report

Merging #4253 (018df45) into develop (40e761c) will decrease coverage by 0.00%.
The diff coverage is 42.85%.

Impacted file tree graph

@@              Coverage Diff              @@
##             develop    #4253      +/-   ##
=============================================
- Coverage      48.33%   48.33%   -0.01%     
- Complexity      3969     3973       +4     
=============================================
  Files            734      734              
  Lines          25279    25300      +21     
  Branches        3105     3109       +4     
=============================================
+ Hits           12218    12228      +10     
- Misses         11740    11748       +8     
- Partials        1321     1324       +3     
Impacted Files Coverage Δ
.../io/seata/rm/datasource/sql/struct/ColumnMeta.java 95.20% <33.33%> (-3.12%) ⬇️
...va/io/seata/rm/datasource/exec/UpdateExecutor.java 71.83% <40.00%> (-4.84%) ⬇️
...tasource/sql/struct/cache/MysqlTableMetaCache.java 80.00% <50.00%> (-1.61%) ⬇️
...a/io/seata/rm/datasource/sql/struct/TableMeta.java 96.42% <100.00%> (+0.06%) ⬆️
...in/java/io/seata/server/session/GlobalSession.java 79.60% <0.00%> (+0.39%) ⬆️

@caohdgege caohdgege changed the title bugfix: could not rollback when update rows after insert rows in one global transaction bugfix: update executor store the actually modified columns but not only the columns in set condition Jan 1, 2022
@funky-eyes funky-eyes added this to the 1.5.0 milestone Jan 22, 2022
@funky-eyes funky-eyes added module/rm-datasource rm-datasource module type: optimize and removed Do Not Merge Do not merge into develop labels Jan 22, 2022
Copy link
Contributor

@funky-eyes funky-eyes left a comment

Choose a reason for hiding this comment

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

LGTM pr登记一下

Copy link
Contributor

@l81893521 l81893521 left a comment

Choose a reason for hiding this comment

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

Which version of mysql are tested?

@caohdgege
Copy link
Contributor Author

Which version of mysql are tested?

这个在驱动层面就是通过show columns from {$tableName} where extra like '%on update CURRENT_TIMESTAMP%';去查询,只要查询到的数据就都是on update的列了。试了一下5.6,5.7,8.0的版本都是支持的。

Copy link
Contributor

@l81893521 l81893521 left a comment

Choose a reason for hiding this comment

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

LGTM

@funky-eyes funky-eyes merged commit 3d7a8b6 into apache:develop Mar 7, 2022
@@ -115,7 +115,8 @@ private TableMeta resultSetMetaToSchema(ResultSetMetaData rsmd, DatabaseMetaData
*/

try (ResultSet rsColumns = dbmd.getColumns(catalogName, schemaName, tableName, "%");
ResultSet rsIndex = dbmd.getIndexInfo(catalogName, schemaName, tableName, false, true)) {
ResultSet rsIndex = dbmd.getIndexInfo(catalogName, schemaName, tableName, false, true);
ResultSet onUpdateColumns = dbmd.getVersionColumns(catalogName, schemaName, tableName)) {
Copy link
Member

Choose a reason for hiding this comment

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

Whether virtual columns are included ?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
5 participants