[GitHub] [shardingsphere] strongduanmu commented on a change in pull request #13305: ShardingSphere-JDBC 5.0.0-RC1-SNAPSHOT: throw StringIndexOutOfBoundsException when config the encrypt rule and use the sql join

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

strongduanmu commented on a change in pull request #13305:
URL: https://github.com/apache/shardingsphere/pull/13305#discussion_r737285324



##########
File path: shardingsphere-features/shardingsphere-encrypt/shardingsphere-encrypt-core/src/main/java/org/apache/shardingsphere/encrypt/rewrite/token/generator/impl/EncryptProjectionTokenGenerator.java
##########
@@ -104,6 +108,31 @@ protected boolean isGenerateSQLTokenForEncrypt(final SQLStatementContext sqlStat
         return result;
     }
     
+    private boolean isOwnerExistsMatchTableAlias(final SelectStatementContext selectStatementContext, final ColumnProjectionSegment columnProjectionSegment, final String tableName) {
+        return selectStatementContext.getAllUniqueTables().stream().anyMatch(table -> tableName.equals(table.getTableName().getIdentifier().getValue()) 
+                && table.getAlias().isPresent() && columnProjectionSegment.getColumn().getOwner().isPresent() 
+                && columnProjectionSegment.getColumn().getOwner().get().getIdentifier().getValue().equals(table.getAlias().get()));
+    }
+    
+    private boolean isOwnerExistsMatchTableName(final SelectStatementContext selectStatementContext, final ColumnProjectionSegment columnProjectionSegment, final String tableName) {
+        return selectStatementContext.getAllUniqueTables().stream().anyMatch(table -> tableName.equals(table.getTableName().getIdentifier().getValue()) 
+                && !table.getAlias().isPresent() && columnProjectionSegment.getColumn().getOwner().isPresent() 
+                && columnProjectionSegment.getColumn().getOwner().get().getIdentifier().getValue().equals(table.getTableName().getIdentifier().getValue()));
+    }
+    
+    private boolean isColumnAmbiguous(final SelectStatementContext selectStatementContext, final ColumnProjectionSegment columnProjectionSegment) {
+        if (columnProjectionSegment.getColumn().getOwner().isPresent()) {
+            return false;
+        }
+        final AtomicInteger columnCount = new AtomicInteger();

Review comment:
       @LeeGuoPing Considering that this is a local variable, can we just use the ordinary int type?

##########
File path: shardingsphere-features/shardingsphere-encrypt/shardingsphere-encrypt-core/src/main/java/org/apache/shardingsphere/encrypt/rewrite/token/generator/impl/EncryptProjectionTokenGenerator.java
##########
@@ -90,7 +91,10 @@ protected boolean isGenerateSQLTokenForEncrypt(final SQLStatementContext sqlStat
         Collection<SubstitutableColumnNameToken> result = new LinkedList<>();
         for (ProjectionSegment each : segment.getProjections()) {
             if (each instanceof ColumnProjectionSegment) {
-                if (encryptTable.getLogicColumns().contains(((ColumnProjectionSegment) each).getColumn().getIdentifier().getValue())) {
+                if (encryptTable.getLogicColumns().contains(((ColumnProjectionSegment) each).getColumn().getIdentifier().getValue()) 
+                        && (isOwnerExistsMatchTableAlias(selectStatementContext, (ColumnProjectionSegment) each, tableName)

Review comment:
       @LeeGuoPing Can we extract `isOwnerExistsMatchTableAlias`， `isOwnerExistsMatchTableName` and `isColumnAmbiguous` to one method?

##########
File path: shardingsphere-infra/shardingsphere-infra-binder/src/main/java/org/apache/shardingsphere/infra/binder/statement/dml/SelectStatementContext.java
##########
@@ -227,6 +227,15 @@ public boolean isSameGroupByAndOrderByItems() {
         return tablesContext.getOriginalTables();
     }
     
+    /**
+     * Get all unique table segments.
+     *
+     * @return all unique table segments
+     */
+    public Collection<SimpleTableSegment> getAllUniqueTables() {

Review comment:
       @LeeGuoPing Can we move this method to TableContext?




-- 
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.

To unsubscribe, e-mail: notifications-unsubscribe@shardingsphere.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org