feature: support multi pk for mysql #2398

0000005 · 2020-03-13T08:56:49Z

Ⅰ. Describe what this PR did

pr content: To support multiple primary key.

It's a lot of work to support multiple primary keys for seata. Because this related to the base structure of seata.
I spend one month to do it. But It just works fine in mysql so far.
Because I don't have the time to compatible the seata with postgresql or oracle. And I‘m not good at PostgreSQL and oracle. I tried to compatible the seata with PostgreSQL and oracle, But I met some problems.
So I gave up.
It would be wonderful if someone can continue my work.

There are several bugs that I found when I implement this future. This fix is breaking changes.
1, Rollback will not work if update SQL changed the pk value. So I forbid the execution of this SQL.
2, We can't obtain the auto-increment value if the program run batch inserts SQL in the form of the statement. So I also forbid the execution of this SQL.

Ⅱ. 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

…1.1.0 # Conflicts: # pom.xml

0000005 · 2020-03-13T09:01:20Z

I tested the feature on my sample.

update\delete\insert\select for update each of them has more than 8 test cases.

codecov-io · 2020-03-16T06:12:37Z

Codecov Report

Merging #2398 into develop will increase coverage by 0.18%.
The diff coverage is 71.06%.

@@              Coverage Diff              @@
##             develop    #2398      +/-   ##
=============================================
+ Coverage      51.06%   51.25%   +0.18%     
- Complexity      2812     2859      +47     
=============================================
  Files            557      557              
  Lines          17825    18017     +192     
  Branches        2105     2144      +39     
=============================================
+ Hits            9103     9234     +131     
- Misses          7858     7907      +49     
- Partials         864      876      +12

Impacted Files	Coverage Δ	Complexity Δ
...o/seata/rm/datasource/AbstractConnectionProxy.java	`11.22% <0.00%> (-0.24%)`	`5.00 <0.00> (ø)`
...in/java/io/seata/rm/datasource/sql/struct/Row.java	`100.00% <ø> (+9.09%)`	`10.00 <0.00> (ø)`
.../undo/postgresql/PostgresqlUndoDeleteExecutor.java	`0.00% <0.00%> (ø)`	`0.00 <0.00> (ø)`
.../undo/postgresql/PostgresqlUndoInsertExecutor.java	`0.00% <0.00%> (ø)`	`0.00 <0.00> (ø)`
.../undo/postgresql/PostgresqlUndoUpdateExecutor.java	`0.00% <0.00%> (ø)`	`0.00 <0.00> (ø)`
.../rm/datasource/exec/BaseTransactionalExecutor.java	`59.58% <48.33%> (-5.80%)`	`31.00 <7.00> (+8.00)`	⬇️
...tasource/undo/oracle/OracleUndoInsertExecutor.java	`60.00% <53.84%> (-9.24%)`	`5.00 <2.00> (+2.00)`	⬇️
...va/io/seata/rm/datasource/exec/InsertExecutor.java	`63.71% <67.61%> (+0.43%)`	`55.00 <14.00> (+10.00)`
...seata/rm/datasource/undo/AbstractUndoExecutor.java	`74.32% <90.90%> (+4.87%)`	`33.00 <15.00> (+12.00)`
.../java/io/seata/rm/datasource/DataCompareUtils.java	`73.49% <91.66%> (+0.41%)`	`26.00 <7.00> (+1.00)`
... and 13 more

l81893521 · 2020-04-27T03:49:34Z

Please fix the conflict.

0000005 · 2020-04-27T03:54:07Z

Ok, I will resolve the conflict this week.

…1.3.0 # Conflicts: # rm-datasource/src/main/java/io/seata/rm/datasource/AbstractConnectionProxy.java # rm-datasource/src/main/java/io/seata/rm/datasource/exec/InsertExecutor.java # rm-datasource/src/test/java/io/seata/rm/datasource/exec/InsertExecutorTest.java

…1.1.0 # Conflicts: # rm-datasource/src/main/java/io/seata/rm/datasource/exec/MultiUpdateExecutor.java # rm-datasource/src/main/java/io/seata/rm/datasource/exec/UpdateExecutor.java

0000005 · 2020-05-06T07:56:16Z

@l81893521 The conflict has been resolved.

slievrly · 2020-06-30T11:16:20Z

rm-datasource/src/main/java/io/seata/rm/datasource/undo/oracle/OracleUndoUpdateExecutor.java

+                .collect(Collectors.toList());
+        String whereSql = buildWhereConditionByPKs(pkNameList,keywordChecker);
+
+        return String.format(UPDATE_SQL_TEMPLATE, keywordChecker.checkAndReplace(sqlUndoLog.getTableName()),updateColumns, whereSql);


sqlUndoLog.getTableName() always has escape.

slievrly · 2020-06-30T11:16:31Z

...ource/src/main/java/io/seata/rm/datasource/undo/postgresql/PostgresqlUndoInsertExecutor.java

+                .stream().map(e -> e.getName())
+                .collect(Collectors.toList());
+        String whereSql = buildWhereConditionByPKs(pkNameList,keywordChecker);
+        return String.format(DELETE_SQL_TEMPLATE,keywordChecker.checkAndReplace(sqlUndoLog.getTableName()), whereSql);


sqlUndoLog.getTableName() always has escape.

slievrly · 2020-06-30T11:16:40Z

...ource/src/main/java/io/seata/rm/datasource/undo/postgresql/PostgresqlUndoUpdateExecutor.java

+                .collect(Collectors.toList());
+        String whereSql = buildWhereConditionByPKs(pkNameList,keywordChecker);
+
+        return String.format(UPDATE_SQL_TEMPLATE, keywordChecker.checkAndReplace(sqlUndoLog.getTableName()),updateColumns, whereSql);


sqlUndoLog.getTableName() always has escape.

slievrly · 2020-06-30T12:03:13Z

rm-datasource/src/main/java/io/seata/rm/datasource/exec/BaseTransactionalExecutor.java

-            whereConditionAppender.add("?");
+    protected String buildWhereConditionByPKs(List<Map<String,Field>> pkRows) throws SQLException {
+        StringBuilder sql = new StringBuilder();
+        List<String> pkColumnNameList = getTableMeta().getPrimaryKeyOnlyName();


same as another buildWhereConditionByPKs

slievrly · 2020-06-30T12:24:47Z

rm-datasource/src/main/java/io/seata/rm/datasource/exec/BaseTransactionalExecutor.java

+        for (int i = 0;i < pkColumnNameList.size();i++)
+        {
+            if (i > 0)
+            {


same as another buildWhereConditionByPKs

slievrly · 2020-06-30T14:21:16Z

rm-datasource/src/main/java/io/seata/rm/datasource/DataCompareUtils.java

-                Field field = row.getFields().get(j);
-                if (field.getName().equalsIgnoreCase(primaryKey)) {
-                    rowKey = String.valueOf(field.getValue());
+            String rowKey = new String();


StringUtils.empty

…multi-pk_based_on_1.1.0

fix comments

l81893521

LGTM, Test case:

MYSQL

//insert
insert into account_tbl(user_id, money, information, create_time) values (?, ?, ?, now())
insert into account_tbl(USER_ID, money, information, create_time) values (?, ?, ?, now())
insert into account_tbl(create_time, money, information, user_id) values (now(), ?, ?, ?)
insert into account_tbl(id, user_id, money, information) values (?, ?, ?, ?)
insert into account_tbl(id, user_id, money, information) values (null, ?, ?, ?)

//insert multi pk
insert into account_tbl_multi_pk(user_id, sex, money) values (?, ?, ?)
insert into account_tbl_multi_pk(USER_ID, sex, money) values (?, ?, ?)

//update
update account_tbl set money = money - ?, sex = 1 where user_id = ?

//update multi pk
update account_tbl_multi_pk set money = money - ?, sex = 1 where user_id = ? or user_id = ?


//delete
delete from account_tbl where user_id = ?

//delete multi pk
delete from account_tbl_multi_pk where user_id = ?

//select
select * from account_tbl where id = ? for update

//select multi pk
select * from account_tbl_multi_pk where id = ? for update

ORACLE

//insert
insert into account_tbl(id, user_id, money, information, description) values (1, ?, ?, ?, ?)
insert into account_tbl(id, user_id, money, information, description) values (?, ?, ?, ?, ?)
insert into account_tbl(id, user_id, money, information, description) values (account_tbl_seq.nextval, ?, ?, ?, ?)

//update
update account_tbl set money = money - ? where user_id = ?

//delete
delete from account_tbl where user_id = ?

//select
select * from account_tbl where id = ? for update

POSTGRE

//insert
insert into account_tbl(id, user_id, money) values (11, trim(both ?), ?)
insert into account_tbl(id, user_id, money) values (?, trim(both ?), ?)
insert into account_tbl(id, user_id, money) values (nextval('seq_account_tbl_id'), trim(both ?), ?)
insert into public.account_tbl(user_id, money, id) values (trim(both ?), ?, nextval('seq_account_tbl_id'))
insert into account_tbl(id, user_id, money) values (default, ?, ?)

//update
update account_tbl set money = money - ? where user_id = ?

//delete
delete from account_tbl where user_id = ?

//select
select * from account_tbl where id = ?

rm-datasource/src/main/java/io/seata/rm/datasource/exec/BaseInsertExecutor.java

slievrly · 2020-07-02T12:56:19Z

rm-datasource/src/main/java/io/seata/rm/datasource/exec/AbstractDMLBaseExecutor.java

@@ -83,6 +85,10 @@ public T doExecute(Object... args) throws Throwable {
     * @throws Exception the exception
     */
    protected T executeAutoCommitFalse(Object[] args) throws Exception {
+        if (!JdbcConstants.MYSQL.equalsIgnoreCase(getDbType()) && getTableMeta().getPrimaryKeyOnlyName().size() > 1)


why only mysql?

rm-datasource/src/test/java/io/seata/rm/datasource/undo/h2/keyword/H2KeywordChecker.java

slievrly · 2020-07-02T13:19:13Z

rm-datasource/pom.xml

@@ -83,7 +83,6 @@
        <dependency>
            <groupId>com.google.guava</groupId>


remove guava dependency， cause rm already add.

slievrly

single pk with DML for mysql is ok.
multi-pk cases for mysql is ok.
jdbcTemplate.update("insert into tb_multi(KEY2,comment1) values('2','3')");
jdbcTemplate.update("update tb_multi set comment1='8' where key1 < 5 ");
jdbcTemplate.update("delete from tb_multi where key1 < 5 ");

CREATE TABLE IF NOT EXISTS tb_multi_1(
key1 int(11) NOT NULL,
key2 VARCHAR(100) NOT NULL,
comment1 VARCHAR(40) NOT NULL,
PRIMARY KEY ( key1,key2 )
)ENGINE=InnoDB DEFAULT CHARSET=utf8;

jsbxyyx

composite primary key.

CREATE TABLE `test_multi_pk` (
  `id1` int(11) NOT NULL AUTO_INCREMENT,
  `id2` int(255) NOT NULL,
  `name` varchar(255) DEFAULT NULL,
  PRIMARY KEY (`id1`,`id2`)
) ENGINE=InnoDB AUTO_INCREMENT=2 DEFAULT CHARSET=utf8mb4;

insert into test_multi_pk(id1, id2, name) values(?, ?, ?)
insert into test_multi_pk(id1, id2, name) values(1, 1, 'xx')
insert into test_multi_pk(id1, id2, name) values(null, 1, ?)
insert into test_multi_pk(id1, id2, name) values(null, FLOOR(RAND() * 10000), ?) not support is right.
insert into test_multi_pk(id1, id2, name) values(null, 1, ?), (null, 2, ?)

oracle single pk:

insert into test1(id, name) values(test1_seq.nextval, ?)
insert into test1(id, name) values(10000, ?)
insert into test1(id, name) values(floor(dbms_random.value(900,1000)), ?)
insert into test1(id, name) values(test1_seq.nextval, 'xx')
insert into test1(id, name) values(10002, 'xx')
insert into test1(id, name) values(floor(dbms_random.value(900,1000)), 'xx') not support is right.

postgresql single pk:

insert into test1(id, name) values(nextval('test1_seq'), ?), (nextval('test1_seq'), ?)
insert into test1(id, name) values(10000, ?), (10001, ?)
insert into test1(id, name) values(floor(random()*1000), ?), (floor(random()*1000), ?)
insert into test1(id, name) values(default, ?), (default, ?)
insert into test1(id, name) values(nextval('test1_seq'), 'xx')
insert into test1(id, name) values(10002, 'xx'), (10003, 'xx1')
insert into test1(id, name) values(floor(random()*1000), 'xx') not support is right.
insert into test1(id, name) values(default, 'xx')

mysql single pk:

insert into test1(name2, name, id) values(?, 'xx', ?)
insert into test1(name2, name, id) values(?, 'xx', ?), (?, 'xx', ?)
insert into test1(id) values(?)
insert into test1(id) values(?), (?)
insert into test1(id, name) values(?, 'xx')
insert into test1(id, name) values(?, 'xx'), (?, 'xx')
insert into test1(name, id) values(?, ?)
insert into test1(name, id) values(?, ?), (?, ?)
insert into test1(id, name) values(1, ?), (2, ?)
insert into test1(name, id) values(?, 1), (?, 2)
insert into test1 values(1, ?, 'x2'), (2, ?, 'x2')
insert into test1 values(?, 'xx', 'xx2'), (?, 'xx', 'xx2')

LGTM, Tests Pass

sabersword · 2020-08-09T14:07:45Z

Why does the feature support multi pk only for mysql?
Is there any incompatible grammer with oracle?
Thank you for the details !

0000005 · 2020-08-14T07:14:40Z

Sorry @sabersword , My company only uses MySQL. And I don't have time to support other databases.
You can create PR to support other databases if you need this feature.

funky-eyes · 2023-01-13T09:11:40Z

请签署Seata社区 cla https://cla-assistant.io/seata/seata 谢谢配合

0000005 added 10 commits February 17, 2020 09:43

新增全局回调

96fb100

新增全局回调

ade5100

multi-pk

6d5f145

stage

08ed030

stage

ca8db9c

处理sql关键字的问题

7eb03a7

Merge remote-tracking branch 'origin/1.1.0' into mulp-pk_based_on_1.0.0

5d36978

Merge remote-tracking branch 'origin/1.1.0' into mulp-pk_based_on_1.0.0

420f5b2

change oracle and postgresql

29687f4

Merge remote-tracking branch 'origin/develop' into multi-pk_based_on_…

2efeb4e

…1.1.0 # Conflicts: # pom.xml

fix wrong code style

7dadb08

slievrly mentioned this pull request Mar 16, 2020

At 模式什么时候会支持复合主键呢？ #2366

Closed

1 task

l81893521 added first-time contributor first-time contributor type: feature Category issues or prs related to feature request. labels Mar 16, 2020

fix text cases

651fcce

slievrly mentioned this pull request Mar 16, 2020

seata集成spring quartz框架无法支持双主键 #2411

Open

1 task

zjinlei added the status: merge-conflict Category prs with merge conflict. label Apr 23, 2020

slievrly mentioned this pull request Apr 24, 2020

联合主键不支持 #2589

Closed

0000005 added 6 commits April 28, 2020 15:52

catch up 1.3.0

eb3a261

Merge remote-tracking branch 'origin/develop' into multi-pk_based_on_…

b3f30a8

…1.1.0 # Conflicts: # rm-datasource/src/main/java/io/seata/rm/datasource/exec/MultiUpdateExecutor.java # rm-datasource/src/main/java/io/seata/rm/datasource/exec/UpdateExecutor.java

resolve conflict

0ea8f14

revert file

61b65c7

fix code style

3c2c83f

Merge branch 'develop' into multi-pk_based_on_1.1.0

333d7ef

slievrly reviewed Jun 30, 2020

View reviewed changes

0000005 and others added 8 commits July 1, 2020 15:02

resolve review

b8028e3

Merge remote-tracking branch 'myGithub/multi-pk_based_on_1.1.0' into …

7a1f98e

…multi-pk_based_on_1.1.0

optimize import

d3351b5

Merge branch 'develop' into multi-pk_based_on_1.1.0

76eaaf1

fix test cases

473b670

Merge remote-tracking branch 'myGithub/multi-pk_based_on_1.1.0' into …

8130209

…multi-pk_based_on_1.1.0

fix oracle build sql

7d755a2

fix comments

Merge branch 'develop' into multi-pk_based_on_1.1.0

7ac2743

l81893521 approved these changes Jul 1, 2020

View reviewed changes

rm-datasource/src/main/java/io/seata/rm/datasource/exec/BaseInsertExecutor.java Show resolved Hide resolved

l81893521 added the module/rm-datasource rm-datasource module label Jul 1, 2020

Merge branch 'develop' into multi-pk_based_on_1.1.0

ee2ed8f

slievrly reviewed Jul 2, 2020

View reviewed changes

slievrly approved these changes Jul 2, 2020

View reviewed changes

slievrly merged commit ba8a9b6 into apache:develop Jul 3, 2020

jsbxyyx approved these changes Jul 3, 2020

View reviewed changes

l81893521 mentioned this pull request Jul 9, 2020

support multi-primary key #1233

Closed

l81893521 changed the title ~~feature: support Multi pk~~ feature: support multi pk for mysql Jul 10, 2020

0000005 deleted the multi-pk_based_on_1.1.0 branch August 14, 2020 07:14

sabersword mentioned this pull request Aug 19, 2020

建议别做1.4.0版本了的新功能，问题这么多先在1.3.0做小版本迭代把问题改完。 #3021

Closed

wqd0214 mentioned this pull request Oct 30, 2020

求助处理SqlServer AT 模式下列阵列无效问题 #3245

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feature: support multi pk for mysql #2398

feature: support multi pk for mysql #2398

0000005 commented Mar 13, 2020

0000005 commented Mar 13, 2020 •

edited

codecov-io commented Mar 16, 2020 •

edited

l81893521 commented Apr 27, 2020

0000005 commented Apr 27, 2020

0000005 commented May 6, 2020

slievrly Jun 30, 2020

0000005 Jul 1, 2020

slievrly Jun 30, 2020

0000005 Jul 1, 2020

slievrly Jun 30, 2020

0000005 Jul 1, 2020

slievrly Jun 30, 2020

0000005 Jul 1, 2020

slievrly Jun 30, 2020

0000005 Jul 1, 2020

slievrly Jun 30, 2020

0000005 Jul 1, 2020

l81893521 left a comment

slievrly Jul 2, 2020

slievrly Jul 2, 2020

slievrly left a comment •

edited

jsbxyyx left a comment

sabersword commented Aug 9, 2020

0000005 commented Aug 14, 2020

funky-eyes commented Jan 13, 2023

		@@ -83,7 +83,6 @@
		<dependency>
		<groupId>com.google.guava</groupId>

feature: support multi pk for mysql #2398

feature: support multi pk for mysql #2398

Conversation

0000005 commented Mar 13, 2020

Ⅰ. Describe what this PR did

Ⅱ. 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

0000005 commented Mar 13, 2020 • edited

codecov-io commented Mar 16, 2020 • edited

Codecov Report

l81893521 commented Apr 27, 2020

0000005 commented Apr 27, 2020

0000005 commented May 6, 2020

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

l81893521 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

slievrly left a comment • edited

Choose a reason for hiding this comment

jsbxyyx left a comment

Choose a reason for hiding this comment

sabersword commented Aug 9, 2020

0000005 commented Aug 14, 2020

funky-eyes commented Jan 13, 2023

0000005 commented Mar 13, 2020 •

edited

codecov-io commented Mar 16, 2020 •

edited

slievrly left a comment •

edited