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: optimize PreparedStatementProxy initialization logic #2242

Merged
merged 73 commits into from
Jul 3, 2020

Conversation

booogu
Copy link
Contributor

@booogu booogu commented Feb 12, 2020

Ⅰ. Describe what this PR did

  Change the data structure of ParameterHolder from 2 Dimension Array to Map so that we don't need to execute getParameterCount() while initializing PreparedStatementProxy,which will execute an additional business independent SQL query.
  A further explanation of the present deficiencies:
  If the ParameterHolder uses ArrayList as storage structure, the method "targetStatement.getParameterMetaData ().getparametercount()" will be executed when initializing parameterholder to determine the size of the ArrayList. In some database drivers (such as postgressql), this method will execute a database query, which will lead to performance degradation. In addition, this method may throw a SQLException , if that happen , the Exception that was thrown before the real business SQL will lead to an error that is hard to understand for the users.

Ⅱ. Does this pull request fix one issue?

fixes #2394

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

Ⅳ. Describe how to verify it

Ⅴ. Special notes for reviews

Hao Zhibei and others added 30 commits November 9, 2019 11:11
@zjinlei zjinlei modified the milestones: 1.2.0, 1.3.0 Apr 10, 2020
@jsbxyyx
Copy link
Member

jsbxyyx commented May 25, 2020

@CharmingRabbit please resolve conflict.

@codecov-commenter
Copy link

codecov-commenter commented Jun 30, 2020

Codecov Report

Merging #2242 into develop will decrease coverage by 0.05%.
The diff coverage is 75.00%.

Impacted file tree graph

@@              Coverage Diff              @@
##             develop    #2242      +/-   ##
=============================================
- Coverage      50.22%   50.16%   -0.06%     
  Complexity      3026     3026              
=============================================
  Files            598      598              
  Lines          19314    19311       -3     
  Branches        2348     2347       -1     
=============================================
- Hits            9700     9688      -12     
- Misses          8652     8664      +12     
+ Partials         962      959       -3     
Impacted Files Coverage Δ Complexity Δ
...io/seata/rm/datasource/PreparedStatementProxy.java 100.00% <ø> (ø) 5.00 <0.00> (ø)
...a/sqlparser/druid/oracle/BaseOracleRecognizer.java 0.00% <0.00%> (ø) 0.00 <0.00> (ø)
...ser/druid/postgresql/BasePostgresqlRecognizer.java 0.00% <0.00%> (ø) 0.00 <0.00> (ø)
.../rm/datasource/AbstractPreparedStatementProxy.java 96.29% <100.00%> (-0.07%) 55.00 <3.00> (-1.00)
...o/seata/rm/datasource/exec/BaseInsertExecutor.java 71.89% <100.00%> (ø) 84.00 <0.00> (ø)
...ata/sqlparser/druid/mysql/BaseMySQLRecognizer.java 90.47% <100.00%> (ø) 6.00 <0.00> (ø)
.../rm/datasource/undo/mysql/MySQLUndoLogManager.java 63.88% <0.00%> (-8.34%) 6.00% <0.00%> (ø%)
...ata/rm/datasource/undo/AbstractUndoLogManager.java 47.20% <0.00%> (-3.73%) 17.00% <0.00%> (ø%)

@booogu
Copy link
Contributor Author

booogu commented Jun 30, 2020

@CharmingRabbit please resolve conflict.
Resolved,please take a look.

@funky-eyes funky-eyes requested a review from ggndnn June 30, 2020 13:07
@@ -50,7 +50,7 @@ public OracleOutputVisitor createOutputVisitor(final ParametersHolder parameters
@Override
public boolean visit(SQLVariantRefExpr x) {
if ("?".equals(x.getName())) {
ArrayList<Object> oneParamValues = parametersHolder.getParameters()[x.getIndex()];
ArrayList<Object> oneParamValues = parametersHolder.getParameters().get(x.getIndex());
Copy link
Member

Choose a reason for hiding this comment

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

if need index+1 ?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, my fault.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Corrected,please take a look.

@l81893521 l81893521 added module/rm-datasource rm-datasource module module/sqlparser sql-parser module labels Jul 1, 2020
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

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

@l81893521 l81893521 merged commit 485facf into apache:develop Jul 3, 2020
@booogu booogu deleted the f_parameters branch February 2, 2021 01:54
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.

springboot+springcloud+PGSQL+Seata1.1.0 项目正常启动后 影响业务sql
9 participants