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 - [ ] 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
---------------------------------------------------------------- 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
