You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@shardingsphere.apache.org by GitBox <gi...@apache.org> on 2021/10/25 01:51:43 UTC

[GitHub] [shardingsphere] strongduanmu commented on a change in pull request #13180: Support table level `queryWithCipherColumn` configuration

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



##########
File path: docs/document/content/user-manual/shardingsphere-jdbc/configuration/spring-namespace/encrypt.cn.md
##########
@@ -17,10 +17,11 @@ weight = 3
 
 \<encrypt:table />
 
-| *名称*     | *类型* | *说明*    |
-| ---------- | ----- | -------- |
-| name       | 属性  | 加密表名称 |
-| column (+) | 标签  | 加密列配置 |
+| *名称*                    | *类型* | *说明*                                                      |
+| ------------------------ | ------ | ---------------------------------------------------------- |
+| name                     | 属性    | 加密表名称                                                   |
+| column (+)               | 标签    | 加密列配置                                                   |
+| queryWithCipherColumn(?) | 属性    | 该表是否使用加密列进行查询。在有原文列的情况下,可以使用原文列进行查询 |

Review comment:
       @totalo Should this attribute be `query-with-cipher-column`?

##########
File path: shardingsphere-features/shardingsphere-encrypt/shardingsphere-encrypt-core/src/main/java/org/apache/shardingsphere/encrypt/rule/EncryptRule.java
##########
@@ -242,6 +247,24 @@ public String getCipherColumn(final String logicTable, final String logicColumn)
         Optional<String> originColumnName = findOriginColumnName(logicTable, logicColumn);
         return originColumnName.isPresent() && tables.containsKey(logicTable) ? tables.get(logicTable).findPlainColumn(originColumnName.get()) : Optional.empty();
     }
+
+    /**
+     * Check the table is support QueryWithCipherColumn.

Review comment:
       @totalo Please add a line after the description, you can refer to other java doc writing.

##########
File path: shardingsphere-features/shardingsphere-encrypt/shardingsphere-encrypt-core/src/main/java/org/apache/shardingsphere/encrypt/rule/EncryptRule.java
##########
@@ -242,6 +247,24 @@ public String getCipherColumn(final String logicTable, final String logicColumn)
         Optional<String> originColumnName = findOriginColumnName(logicTable, logicColumn);
         return originColumnName.isPresent() && tables.containsKey(logicTable) ? tables.get(logicTable).findPlainColumn(originColumnName.get()) : Optional.empty();
     }
+
+    /**
+     * Check the table is support QueryWithCipherColumn.
+     * @param sqlStatementContext sqlStatementContext
+     * @return isQueryWithCipherColumn
+     */
+    public boolean isQueryWithCipherColumn(final SQLStatementContext<?> sqlStatementContext) {
+        Collection<SimpleTableSegment> simpleTables = sqlStatementContext instanceof SelectStatementContext
+                ? ((TableAvailable) sqlStatementContext).getAllTables()
+                : Collections.emptyList();
+        if (CollectionUtils.isNotEmpty(simpleTables)) {
+            String tableName = simpleTables.iterator().next().getTableName().getIdentifier().getValue();
+            if (tables.containsKey(tableName) && tables.get(tableName).getQueryWithCipherColumn() != null) {
+                return tables.get(tableName).getQueryWithCipherColumn();
+            }
+        }
+        return queryWithCipherColumn;

Review comment:
       Please rename `queryWithCipherColumn` to `result`.

##########
File path: shardingsphere-features/shardingsphere-encrypt/shardingsphere-encrypt-core/src/main/java/org/apache/shardingsphere/encrypt/rule/EncryptRule.java
##########
@@ -242,6 +247,24 @@ public String getCipherColumn(final String logicTable, final String logicColumn)
         Optional<String> originColumnName = findOriginColumnName(logicTable, logicColumn);
         return originColumnName.isPresent() && tables.containsKey(logicTable) ? tables.get(logicTable).findPlainColumn(originColumnName.get()) : Optional.empty();
     }
+
+    /**
+     * Check the table is support QueryWithCipherColumn.
+     * @param sqlStatementContext sqlStatementContext
+     * @return isQueryWithCipherColumn
+     */
+    public boolean isQueryWithCipherColumn(final SQLStatementContext<?> sqlStatementContext) {
+        Collection<SimpleTableSegment> simpleTables = sqlStatementContext instanceof SelectStatementContext
+                ? ((TableAvailable) sqlStatementContext).getAllTables()
+                : Collections.emptyList();
+        if (CollectionUtils.isNotEmpty(simpleTables)) {

Review comment:
       @totalo `if (!simpleTables.isEmpty())` may be better?

##########
File path: shardingsphere-features/shardingsphere-encrypt/shardingsphere-encrypt-core/src/main/java/org/apache/shardingsphere/encrypt/rewrite/token/generator/impl/EncryptPredicateColumnTokenGenerator.java
##########
@@ -83,7 +83,8 @@ protected boolean isGenerateSQLTokenForEncrypt(final SQLStatementContext sqlStat
                 }
                 int startIndex = column.getOwner().isPresent() ? column.getOwner().get().getStopIndex() + 2 : column.getStartIndex();
                 int stopIndex = column.getStopIndex();
-                if (!queryWithCipherColumn) {
+                EncryptTable table = encryptTable.get();
+                if ((table.getQueryWithCipherColumn() != null && !table.getQueryWithCipherColumn()) || !queryWithCipherColumn) {

Review comment:
       @totalo Please put null on the left side of the expression.

##########
File path: shardingsphere-features/shardingsphere-encrypt/shardingsphere-encrypt-api/src/main/java/org/apache/shardingsphere/encrypt/api/config/rule/EncryptTableRuleConfiguration.java
##########
@@ -32,4 +32,6 @@
     private final String name;
     
     private final Collection<EncryptColumnRuleConfiguration> columns;
+    
+    private final Boolean queryWithCipherColumn;

Review comment:
       @totalo Can we use boolean type instead of Boolean? If the user does not configure this attribute, we expect a default value instead of null.

##########
File path: shardingsphere-features/shardingsphere-encrypt/shardingsphere-encrypt-distsql/shardingsphere-encrypt-distsql-statement/src/main/java/org/apache/shardingsphere/encrypt/distsql/parser/segment/EncryptRuleSegment.java
##########
@@ -33,4 +33,6 @@
     private final String tableName;
     
     private final Collection<EncryptColumnSegment> columns;
+    
+    private final Boolean queryWithCipherColumn;

Review comment:
       @totalo Can we replace this Boolean type with boolean?

##########
File path: shardingsphere-features/shardingsphere-encrypt/shardingsphere-encrypt-core/src/main/java/org/apache/shardingsphere/encrypt/rule/EncryptRule.java
##########
@@ -242,6 +247,24 @@ public String getCipherColumn(final String logicTable, final String logicColumn)
         Optional<String> originColumnName = findOriginColumnName(logicTable, logicColumn);
         return originColumnName.isPresent() && tables.containsKey(logicTable) ? tables.get(logicTable).findPlainColumn(originColumnName.get()) : Optional.empty();
     }
+
+    /**
+     * Check the table is support QueryWithCipherColumn.
+     * @param sqlStatementContext sqlStatementContext
+     * @return isQueryWithCipherColumn
+     */
+    public boolean isQueryWithCipherColumn(final SQLStatementContext<?> sqlStatementContext) {
+        Collection<SimpleTableSegment> simpleTables = sqlStatementContext instanceof SelectStatementContext
+                ? ((TableAvailable) sqlStatementContext).getAllTables()
+                : Collections.emptyList();
+        if (CollectionUtils.isNotEmpty(simpleTables)) {
+            String tableName = simpleTables.iterator().next().getTableName().getIdentifier().getValue();
+            if (tables.containsKey(tableName) && tables.get(tableName).getQueryWithCipherColumn() != null) {

Review comment:
       Please put null on the left side of the conditional expression.




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