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

optimize: refactor InsertExecutor #2280

Merged
merged 29 commits into from
May 20, 2020
Merged

optimize: refactor InsertExecutor #2280

merged 29 commits into from
May 20, 2020

Conversation

jsbxyyx
Copy link
Member

@jsbxyyx jsbxyyx commented Feb 22, 2020

Ⅰ. Describe what this PR did

Refactor insert executor.

Ⅱ. Does this pull request fix one issue?

fixes #2042

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

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

@jsbxyyx jsbxyyx requested review from l81893521, zjinlei and slievrly and removed request for l81893521 February 22, 2020 08:58
@codecov-io
Copy link

codecov-io commented Feb 22, 2020

Codecov Report

Merging #2280 into develop will decrease coverage by 0.05%.
The diff coverage is 67.31%.

Impacted file tree graph

@@              Coverage Diff              @@
##             develop    #2280      +/-   ##
=============================================
- Coverage      50.91%   50.85%   -0.06%     
- Complexity      2815     2827      +12     
=============================================
  Files            558      561       +3     
  Lines          17941    17965      +24     
  Branches        2129     2102      -27     
=============================================
+ Hits            9134     9136       +2     
+ Misses          7938     7931       -7     
- Partials         869      898      +29     
Impacted Files Coverage Δ Complexity Δ
...ta/rm/datasource/exec/AbstractDMLBaseExecutor.java 76.92% <ø> (ø) 8.00 <0.00> (ø)
...a/io/seata/rm/datasource/exec/ExecuteTemplate.java 7.89% <0.00%> (ø) 3.00 <0.00> (ø)
...a/sqlparser/druid/mysql/MySQLInsertRecognizer.java 97.56% <0.00%> (-2.44%) 15.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> (ø)
...m/datasource/exec/oracle/OracleInsertExecutor.java 50.00% <50.00%> (ø) 4.00 <4.00> (?)
...urce/exec/postgresql/PostgresqlInsertExecutor.java 54.54% <54.54%> (ø) 4.00 <4.00> (?)
...o/seata/rm/datasource/exec/BaseInsertExecutor.java 71.53% <71.53%> (ø) 38.00 <38.00> (?)
.../rm/datasource/exec/mysql/MySQLInsertExecutor.java 78.78% <78.78%> (ø) 11.00 <11.00> (?)
...obuf/convertor/BranchRegisterRequestConvertor.java 90.47% <0.00%> (-9.53%) 3.00% <0.00%> (ø%)
... and 26 more

@zjinlei
Copy link
Contributor

zjinlei commented Feb 22, 2020

please restore UpdateExecutorTest.java, SelectForUpdateExecutorTest.java, PlainExecutorTest.java and DeleteExecutorTest.java.

@jsbxyyx
Copy link
Member Author

jsbxyyx commented Feb 24, 2020

@zjinlei fix done.

@slievrly slievrly added this to the 1.2.0 milestone Feb 24, 2020
@jsbxyyx
Copy link
Member Author

jsbxyyx commented Apr 10, 2020

base on #2301

@slievrly slievrly modified the milestones: 1.2.0, 1.3.0 Apr 12, 2020
@zjinlei zjinlei added the status: merge-conflict Category prs with merge conflict. label Apr 23, 2020
@zjinlei zjinlei removed the status: merge-conflict Category prs with merge conflict. label Apr 24, 2020
@l81893521 l81893521 changed the title Refactor InsertExecutor optimize: refactor InsertExecutor May 8, 2020
Copy link
Contributor

@zjinlei zjinlei left a comment

Choose a reason for hiding this comment

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

LGTM

}
}
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Reduce coupling and implement in 3 steps
1.getInsertRows
2.get pkIndex list from rows
3.get pkValue list from parameters

@l81893521 l81893521 added module/rm-datasource rm-datasource module module/sqlparser sql-parser module labels May 19, 2020
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, Test Passed.
MYSQL

insert into account_tbl(user_id, money, information) values (?, ?, ?)
insert into account_tbl(id, user_id, money, information) values (?, ?, ?, ?)
insert into account_tbl(id, user_id, money, information) values (null, ?, ?, ?)

ORACLE

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, ?, ?, ?, ?)

POSTGRE

insert into account_tbl(id, user_id, money) values (1, 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 ?), ?)

@codecov-commenter
Copy link

codecov-commenter commented May 19, 2020

Codecov Report

Merging #2280 into develop will increase coverage by 0.08%.
The diff coverage is 67.31%.

Impacted file tree graph

@@              Coverage Diff              @@
##             develop    #2280      +/-   ##
=============================================
+ Coverage      51.05%   51.13%   +0.08%     
- Complexity      2835     2849      +14     
=============================================
  Files            563      566       +3     
  Lines          18047    18072      +25     
  Branches        2138     2141       +3     
=============================================
+ Hits            9214     9242      +28     
+ Misses          7951     7944       -7     
- Partials         882      886       +4     
Impacted Files Coverage Δ Complexity Δ
...ta/rm/datasource/exec/AbstractDMLBaseExecutor.java 76.92% <ø> (ø) 8.00 <0.00> (ø)
...a/io/seata/rm/datasource/exec/ExecuteTemplate.java 7.89% <0.00%> (ø) 3.00 <0.00> (ø)
...a/sqlparser/druid/mysql/MySQLInsertRecognizer.java 97.56% <0.00%> (-2.44%) 15.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> (ø)
...m/datasource/exec/oracle/OracleInsertExecutor.java 50.00% <50.00%> (ø) 4.00 <4.00> (?)
...urce/exec/postgresql/PostgresqlInsertExecutor.java 54.54% <54.54%> (ø) 4.00 <4.00> (?)
...o/seata/rm/datasource/exec/BaseInsertExecutor.java 71.53% <71.53%> (ø) 38.00 <38.00> (?)
.../rm/datasource/exec/mysql/MySQLInsertExecutor.java 78.78% <78.78%> (ø) 11.00 <11.00> (?)
... and 5 more

@l81893521 l81893521 merged commit 6d309a5 into apache:develop May 20, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
module/rm-datasource rm-datasource module module/sqlparser sql-parser module
Projects
None yet
Development

Successfully merging this pull request may close these issues.

refactor: InsertExecutor refactor
7 participants