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.

Sign up for GitHub

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

Jump to bottom

bugfix:throw exception when update pk #3287

Merged
merged 5 commits into from
Dec 8, 2020
Merged

bugfix:throw exception when update pk #3287

merged 5 commits into from
Dec 8, 2020

Conversation

0000005
Copy link
Contributor

@0000005 0000005 commented Nov 16, 2020

Ⅰ. Describe what this PR did

optimize this pr. #3129

Ⅱ. Does this pull request fix one issue?

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

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

@codecov-io
Copy link

codecov-io commented Nov 16, 2020 •
edited

Codecov Report

Merging #3287 (5fa764e) into develop (be83b1e) will decrease coverage by 0.02%.
The diff coverage is 33.33%.

Impacted file tree graph

@@              Coverage Diff              @@
##             develop    #3287      +/-   ##
=============================================
- Coverage      50.87%   50.85%   -0.03%     
+ Complexity      3280     3279       -1     
=============================================
  Files            612      612              
  Lines          20113    20111       -2     
  Branches        2519     2522       +3     
=============================================
- Hits           10233    10227       -6     
- Misses          8853     8854       +1     
- Partials        1027     1030       +3
Impacted Files Coverage Δ Complexity Δ
...ta/rm/datasource/exec/AbstractDMLBaseExecutor.java 60.37% <ø> (-0.64%) 11.00 <0.00> (-2.00)
.../seata/rm/datasource/exec/MultiUpdateExecutor.java 76.00% <ø> (-0.32%) 12.00 <0.00> (ø)
...va/io/seata/rm/datasource/exec/UpdateExecutor.java 75.40% <ø> (-0.40%) 11.00 <0.00> (ø)
.../rm/datasource/exec/BaseTransactionalExecutor.java 59.23% <33.33%> (-3.77%) 27.00 <0.00> (-1.00)
...ava/io/seata/rm/datasource/exec/MultiExecutor.java 82.60% <33.33%> (-3.44%) 11.00 <0.00> (+1.00) ⬇️
...o/seata/server/coordinator/DefaultCoordinator.java 54.63% <0.00%> (-0.52%) 29.00% <0.00%> (ø%)
...in/java/io/seata/server/session/GlobalSession.java 82.72% <0.00%> (-0.46%) 70.00% <0.00%> (-1.00%)
.../java/io/seata/server/coordinator/DefaultCore.java 50.00% <0.00%> (+0.60%) 24.00% <0.00%> (+1.00%)
...erver/storage/file/session/FileSessionManager.java 64.16% <0.00%> (+2.49%) 28.00% <0.00%> (+1.00%)

@0000005 0000005 changed the title bugfix:update pk throw exception bugfix:throw exception when update pk Nov 17, 2020
funky-eyes
funky-eyes requested changes Nov 17, 2020
View reviewed changes
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.

please use p3C to format the code

rm-datasource/src/main/java/io/seata/rm/datasource/exec/AbstractDMLBaseExecutor.java Outdated
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.sql.SQLException;
Copy link
Contributor

Choose a reason for hiding this comment

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

java packages should be at the top

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

@caohdgege
Copy link
Contributor

关联一下#3289

@xingfudeshi
Copy link
Member

This problem is that the primary key should not be changed. If the new primary key value is consistent with the old primary key value, can it be ignored directly?@81519434

@0000005
Copy link
Contributor Author

0000005 commented Nov 30, 2020

This problem is that the primary key should not be changed. If the new primary key value is consistent with the old primary key value, can it be ignored directly?@81519434

Yes, this pr is doing exactly what you said!

xingfudeshi
xingfudeshi approved these changes Nov 30, 2020
View reviewed changes
Copy link
Member

@xingfudeshi xingfudeshi 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 added this to the 1.5.0 milestone Dec 1, 2020
@funky-eyes funky-eyes added the module/rm-datasource rm-datasource module label Dec 1, 2020
slievrly
slievrly approved these changes Dec 8, 2020
View reviewed changes
Copy link
Member

@slievrly slievrly 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 approved these changes Dec 8, 2020
View reviewed changes
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

@slievrly slievrly merged commit c2c3488 into apache:develop Dec 8, 2020
@wangliang181230 wangliang181230 modified the milestones: 1.5.0, 1.4.1 Dec 11, 2020
@caohdgege caohdgege mentioned this pull request Jan 17, 2021
Closed
1 task
@zly123987123 zly123987123 mentioned this pull request Jan 1, 2024
Open
@zly123987123 zly123987123 mentioned this pull request Mar 15, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@slievrly slievrly slievrly approved these changes

@funky-eyes funky-eyes funky-eyes approved these changes

@xingfudeshi xingfudeshi xingfudeshi approved these changes

Assignees
No one assigned
Labels
module/rm-datasource rm-datasource module
Projects
None yet
Milestone
1.4.1
Development

Successfully merging this pull request may close these issues.

None yet
7 participants
@0000005 @codecov-io @caohdgege @xingfudeshi @slievrly @funky-eyes @wangliang181230