[GitHub] [incubator-shardingsphere] terrymanu commented on issue #2275: Reconstruct rewriting module

[GitBox <gi...@apache.org>]

terrymanu commented on issue #2275: Reconstruct rewriting module
URL: https://github.com/apache/incubator-shardingsphere/issues/2275#issuecomment-491602194
 
 
   Some code review suggestion:
   
   1. XXXSQLRewriteEngine move to new package name as `engine`
   2. XXXSQLRewriteEngine need implement a interface name as `SQLRewriteEngine`
   3. MasterSlaveSQLRewriteEngine use @RequiredArgsConstructor
   4. Constructor don't need java-doc
   5. method of XXXSQLRewriteEngine.generateSQL is unnecessary, just call sqlBuilder.toSQL
   6. Beside Alterable, add Addable interface
   7. split sharding and encrypt placeholder in package `org.apache.shardingsphere.core.rewrite.placeholder`
   8. EncryptXXXPlaceholder should not implement `ShardingPlaceholder`
   9. Don't use @SuppressWarnings("all") on class EncryptUpdateItemColumnPlaceholder 
   10. EncryptUpdateItemColumnPlaceholder has lots of field can be null, consider about the design, maybe split assistedColumn and not has assistedColumn situation
   11. Question in EncryptWhereColumnPlaceholder, between should not supported
   12. Should not call SQLUtil.getExactlyValue on rewrite module, current value should come from parse module. For example: IndexPlaceholder
   13. Rewrite of class InsertSetPlaceholder is out of scope, should not rewrite "SET"
   
   

----------------------------------------------------------------
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:
users@infra.apache.org


With regards,
Apache Git Services