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

Reconstruct rewriting module #2275

Closed
14 tasks done
tristaZero opened this issue Apr 26, 2019 · 3 comments · Fixed by #2311, #2313, #2317, #2342 or #2364
Closed
14 tasks done

Reconstruct rewriting module #2275

tristaZero opened this issue Apr 26, 2019 · 3 comments · Fixed by #2311, #2313, #2317, #2342 or #2364

Comments

@tristaZero
Copy link
Contributor

tristaZero commented Apr 26, 2019

Reconstruct rewriting module
The tasks of parsing module:

  • Make all of the tokens store startIndex parameter and stopIndex parameter.
  • Remove the invalid parameters for calculating the stopIndex.
  • Add different interface to distinguish the SQLTokens for startIndex and stopIndex.

The tasks of rewriteEngine module:

  • Use startIndex and stopIndex to rewrite SQLs.
  • Make all of the tokens rewritten in the consistent method.
  • Generate placeHolders to handle ItemsToken, LimitRowCountToken and so on.
  • Delete getLogicTableName() from ShardingPlaceholder.
  • Move encrypting processing part to ShardingEncryptorEngine.
  • Use pattern() to create constant placeHolders, variant placeHolders, original literal from SQLTokens.
  • Add interface for variant placeHolders to toString(parameters).
  • Use pattern() to make constant placeHolders become literals, and create SQLBuilder to store placeHolder-literals, variant placeHolders and original literals.
  • Use generateSQL() to generate SQLUnit.

The tasks of SQLBuilder module:

  • Store variant placeHolders and placeHolder-literals and original literals.
  • Use toSQL() to generate SQLUnit.
@terrymanu
Copy link
Member

terrymanu commented May 12, 2019

Some code review suggestion:

  • XXXSQLRewriteEngine move to new package name as engine
  • XXXSQLRewriteEngine need implement a interface name as SQLRewriteEngine
  • MasterSlaveSQLRewriteEngine use @requiredargsconstructor
  • Constructor don't need java-doc
  • Don't use @SuppressWarnings("all") on class EncryptUpdateItemColumnPlaceholder
  • EncryptUpdateItemColumnPlaceholder has lots of field can be null, consider about the design, maybe split assistedColumn and not has assistedColumn situation
  • Question in EncryptWhereColumnPlaceholder, between should not supported
  • Should not call SQLUtil.getExactlyValue on rewrite module, current value should come from parse module. For example: IndexPlaceholder

@terrymanu
Copy link
Member

terrymanu commented May 22, 2019

Some suggestions:

  1. Remove SQLRewriteEngine.originalSQL, use sqlStatement.getLogicSQL()
  2. For EncryptRule with SQLRewriteEngine, use current DatabaseType instead of DatabaseType.MySQL
  3. SQLRewriteEngine.isRewriteDistinctLiteral => isMeedRewriteDistinctLiteral
  4. SQLRewriteEngine.isContainsAggregationDistinctToken can move to sqlStatement, and let token type as an input parameter
  5. SelectStatement.getFirstSelectItemStartIndex() should as a Token and Split append distinct from rewriteInitialLiteral
  6. Make sure responsibility of rewriter and engine, only rewriter can use token
  7. EncryptSQLRewriter & ShardingSQLRewriter can new instance out of loop
  8. Make unify with stopPosition & stopIndex
  9. sqlBuilder is global field with SQLRewriteEngine, may avoid pass from private method by input parameter
  10. Make accurate with javadoc on SQLRewriter.rewrite
  11. Avoid start a new line if not exceed 200 char one line, at line 107 on EncryptSQLRewriter
  12. Make input parameter as same sequence for differnet methods, like: EncryptSQLRewriter.encryptInsertOptimizeResultUnit and EncryptSQLRewriter.encryptInsertOptimizeResult's param: unit
  13. add Predicate if Optional can not be absent, at line 105 on EncryptSQLRewriter
  14. InsertParameterUnit => InsertParameterGroup?
  15. Decouple SQLBuilder & ParameterBuilder
  16. Remove SchemaPlaceholder and test cases
  17. Add new method appendPlaceholder for SQLToken and polymorphic appendXXXPlaceholder

@tristaZero
Copy link
Contributor Author

tristaZero commented May 22, 2019

Some suggestions:

  • Remove SQLRewriteEngine.originalSQL, use sqlStatement.getLogicSQL()
  • For EncryptRule with SQLRewriteEngine, use current DatabaseType instead of DatabaseType.MySQL
  • SQLRewriteEngine.isRewriteDistinctLiteral => isMeedRewriteDistinctLiteral
  • SQLRewriteEngine.isContainsAggregationDistinctToken can move to sqlStatement, and let token type as an input parameter
  • SelectStatement.getFirstSelectItemStartIndex() should as a Token and Split append distinct from rewriteInitialLiteral
  • Make sure responsibility of rewriter and engine, only rewriter can use token
  • EncryptSQLRewriter & ShardingSQLRewriter can new instance out of loop
  • Make unify with stopPosition & stopIndex
  • sqlBuilder is global field with SQLRewriteEngine, may avoid pass from private method by input parameter
  • Make accurate with javadoc on SQLRewriter.rewrite
  • Avoid start a new line if not exceed 200 char one line, at line 107 on EncryptSQLRewriter
  • Make input parameter as same sequence for differnet methods, like: EncryptSQLRewriter.encryptInsertOptimizeResultUnit and EncryptSQLRewriter.encryptInsertOptimizeResult's param: unit add Predicate if Optional can not be absent, at line 105 on EncryptSQLRewriter
  • InsertParameterUnit => InsertParameterGroup?
  • Decouple SQLBuilder & ParameterBuilder
  • Remove SchemaPlaceholder and test cases
  • Add new method appendPlaceholder for SQLToken and polymorphic appendXXXPlaceholder

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment