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

feature: add a Executor for INSERT ON DUPLICATE KEY UPDATE #3374

Merged
merged 70 commits into from
May 3, 2021

Conversation

huan415
Copy link
Contributor

@huan415 huan415 commented Dec 14, 2020

Ⅰ. Describe what this PR did

新加一个Executor, 执行INSERT ON DUPLICATE KEY UPDATE

Ⅱ. Does this pull request fix one issue?

INSERT ON DUPLICATE KEY UPDATE当update时会抛异常

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

Ⅳ. Describe how to verify it

如果含有ON DUPLICATE KEY UPDATE则走新的执行器,利用唯一索引查前置和后置镜像,如果有前置镜像则包含update,没有前置镜像则为insert
如果采用foreach,一条sql可能有三种情况:
1.只有insert,SQLUndoLog的sqlType是insert(唯一索引都不存在)
2.只有update,SQLUndoLog的sqlType是update(唯一索引已全部存在)
3.既有insert,又有有update,两个SQLUndoLog,sqlType分别是insert、update(唯一索引检验部分存在)

Ⅴ. Special notes for reviews

@caohdgege
Copy link
Contributor

为什么这个有executor,但是没有对应的undoExecutor的,这样的话,在回滚的场景下是怎么处理的?

@huan415
Copy link
Contributor Author

huan415 commented Dec 22, 2020

为什么这个有executor,但是没有对应的undoExecutor的,这样的话,在回滚的场景下是怎么处理的?

undoExecutor这个逻辑不用动。还是走原来的逻辑。
回滚场景是根据SQLUndoLog的SqlType来判断,这种sql生成的SQLUndoLog可能是insert也可能是update,也有可能两种都有,

@caohdgege
Copy link
Contributor

    @GlobalTransactional(timeoutMills = 3000000)
    public void tcc(@RequestParam(value = "times", required = false, defaultValue = "1") int times,
                    @RequestParam(value = "batchNum", required = false, defaultValue = "100") int batchNum,
                    @RequestParam(value = "thr", required = false, defaultValue = "0") int thr) throws SQLException {
        Connection connection = dataSource.getConnection();
        System.out.println(connection instanceof ConnectionProxy);
        String sql = "insert into user(id, username, PASSWORD, email, phone, sex) values (1, 'fdafdasdfa', 'fdasfas', 'fdafasdd', 'fdasfasdfasdf', ?),  (3002, 'fdafdasdfa', 'fdasfas', 'fdafasdd', 'fdasfasdfasdf', ?) ON DUPLICATE KEY UPDATE username='fdafas'";
        PreparedStatement ps = connection.prepareStatement(sql);
        ps.setInt(1, 0);
        ps.setInt(2, 1);
        ps.executeUpdate();
        ps.executeUpdate();
        if (true) {
            throw new RuntimeException();
        }
    }

我数据库里面3002的数据是存在的,id=1的数据是不存在的,username是唯一键,这样测试的话,在回滚的时候会把我id=3002的数据误删除。
附上我user表的建表语句。

create table if not exists seata_tx_test.user
(
	id bigint unsigned auto_increment
		primary key,
	username varchar(255) not null comment '用户名',
	PASSWORD char(36) not null comment '密码',
	email char(100) not null comment '邮箱',
	phone char(20) not null comment '电话号码',
	logo char(255) default '/default-logo.png' not null comment '头像',
	address char(150) default '' not null comment '地址',
	sex tinyint default 0 not null comment '0->未设置 1->男 2->女',
	constraint email
		unique (email),
	constraint uniq_phone_sex
		unique (phone, sex),
	constraint username
		unique (username)
)
collate=utf8mb4_general_ci;

@caohdgege
Copy link
Contributor

把开发分支合并到你的分支再推一下,看一下ci能不能过。

@codecov-io
Copy link

codecov-io commented Dec 22, 2020

Codecov Report

Merging #3374 (7b4408c) into develop (aa17303) will decrease coverage by 0.21%.
The diff coverage is 29.94%.

Impacted file tree graph

@@              Coverage Diff              @@
##             develop    #3374      +/-   ##
=============================================
- Coverage      52.09%   51.88%   -0.22%     
- Complexity      3511     3524      +13     
=============================================
  Files            638      639       +1     
  Lines          21147    21333     +186     
  Branches        2629     2658      +29     
=============================================
+ Hits           11016    11068      +52     
- Misses          9044     9168     +124     
- Partials        1087     1097      +10     
Impacted Files Coverage Δ Complexity Δ
...a/io/seata/rm/datasource/exec/ExecuteTemplate.java 5.88% <0.00%> (-0.57%) 2.00 <0.00> (ø)
...parser/antlr/mysql/AntlrMySQLInsertRecognizer.java 0.00% <0.00%> (ø) 0.00 <0.00> (ø)
...sqlparser/druid/oracle/OracleInsertRecognizer.java 0.00% <0.00%> (ø) 0.00 <0.00> (ø)
...r/druid/postgresql/PostgresqlInsertRecognizer.java 0.00% <0.00%> (ø) 0.00 <0.00> (ø)
...a/sqlparser/druid/mysql/MySQLInsertRecognizer.java 75.47% <9.09%> (-17.56%) 16.00 <2.00> (+1.00) ⬇️
...source/exec/mysql/MySQLInsertOrUpdateExecutor.java 32.93% <32.93%> (ø) 13.00 <13.00> (?)
...n/src/main/java/io/seata/common/util/IdWorker.java 77.08% <0.00%> (-6.25%) 11.00% <0.00%> (-1.00%)

@caohdgege
Copy link
Contributor

我觉得这个应该算是特性 feature,之前不支持 ON DUPLICATE KEY UPDATE,这个pr就是让seata支持这种语法。
还有标题稍微改一下,pr的标题不要用中文。

@funky-eyes funky-eyes added first-time contributor first-time contributor module/rm-datasource rm-datasource module labels Dec 23, 2020
@huan415
Copy link
Contributor Author

huan415 commented Dec 24, 2020

    @GlobalTransactional(timeoutMills = 3000000)
    public void tcc(@RequestParam(value = "times", required = false, defaultValue = "1") int times,
                    @RequestParam(value = "batchNum", required = false, defaultValue = "100") int batchNum,
                    @RequestParam(value = "thr", required = false, defaultValue = "0") int thr) throws SQLException {
        Connection connection = dataSource.getConnection();
        System.out.println(connection instanceof ConnectionProxy);
        String sql = "insert into user(id, username, PASSWORD, email, phone, sex) values (1, 'fdafdasdfa', 'fdasfas', 'fdafasdd', 'fdasfasdfasdf', ?),  (3002, 'fdafdasdfa', 'fdasfas', 'fdafasdd', 'fdasfasdfasdf', ?) ON DUPLICATE KEY UPDATE username='fdafas'";
        PreparedStatement ps = connection.prepareStatement(sql);
        ps.setInt(1, 0);
        ps.setInt(2, 1);
        ps.executeUpdate();
        ps.executeUpdate();
        if (true) {
            throw new RuntimeException();
        }
    }

我数据库里面3002的数据是存在的,id=1的数据是不存在的,username是唯一键,这样测试的话,在回滚的时候会把我id=3002的数据误删除。
附上我user表的建表语句。

create table if not exists seata_tx_test.user
(
	id bigint unsigned auto_increment
		primary key,
	username varchar(255) not null comment '用户名',
	PASSWORD char(36) not null comment '密码',
	email char(100) not null comment '邮箱',
	phone char(20) not null comment '电话号码',
	logo char(255) default '/default-logo.png' not null comment '头像',
	address char(150) default '' not null comment '地址',
	sex tinyint default 0 not null comment '0->未设置 1->男 2->女',
	constraint email
		unique (email),
	constraint uniq_phone_sex
		unique (phone, sex),
	constraint username
		unique (username)
)
collate=utf8mb4_general_ci;

刚刚试了一下,用mybatis不会。这个是因为单引号的问题(SELECT * FROM user WHERE (email = ' 'fdafasdd'' ) OR (phone = ' 'fdasfasdfasdf'' and sex = 0 ) OR (username = ' 'fdafdasdfa'' ) OR (email = ' 'fdafasdd'' ) OR (phone = ' 'fdasfasdfasdf'' and sex = 1 ) OR (username = ' 'fdafdasdfa'' ) FOR UPDATE), 所以认为获取不到前置镜像,认为两条都是insert,回滚时都删掉。 这是个bug,我想想

@huan415
Copy link
Contributor Author

huan415 commented Dec 24, 2020

feature

ok

@huan415 huan415 changed the title bugfix:INSERT ON DUPLICATE KEY UPDATE当update时会抛异常,新加一个Executor feature:INSERT ON DUPLICATE KEY UPDATE当update时会抛异常,新加一个Executor Dec 26, 2020
@huan415
Copy link
Contributor Author

huan415 commented Mar 16, 2021

image
@huan415 CI failed.Please fix it.

已解决

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.

@codecov-commenter
Copy link

codecov-commenter commented Apr 24, 2021

Codecov Report

Merging #3374 (442b795) into develop (0e98040) will decrease coverage by 0.25%.
The diff coverage is 26.92%.

Impacted file tree graph

@@              Coverage Diff              @@
##             develop    #3374      +/-   ##
=============================================
- Coverage      51.94%   51.68%   -0.26%     
- Complexity      3506     3517      +11     
=============================================
  Files            639      640       +1     
  Lines          21184    21391     +207     
  Branches        2633     2670      +37     
=============================================
+ Hits           11004    11056      +52     
- Misses          9087     9231     +144     
- Partials        1093     1104      +11     
Impacted Files Coverage Δ Complexity Δ
...a/io/seata/rm/datasource/exec/ExecuteTemplate.java 5.88% <0.00%> (-0.57%) 2.00 <0.00> (ø)
...parser/antlr/mysql/AntlrMySQLInsertRecognizer.java 0.00% <0.00%> (ø) 0.00 <0.00> (ø)
...a/sqlparser/druid/mysql/MySQLInsertRecognizer.java 61.90% <0.00%> (-31.12%) 15.00 <1.00> (ø)
...sqlparser/druid/oracle/OracleInsertRecognizer.java 0.00% <0.00%> (ø) 0.00 <0.00> (ø)
...r/druid/postgresql/PostgresqlInsertRecognizer.java 0.00% <0.00%> (ø) 0.00 <0.00> (ø)
...source/exec/mysql/MySQLInsertOrUpdateExecutor.java 31.46% <31.46%> (ø) 13.00 <13.00> (?)
...n/src/main/java/io/seata/common/util/IdWorker.java 77.08% <0.00%> (-6.25%) 11.00% <0.00%> (-1.00%)

Copy link
Contributor

@caohdgege caohdgege left a comment

Choose a reason for hiding this comment

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

LGTM

@huan415 huan415 closed this May 3, 2021
@huan415 huan415 reopened this May 3, 2021
@funky-eyes funky-eyes changed the title feature:add a Executor for INSERT ON DUPLICATE KEY UPDATE feature: add a Executor for INSERT ON DUPLICATE KEY UPDATE May 3, 2021
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

@funky-eyes funky-eyes added this to the 1.5.0 milestone May 3, 2021
@funky-eyes funky-eyes merged commit 38226ba into apache:develop May 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
first-time contributor first-time contributor module/rm-datasource rm-datasource module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

8 participants