tristaZero edited a comment on issue #2275: Reconstruct rewriting module
URL: 
https://github.com/apache/incubator-shardingsphere/issues/2275#issuecomment-494758009
 
 
   Some suggestions:
   
    - [x] Remove SQLRewriteEngine.originalSQL, use sqlStatement.getLogicSQL()
    - [x] For EncryptRule with SQLRewriteEngine, use current DatabaseType 
instead of DatabaseType.MySQL
    - [x] SQLRewriteEngine.isRewriteDistinctLiteral => 
isMeedRewriteDistinctLiteral
    - [x] SQLRewriteEngine.isContainsAggregationDistinctToken can move to 
sqlStatement, and let token type as an input parameter
    - [x] SelectStatement.getFirstSelectItemStartIndex() should as a Token and 
Split append distinct from rewriteInitialLiteral
    - [x] Make sure responsibility of rewriter and engine, only rewriter can 
use token
    - [x] 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
   

----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
 
For queries about this service, please contact Infrastructure at:
us...@infra.apache.org


With regards,
Apache Git Services

Reply via email to