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 2022/05/12 07:30:00 UTC
[GitHub] [shardingsphere] zhaoguhong opened a new pull request, #17603: 17196
zhaoguhong opened a new pull request, #17603:
URL: https://github.com/apache/shardingsphere/pull/17603
#17196
Support encrypt config queryWithCipherColumn with column levels
--
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
[GitHub] [shardingsphere] zhaoguhong commented on a diff in pull request #17603: Support encrypt config queryWithCipherColumn with column levels
Posted by GitBox <gi...@apache.org>.
zhaoguhong commented on code in PR #17603:
URL: https://github.com/apache/shardingsphere/pull/17603#discussion_r876839812
##########
shardingsphere-features/shardingsphere-encrypt/shardingsphere-encrypt-core/src/main/java/org/apache/shardingsphere/encrypt/rule/EncryptTable.java:
##########
@@ -48,7 +48,7 @@ public EncryptTable(final EncryptTableRuleConfiguration config, final Map<String
columns.put(each.getLogicColumn(), new EncryptColumn(getEncryptColumnDataType(each.getLogicDataType(), dataTypes), each.getCipherColumn(),
getEncryptColumnDataType(each.getCipherDataType(), dataTypes), each.getAssistedQueryColumn(), getEncryptColumnDataType(each.getAssistedQueryDataType(),
dataTypes),
- each.getPlainColumn(), getEncryptColumnDataType(each.getPlainDataType(), dataTypes), each.getEncryptorName()));
+ each.getPlainColumn(), getEncryptColumnDataType(each.getPlainDataType(), dataTypes), each.getEncryptorName(), each.getQueryWithCipherColumn()));
Review Comment:
I added some test cases for `isQueryWithCipherColumn`.
What do you recommend for performance testing?
--
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
[GitHub] [shardingsphere] cheese8 commented on a diff in pull request #17603: Support encrypt config queryWithCipherColumn with column levels
Posted by GitBox <gi...@apache.org>.
cheese8 commented on code in PR #17603:
URL: https://github.com/apache/shardingsphere/pull/17603#discussion_r877657192
##########
shardingsphere-features/shardingsphere-encrypt/shardingsphere-encrypt-distsql/shardingsphere-encrypt-distsql-parser/src/main/java/org/apache/shardingsphere/encrypt/distsql/parser/core/EncryptDistSQLStatementVisitor.java:
##########
@@ -88,7 +88,8 @@ public ASTNode visitEncryptColumnDefinition(final EncryptColumnDefinitionContext
getIdentifierValue(ctx.cipherColumnDefinition().dataType()),
null == ctx.plainColumnDefinition() ? null : getIdentifierValue(ctx.plainColumnDefinition().dataType()),
null == ctx.assistedQueryColumnDefinition() ? null : getIdentifierValue(ctx.assistedQueryColumnDefinition().dataType()),
- (AlgorithmSegment) visit(ctx.algorithmDefinition()));
+ (AlgorithmSegment) visit(ctx.algorithmDefinition()),
Review Comment:
How about support distsql in another PR？Is it error occurred?
##########
shardingsphere-features/shardingsphere-encrypt/shardingsphere-encrypt-distsql/shardingsphere-encrypt-distsql-parser/src/main/antlr4/imports/encrypt/RDLStatement.g4:
##########
@@ -44,7 +44,7 @@ resourceName
;
encryptColumnDefinition
- : LP columnDefinition (COMMA plainColumnDefinition)? COMMA cipherColumnDefinition (COMMA assistedQueryColumnDefinition)? COMMA algorithmDefinition RP
+ : LP columnDefinition (COMMA plainColumnDefinition)? COMMA cipherColumnDefinition (COMMA assistedQueryColumnDefinition)? COMMA algorithmDefinition (COMMA QUERY_WITH_CIPHER_COLUMN EQ queryWithCipherColumn)? RP
Review Comment:
How about support distsql in another PR？Is it error occurred?
##########
shardingsphere-features/shardingsphere-encrypt/shardingsphere-encrypt-core/src/main/java/org/apache/shardingsphere/encrypt/rule/EncryptTable.java:
##########
@@ -48,7 +48,7 @@ public EncryptTable(final EncryptTableRuleConfiguration config, final Map<String
columns.put(each.getLogicColumn(), new EncryptColumn(getEncryptColumnDataType(each.getLogicDataType(), dataTypes), each.getCipherColumn(),
getEncryptColumnDataType(each.getCipherDataType(), dataTypes), each.getAssistedQueryColumn(), getEncryptColumnDataType(each.getAssistedQueryDataType(),
dataTypes),
- each.getPlainColumn(), getEncryptColumnDataType(each.getPlainDataType(), dataTypes), each.getEncryptorName()));
+ each.getPlainColumn(), getEncryptColumnDataType(each.getPlainDataType(), dataTypes), each.getEncryptorName(), each.getQueryWithCipherColumn()));
Review Comment:
Can you provide your wechat id, I will add you into our kernel group.
--
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
[GitHub] [shardingsphere] zhaoguhong commented on a diff in pull request #17603: Support encrypt config queryWithCipherColumn with column levels
Posted by GitBox <gi...@apache.org>.
zhaoguhong commented on code in PR #17603:
URL: https://github.com/apache/shardingsphere/pull/17603#discussion_r877863920
##########
shardingsphere-features/shardingsphere-encrypt/shardingsphere-encrypt-core/src/main/java/org/apache/shardingsphere/encrypt/rule/EncryptTable.java:
##########
@@ -48,7 +48,7 @@ public EncryptTable(final EncryptTableRuleConfiguration config, final Map<String
columns.put(each.getLogicColumn(), new EncryptColumn(getEncryptColumnDataType(each.getLogicDataType(), dataTypes), each.getCipherColumn(),
getEncryptColumnDataType(each.getCipherDataType(), dataTypes), each.getAssistedQueryColumn(), getEncryptColumnDataType(each.getAssistedQueryDataType(),
dataTypes),
- each.getPlainColumn(), getEncryptColumnDataType(each.getPlainDataType(), dataTypes), each.getEncryptorName()));
+ each.getPlainColumn(), getEncryptColumnDataType(each.getPlainDataType(), dataTypes), each.getEncryptorName(), each.getQueryWithCipherColumn()));
Review Comment:
guhong06
--
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
[GitHub] [shardingsphere] cheese8 commented on a diff in pull request #17603: Support encrypt config queryWithCipherColumn with column levels
Posted by GitBox <gi...@apache.org>.
cheese8 commented on code in PR #17603:
URL: https://github.com/apache/shardingsphere/pull/17603#discussion_r874717643
##########
shardingsphere-features/shardingsphere-encrypt/shardingsphere-encrypt-core/src/main/java/org/apache/shardingsphere/encrypt/rule/EncryptColumn.java:
##########
@@ -64,4 +71,5 @@ public Optional<String> getAssistedQueryColumn() {
public Optional<String> getPlainColumn() {
return Strings.isNullOrEmpty(plainColumn) ? Optional.empty() : Optional.of(plainColumn);
}
+
Review Comment:
Is it a new added blank line? How about to remove it?
##########
shardingsphere-features/shardingsphere-encrypt/shardingsphere-encrypt-core/src/main/java/org/apache/shardingsphere/encrypt/rule/EncryptColumn.java:
##########
@@ -47,6 +47,13 @@ public final class EncryptColumn {
private final String encryptorName;
+ private final Boolean queryWithCipherColumn;
+
+ public EncryptColumn(final EncryptColumnDataType logicDataType, final String cipherColumn, final EncryptColumnDataType cipherDataType, final String assistedQueryColumn,
Review Comment:
How about remove this new added constructor and adapt the original one where it is used?
##########
shardingsphere-features/shardingsphere-encrypt/shardingsphere-encrypt-api/src/main/java/org/apache/shardingsphere/encrypt/api/config/rule/EncryptColumnRuleConfiguration.java:
##########
@@ -45,7 +45,19 @@ public final class EncryptColumnRuleConfiguration {
private final String encryptorName;
+ private final Boolean queryWithCipherColumn;
+
public EncryptColumnRuleConfiguration(final String logicColumn, final String cipherColumn, final String assistedQueryColumn, final String plainColumn, final String encryptorName) {
- this(logicColumn, null, cipherColumn, null, assistedQueryColumn, null, plainColumn, null, encryptorName);
+ this(logicColumn, null, cipherColumn, null, assistedQueryColumn, null, plainColumn, null, encryptorName, null);
+ }
+
+ public EncryptColumnRuleConfiguration(final String logicColumn, final String cipherColumn, final String assistedQueryColumn, final String plainColumn, final String encryptorName,
+ final Boolean queryWithCipherColumn) {
+ this(logicColumn, null, cipherColumn, null, assistedQueryColumn, null, plainColumn, null, encryptorName, queryWithCipherColumn);
+ }
+
+ public EncryptColumnRuleConfiguration(final String logicColumn, final String logicDataType, final String cipherColumn, final String cipherDataType, final String assistedQueryColumn,
Review Comment:
How about remove this new added constructor and adapt the original one where it is used?
##########
shardingsphere-features/shardingsphere-encrypt/shardingsphere-encrypt-core/src/main/java/org/apache/shardingsphere/encrypt/rule/EncryptRule.java:
##########
@@ -294,6 +294,17 @@ public boolean isQueryWithCipherColumn(final String tableName) {
return findEncryptTable(tableName).flatMap(EncryptTable::getQueryWithCipherColumn).orElse(queryWithCipherColumn);
}
+ /**
+ * Judge whether column is support QueryWithCipherColumn or not.
+ *
+ * @param logicTable logic table name
+ * @param logicColumn logic column name
+ * @return whether column is support QueryWithCipherColumn or not
+ */
+ public boolean isQueryWithCipherColumn(final String logicTable, final String logicColumn) {
Review Comment:
It looks like another `isQueryWithCipherColumn(logicTable)` exists, how about to adopt that one, and pay attention to the inheritance order on `queryWithCipherColumn` from global to table and column.
##########
shardingsphere-features/shardingsphere-encrypt/shardingsphere-encrypt-core/src/main/java/org/apache/shardingsphere/encrypt/rule/EncryptTable.java:
##########
@@ -196,4 +206,5 @@ public Optional<Boolean> getQueryWithCipherColumn() {
public Optional<EncryptColumn> findEncryptColumn(final String logicColumn) {
return Optional.ofNullable(columns.get(logicColumn));
}
+
Review Comment:
Is it a new added blank line? How about to remove it?
##########
shardingsphere-features/shardingsphere-encrypt/shardingsphere-encrypt-core/src/main/java/org/apache/shardingsphere/encrypt/rule/EncryptTable.java:
##########
@@ -187,6 +187,16 @@ public Optional<Boolean> getQueryWithCipherColumn() {
return Optional.ofNullable(queryWithCipherColumn);
}
+ /**
+ * Get query with cipher column.
+ *
+ * @param logicColumn logic column
+ * @return query with cipher column
+ */
+ public Optional<Boolean> getQueryWithCipherColumn(final String logicColumn) {
Review Comment:
How about inherit the isQueryWithCipherColumn on table with column here?
##########
shardingsphere-features/shardingsphere-encrypt/shardingsphere-encrypt-core/src/main/java/org/apache/shardingsphere/encrypt/rule/EncryptTable.java:
##########
@@ -48,7 +48,7 @@ public EncryptTable(final EncryptTableRuleConfiguration config, final Map<String
columns.put(each.getLogicColumn(), new EncryptColumn(getEncryptColumnDataType(each.getLogicDataType(), dataTypes), each.getCipherColumn(),
getEncryptColumnDataType(each.getCipherDataType(), dataTypes), each.getAssistedQueryColumn(), getEncryptColumnDataType(each.getAssistedQueryDataType(),
dataTypes),
- each.getPlainColumn(), getEncryptColumnDataType(each.getPlainDataType(), dataTypes), each.getEncryptorName()));
+ each.getPlainColumn(), getEncryptColumnDataType(each.getPlainDataType(), dataTypes), each.getEncryptorName(), each.getQueryWithCipherColumn()));
Review Comment:
How about inherit the `isQueryWithCipherColumn` on table with column here?
--
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
[GitHub] [shardingsphere] tuichenchuxin commented on a diff in pull request #17603: Support encrypt config queryWithCipherColumn with column levels
Posted by GitBox <gi...@apache.org>.
tuichenchuxin commented on code in PR #17603:
URL: https://github.com/apache/shardingsphere/pull/17603#discussion_r880010657
##########
shardingsphere-features/shardingsphere-encrypt/shardingsphere-encrypt-core/src/test/java/org/apache/shardingsphere/encrypt/rule/EncryptRuleTest.java:
##########
@@ -143,12 +143,34 @@ public void assertFindPlainColumn() {
@Test
public void assertIsQueryWithCipherColumn() {
- EncryptColumnRuleConfiguration encryptColumnConfig = new EncryptColumnRuleConfiguration("encrypt_column", "encrypt_cipher", "", "", "test_encryptor");
- EncryptTableRuleConfiguration tableConfig = new EncryptTableRuleConfiguration("t_encrypt", Collections.singletonList(encryptColumnConfig), true);
+ // global(true) -> table(null) -> column(null)
Review Comment:
This comment need to be removed
##########
shardingsphere-features/shardingsphere-encrypt/shardingsphere-encrypt-core/src/test/java/org/apache/shardingsphere/encrypt/rule/EncryptRuleTest.java:
##########
@@ -143,12 +143,34 @@ public void assertFindPlainColumn() {
@Test
public void assertIsQueryWithCipherColumn() {
- EncryptColumnRuleConfiguration encryptColumnConfig = new EncryptColumnRuleConfiguration("encrypt_column", "encrypt_cipher", "", "", "test_encryptor");
- EncryptTableRuleConfiguration tableConfig = new EncryptTableRuleConfiguration("t_encrypt", Collections.singletonList(encryptColumnConfig), true);
+ // global(true) -> table(null) -> column(null)
+ EncryptColumnRuleConfiguration encryptColumnConfig = new EncryptColumnRuleConfiguration("encrypt_column", "encrypt_cipher", "", "", "test_encryptor", null);
+ EncryptTableRuleConfiguration tableConfig = new EncryptTableRuleConfiguration("t_encrypt", Collections.singletonList(encryptColumnConfig), null);
AlgorithmProvidedEncryptRuleConfiguration ruleConfig = new AlgorithmProvidedEncryptRuleConfiguration(
Collections.singleton(tableConfig), Collections.singletonMap("test_encryptor", new CoreEncryptAlgorithmFixture()), true);
EncryptRule actual = new EncryptRule(ruleConfig, Collections.emptyMap());
- assertTrue(actual.isQueryWithCipherColumn("t_encrypt"));
+ assertTrue(actual.isQueryWithCipherColumn("t_encrypt", "encrypt_column"));
+
+ // global(true) -> table(false) -> column(null)
+ encryptColumnConfig = new EncryptColumnRuleConfiguration("encrypt_column", "encrypt_cipher", "", "", "test_encryptor", null);
+ tableConfig = new EncryptTableRuleConfiguration("t_encrypt", Collections.singletonList(encryptColumnConfig), false);
+ ruleConfig = new AlgorithmProvidedEncryptRuleConfiguration(Collections.singleton(tableConfig), Collections.singletonMap("test_encryptor", new CoreEncryptAlgorithmFixture()), true);
+ actual = new EncryptRule(ruleConfig, Collections.emptyMap());
+ assertFalse(actual.isQueryWithCipherColumn("t_encrypt", "encrypt_column"));
+
+ // global(true) -> table(false) -> column(true)
+ encryptColumnConfig = new EncryptColumnRuleConfiguration("encrypt_column", "encrypt_cipher", "", "", "test_encryptor", true);
+ tableConfig = new EncryptTableRuleConfiguration("t_encrypt", Collections.singletonList(encryptColumnConfig), false);
+ ruleConfig = new AlgorithmProvidedEncryptRuleConfiguration(Collections.singleton(tableConfig), Collections.singletonMap("test_encryptor", new CoreEncryptAlgorithmFixture()), true);
+ actual = new EncryptRule(ruleConfig, Collections.emptyMap());
+ assertTrue(actual.isQueryWithCipherColumn("t_encrypt", "encrypt_column"));
+
+ // global(true) -> table(null) -> column(false)
Review Comment:
This comment need to be removed
##########
shardingsphere-features/shardingsphere-encrypt/shardingsphere-encrypt-core/src/test/java/org/apache/shardingsphere/encrypt/rule/EncryptRuleTest.java:
##########
@@ -143,12 +143,34 @@ public void assertFindPlainColumn() {
@Test
public void assertIsQueryWithCipherColumn() {
- EncryptColumnRuleConfiguration encryptColumnConfig = new EncryptColumnRuleConfiguration("encrypt_column", "encrypt_cipher", "", "", "test_encryptor");
- EncryptTableRuleConfiguration tableConfig = new EncryptTableRuleConfiguration("t_encrypt", Collections.singletonList(encryptColumnConfig), true);
+ // global(true) -> table(null) -> column(null)
+ EncryptColumnRuleConfiguration encryptColumnConfig = new EncryptColumnRuleConfiguration("encrypt_column", "encrypt_cipher", "", "", "test_encryptor", null);
+ EncryptTableRuleConfiguration tableConfig = new EncryptTableRuleConfiguration("t_encrypt", Collections.singletonList(encryptColumnConfig), null);
AlgorithmProvidedEncryptRuleConfiguration ruleConfig = new AlgorithmProvidedEncryptRuleConfiguration(
Collections.singleton(tableConfig), Collections.singletonMap("test_encryptor", new CoreEncryptAlgorithmFixture()), true);
EncryptRule actual = new EncryptRule(ruleConfig, Collections.emptyMap());
- assertTrue(actual.isQueryWithCipherColumn("t_encrypt"));
+ assertTrue(actual.isQueryWithCipherColumn("t_encrypt", "encrypt_column"));
+
+ // global(true) -> table(false) -> column(null)
Review Comment:
This comment need to be removed
##########
shardingsphere-features/shardingsphere-encrypt/shardingsphere-encrypt-core/src/test/java/org/apache/shardingsphere/encrypt/rule/EncryptRuleTest.java:
##########
@@ -143,12 +143,34 @@ public void assertFindPlainColumn() {
@Test
public void assertIsQueryWithCipherColumn() {
- EncryptColumnRuleConfiguration encryptColumnConfig = new EncryptColumnRuleConfiguration("encrypt_column", "encrypt_cipher", "", "", "test_encryptor");
- EncryptTableRuleConfiguration tableConfig = new EncryptTableRuleConfiguration("t_encrypt", Collections.singletonList(encryptColumnConfig), true);
+ // global(true) -> table(null) -> column(null)
+ EncryptColumnRuleConfiguration encryptColumnConfig = new EncryptColumnRuleConfiguration("encrypt_column", "encrypt_cipher", "", "", "test_encryptor", null);
+ EncryptTableRuleConfiguration tableConfig = new EncryptTableRuleConfiguration("t_encrypt", Collections.singletonList(encryptColumnConfig), null);
AlgorithmProvidedEncryptRuleConfiguration ruleConfig = new AlgorithmProvidedEncryptRuleConfiguration(
Collections.singleton(tableConfig), Collections.singletonMap("test_encryptor", new CoreEncryptAlgorithmFixture()), true);
EncryptRule actual = new EncryptRule(ruleConfig, Collections.emptyMap());
- assertTrue(actual.isQueryWithCipherColumn("t_encrypt"));
+ assertTrue(actual.isQueryWithCipherColumn("t_encrypt", "encrypt_column"));
+
+ // global(true) -> table(false) -> column(null)
+ encryptColumnConfig = new EncryptColumnRuleConfiguration("encrypt_column", "encrypt_cipher", "", "", "test_encryptor", null);
+ tableConfig = new EncryptTableRuleConfiguration("t_encrypt", Collections.singletonList(encryptColumnConfig), false);
+ ruleConfig = new AlgorithmProvidedEncryptRuleConfiguration(Collections.singleton(tableConfig), Collections.singletonMap("test_encryptor", new CoreEncryptAlgorithmFixture()), true);
+ actual = new EncryptRule(ruleConfig, Collections.emptyMap());
+ assertFalse(actual.isQueryWithCipherColumn("t_encrypt", "encrypt_column"));
+
+ // global(true) -> table(false) -> column(true)
Review Comment:
This comment need to be removed
--
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
[GitHub] [shardingsphere] cheese8 commented on pull request #17603: 17196
Posted by GitBox <gi...@apache.org>.
cheese8 commented on PR #17603:
URL: https://github.com/apache/shardingsphere/pull/17603#issuecomment-1128478306
Please resolve the ci failure. @zhaoguhong
--
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
[GitHub] [shardingsphere] cheese8 commented on a diff in pull request #17603: Support encrypt config queryWithCipherColumn with column levels
Posted by GitBox <gi...@apache.org>.
cheese8 commented on code in PR #17603:
URL: https://github.com/apache/shardingsphere/pull/17603#discussion_r875818415
##########
shardingsphere-features/shardingsphere-encrypt/shardingsphere-encrypt-core/src/main/java/org/apache/shardingsphere/encrypt/rule/EncryptTable.java:
##########
@@ -48,7 +48,7 @@ public EncryptTable(final EncryptTableRuleConfiguration config, final Map<String
columns.put(each.getLogicColumn(), new EncryptColumn(getEncryptColumnDataType(each.getLogicDataType(), dataTypes), each.getCipherColumn(),
getEncryptColumnDataType(each.getCipherDataType(), dataTypes), each.getAssistedQueryColumn(), getEncryptColumnDataType(each.getAssistedQueryDataType(),
dataTypes),
- each.getPlainColumn(), getEncryptColumnDataType(each.getPlainDataType(), dataTypes), each.getEncryptorName()));
+ each.getPlainColumn(), getEncryptColumnDataType(each.getPlainDataType(), dataTypes), each.getEncryptorName(), each.getQueryWithCipherColumn()));
Review Comment:
OK, can you add some UTs to ensure that the inheritance order on `isQueryWithCipherColumn` is global->table->column, and the `isQueryWithCipherColumn` priority is column > table > global.
--
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
[GitHub] [shardingsphere] strongduanmu commented on pull request #17603: Support encrypt config queryWithCipherColumn with column levels
Posted by GitBox <gi...@apache.org>.
strongduanmu commented on PR #17603:
URL: https://github.com/apache/shardingsphere/pull/17603#issuecomment-1138122821
Hi @zhaoguhong After this PR is completed, the encrypt feature supports `queryWithCipherColumn` configuration at the column level, and users can control encryption and decryption in a more fine-grained manner. Also, is it possible to continue with the task of adding test cases, examples, and documentation?
--
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
[GitHub] [shardingsphere] zhaoguhong commented on a diff in pull request #17603: Support encrypt config queryWithCipherColumn with column levels
Posted by GitBox <gi...@apache.org>.
zhaoguhong commented on code in PR #17603:
URL: https://github.com/apache/shardingsphere/pull/17603#discussion_r878985989
##########
shardingsphere-features/shardingsphere-encrypt/shardingsphere-encrypt-distsql/shardingsphere-encrypt-distsql-parser/src/main/java/org/apache/shardingsphere/encrypt/distsql/parser/core/EncryptDistSQLStatementVisitor.java:
##########
@@ -88,7 +88,8 @@ public ASTNode visitEncryptColumnDefinition(final EncryptColumnDefinitionContext
getIdentifierValue(ctx.cipherColumnDefinition().dataType()),
null == ctx.plainColumnDefinition() ? null : getIdentifierValue(ctx.plainColumnDefinition().dataType()),
null == ctx.assistedQueryColumnDefinition() ? null : getIdentifierValue(ctx.assistedQueryColumnDefinition().dataType()),
- (AlgorithmSegment) visit(ctx.algorithmDefinition()));
+ (AlgorithmSegment) visit(ctx.algorithmDefinition()),
Review Comment:
ok
--
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
[GitHub] [shardingsphere] zhaoguhong commented on a diff in pull request #17603: Support encrypt config queryWithCipherColumn with column levels
Posted by GitBox <gi...@apache.org>.
zhaoguhong commented on code in PR #17603:
URL: https://github.com/apache/shardingsphere/pull/17603#discussion_r878986374
##########
shardingsphere-features/shardingsphere-encrypt/shardingsphere-encrypt-distsql/shardingsphere-encrypt-distsql-parser/src/main/antlr4/imports/encrypt/RDLStatement.g4:
##########
@@ -44,7 +44,7 @@ resourceName
;
encryptColumnDefinition
- : LP columnDefinition (COMMA plainColumnDefinition)? COMMA cipherColumnDefinition (COMMA assistedQueryColumnDefinition)? COMMA algorithmDefinition RP
+ : LP columnDefinition (COMMA plainColumnDefinition)? COMMA cipherColumnDefinition (COMMA assistedQueryColumnDefinition)? COMMA algorithmDefinition (COMMA QUERY_WITH_CIPHER_COLUMN EQ queryWithCipherColumn)? RP
Review Comment:
ok
--
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
[GitHub] [shardingsphere] codecov-commenter commented on pull request #17603: Support encrypt config queryWithCipherColumn with column levels
Posted by GitBox <gi...@apache.org>.
codecov-commenter commented on PR #17603:
URL: https://github.com/apache/shardingsphere/pull/17603#issuecomment-1128667200
# [Codecov](https://codecov.io/gh/apache/shardingsphere/pull/17603?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) Report
> Merging [#17603](https://codecov.io/gh/apache/shardingsphere/pull/17603?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (21c6ce6) into [master](https://codecov.io/gh/apache/shardingsphere/commit/a0e561d17b54ac6bcd2bb98de010c73cf5e5e162?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) (a0e561d) will **decrease** coverage by `0.00%`.
> The diff coverage is `57.14%`.
> :exclamation: Current head 21c6ce6 differs from pull request most recent head 1a334fb. Consider uploading reports for the commit 1a334fb to get more accurate results
```diff
@@ Coverage Diff @@
## master #17603 +/- ##
============================================
- Coverage 59.01% 59.01% -0.01%
Complexity 2127 2127
============================================
Files 3592 3592
Lines 53387 53397 +10
Branches 9115 9115
============================================
+ Hits 31509 31512 +3
- Misses 19210 19217 +7
Partials 2668 2668
```
| [Impacted Files](https://codecov.io/gh/apache/shardingsphere/pull/17603?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | Coverage Δ | |
|---|---|---|
| [...pi/config/rule/EncryptColumnRuleConfiguration.java](https://codecov.io/gh/apache/shardingsphere/pull/17603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtZmVhdHVyZXMvc2hhcmRpbmdzcGhlcmUtZW5jcnlwdC9zaGFyZGluZ3NwaGVyZS1lbmNyeXB0LWFwaS9zcmMvbWFpbi9qYXZhL29yZy9hcGFjaGUvc2hhcmRpbmdzcGhlcmUvZW5jcnlwdC9hcGkvY29uZmlnL3J1bGUvRW5jcnlwdENvbHVtblJ1bGVDb25maWd1cmF0aW9uLmphdmE=) | `0.00% <0.00%> (ø)` | |
| [...er/rewriter/EncryptPredicateParameterRewriter.java](https://codecov.io/gh/apache/shardingsphere/pull/17603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtZmVhdHVyZXMvc2hhcmRpbmdzcGhlcmUtZW5jcnlwdC9zaGFyZGluZ3NwaGVyZS1lbmNyeXB0LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2VuY3J5cHQvcmV3cml0ZS9wYXJhbWV0ZXIvcmV3cml0ZXIvRW5jcnlwdFByZWRpY2F0ZVBhcmFtZXRlclJld3JpdGVyLmphdmE=) | `5.26% <0.00%> (ø)` | |
| [...enerator/EncryptPredicateColumnTokenGenerator.java](https://codecov.io/gh/apache/shardingsphere/pull/17603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtZmVhdHVyZXMvc2hhcmRpbmdzcGhlcmUtZW5jcnlwdC9zaGFyZGluZ3NwaGVyZS1lbmNyeXB0LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2VuY3J5cHQvcmV3cml0ZS90b2tlbi9nZW5lcmF0b3IvRW5jcnlwdFByZWRpY2F0ZUNvbHVtblRva2VuR2VuZXJhdG9yLmphdmE=) | `3.57% <0.00%> (ø)` | |
| [...ator/EncryptPredicateRightValueTokenGenerator.java](https://codecov.io/gh/apache/shardingsphere/pull/17603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtZmVhdHVyZXMvc2hhcmRpbmdzcGhlcmUtZW5jcnlwdC9zaGFyZGluZ3NwaGVyZS1lbmNyeXB0LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2VuY3J5cHQvcmV3cml0ZS90b2tlbi9nZW5lcmF0b3IvRW5jcnlwdFByZWRpY2F0ZVJpZ2h0VmFsdWVUb2tlbkdlbmVyYXRvci5qYXZh) | `2.94% <0.00%> (ø)` | |
| [...onfig/rule/YamlEncryptColumnRuleConfiguration.java](https://codecov.io/gh/apache/shardingsphere/pull/17603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtZmVhdHVyZXMvc2hhcmRpbmdzcGhlcmUtZW5jcnlwdC9zaGFyZGluZ3NwaGVyZS1lbmNyeXB0LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2VuY3J5cHQveWFtbC9jb25maWcvcnVsZS9ZYW1sRW5jcnlwdENvbHVtblJ1bGVDb25maWd1cmF0aW9uLmphdmE=) | `100.00% <ø> (ø)` | |
| [...ken/generator/EncryptProjectionTokenGenerator.java](https://codecov.io/gh/apache/shardingsphere/pull/17603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtZmVhdHVyZXMvc2hhcmRpbmdzcGhlcmUtZW5jcnlwdC9zaGFyZGluZ3NwaGVyZS1lbmNyeXB0LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2VuY3J5cHQvcmV3cml0ZS90b2tlbi9nZW5lcmF0b3IvRW5jcnlwdFByb2plY3Rpb25Ub2tlbkdlbmVyYXRvci5qYXZh) | `34.00% <50.00%> (ø)` | |
| [...re/encrypt/merge/dql/EncryptAlgorithmMetaData.java](https://codecov.io/gh/apache/shardingsphere/pull/17603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtZmVhdHVyZXMvc2hhcmRpbmdzcGhlcmUtZW5jcnlwdC9zaGFyZGluZ3NwaGVyZS1lbmNyeXB0LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2VuY3J5cHQvbWVyZ2UvZHFsL0VuY3J5cHRBbGdvcml0aG1NZXRhRGF0YS5qYXZh) | `82.60% <100.00%> (ø)` | |
| [...gsphere/encrypt/merge/dql/EncryptMergedResult.java](https://codecov.io/gh/apache/shardingsphere/pull/17603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtZmVhdHVyZXMvc2hhcmRpbmdzcGhlcmUtZW5jcnlwdC9zaGFyZGluZ3NwaGVyZS1lbmNyeXB0LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2VuY3J5cHQvbWVyZ2UvZHFsL0VuY3J5cHRNZXJnZWRSZXN1bHQuamF2YQ==) | `100.00% <100.00%> (ø)` | |
| [...en/generator/EncryptOrderByItemTokenGenerator.java](https://codecov.io/gh/apache/shardingsphere/pull/17603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtZmVhdHVyZXMvc2hhcmRpbmdzcGhlcmUtZW5jcnlwdC9zaGFyZGluZ3NwaGVyZS1lbmNyeXB0LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2VuY3J5cHQvcmV3cml0ZS90b2tlbi9nZW5lcmF0b3IvRW5jcnlwdE9yZGVyQnlJdGVtVG9rZW5HZW5lcmF0b3IuamF2YQ==) | `58.49% <100.00%> (ø)` | |
| [...che/shardingsphere/encrypt/rule/EncryptColumn.java](https://codecov.io/gh/apache/shardingsphere/pull/17603/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation#diff-c2hhcmRpbmdzcGhlcmUtZmVhdHVyZXMvc2hhcmRpbmdzcGhlcmUtZW5jcnlwdC9zaGFyZGluZ3NwaGVyZS1lbmNyeXB0LWNvcmUvc3JjL21haW4vamF2YS9vcmcvYXBhY2hlL3NoYXJkaW5nc3BoZXJlL2VuY3J5cHQvcnVsZS9FbmNyeXB0Q29sdW1uLmphdmE=) | `75.00% <100.00%> (+25.00%)` | :arrow_up: |
| ... and [4 more](https://codecov.io/gh/apache/shardingsphere/pull/17603/diff?src=pr&el=tree-more&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation) | |
------
[Continue to review full report at Codecov](https://codecov.io/gh/apache/shardingsphere/pull/17603?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/apache/shardingsphere/pull/17603?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Last update [a0e561d...1a334fb](https://codecov.io/gh/apache/shardingsphere/pull/17603?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=The+Apache+Software+Foundation).
--
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
[GitHub] [shardingsphere] zhaoguhong commented on a diff in pull request #17603: Support encrypt config queryWithCipherColumn with column levels
Posted by GitBox <gi...@apache.org>.
zhaoguhong commented on code in PR #17603:
URL: https://github.com/apache/shardingsphere/pull/17603#discussion_r875485698
##########
shardingsphere-features/shardingsphere-encrypt/shardingsphere-encrypt-core/src/main/java/org/apache/shardingsphere/encrypt/rule/EncryptTable.java:
##########
@@ -196,4 +206,5 @@ public Optional<Boolean> getQueryWithCipherColumn() {
public Optional<EncryptColumn> findEncryptColumn(final String logicColumn) {
return Optional.ofNullable(columns.get(logicColumn));
}
+
Review Comment:
ok
##########
shardingsphere-features/shardingsphere-encrypt/shardingsphere-encrypt-core/src/main/java/org/apache/shardingsphere/encrypt/rule/EncryptTable.java:
##########
@@ -187,6 +187,16 @@ public Optional<Boolean> getQueryWithCipherColumn() {
return Optional.ofNullable(queryWithCipherColumn);
}
+ /**
+ * Get query with cipher column.
+ *
+ * @param logicColumn logic column
+ * @return query with cipher column
+ */
+ public Optional<Boolean> getQueryWithCipherColumn(final String logicColumn) {
Review Comment:
yes, It can be
```java
public Optional<Boolean> getQueryWithCipherColumn(final String logicColumn) {
return Optional.ofNullable(findEncryptColumn(logicColumn).map(EncryptColumn::getQueryWithCipherColumn).orElse(queryWithCipherColumn));
}
```
--
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
[GitHub] [shardingsphere] zhaoguhong commented on a diff in pull request #17603: Support encrypt config queryWithCipherColumn with column levels
Posted by GitBox <gi...@apache.org>.
zhaoguhong commented on code in PR #17603:
URL: https://github.com/apache/shardingsphere/pull/17603#discussion_r875488498
##########
shardingsphere-features/shardingsphere-encrypt/shardingsphere-encrypt-core/src/main/java/org/apache/shardingsphere/encrypt/rule/EncryptTable.java:
##########
@@ -48,7 +48,7 @@ public EncryptTable(final EncryptTableRuleConfiguration config, final Map<String
columns.put(each.getLogicColumn(), new EncryptColumn(getEncryptColumnDataType(each.getLogicDataType(), dataTypes), each.getCipherColumn(),
getEncryptColumnDataType(each.getCipherDataType(), dataTypes), each.getAssistedQueryColumn(), getEncryptColumnDataType(each.getAssistedQueryDataType(),
dataTypes),
- each.getPlainColumn(), getEncryptColumnDataType(each.getPlainDataType(), dataTypes), each.getEncryptorName()));
+ each.getPlainColumn(), getEncryptColumnDataType(each.getPlainDataType(), dataTypes), each.getEncryptorName(), each.getQueryWithCipherColumn()));
Review Comment:
I think that's ok， because I have modified `org.apache.shardingsphere.encrypt.rule.EncryptTable#getQueryWithCipherColumn(java.lang.String)`
--
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
[GitHub] [shardingsphere] zhaoguhong commented on a diff in pull request #17603: Support encrypt config queryWithCipherColumn with column levels
Posted by GitBox <gi...@apache.org>.
zhaoguhong commented on code in PR #17603:
URL: https://github.com/apache/shardingsphere/pull/17603#discussion_r875491162
##########
shardingsphere-features/shardingsphere-encrypt/shardingsphere-encrypt-core/src/main/java/org/apache/shardingsphere/encrypt/rule/EncryptColumn.java:
##########
@@ -47,6 +47,13 @@ public final class EncryptColumn {
private final String encryptorName;
+ private final Boolean queryWithCipherColumn;
+
+ public EncryptColumn(final EncryptColumnDataType logicDataType, final String cipherColumn, final EncryptColumnDataType cipherDataType, final String assistedQueryColumn,
Review Comment:
ok
--
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
[GitHub] [shardingsphere] zhaoguhong commented on a diff in pull request #17603: Support encrypt config queryWithCipherColumn with column levels
Posted by GitBox <gi...@apache.org>.
zhaoguhong commented on code in PR #17603:
URL: https://github.com/apache/shardingsphere/pull/17603#discussion_r875602091
##########
shardingsphere-features/shardingsphere-encrypt/shardingsphere-encrypt-core/src/main/java/org/apache/shardingsphere/encrypt/rule/EncryptRule.java:
##########
@@ -294,6 +294,17 @@ public boolean isQueryWithCipherColumn(final String tableName) {
return findEncryptTable(tableName).flatMap(EncryptTable::getQueryWithCipherColumn).orElse(queryWithCipherColumn);
}
+ /**
+ * Judge whether column is support QueryWithCipherColumn or not.
+ *
+ * @param logicTable logic table name
+ * @param logicColumn logic column name
+ * @return whether column is support QueryWithCipherColumn or not
+ */
+ public boolean isQueryWithCipherColumn(final String logicTable, final String logicColumn) {
Review Comment:
yes, It can be, and I removed `isQueryWithCipherColumn(logicTable) `
```java
public boolean isQueryWithCipherColumn(final String logicTable, final String logicColumn) {
return findEncryptTable(logicTable).flatMap(encryptTable -> encryptTable.getQueryWithCipherColumn(logicColumn)).orElse(queryWithCipherColumn);
}
```
--
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
[GitHub] [shardingsphere] zhaoguhong commented on a diff in pull request #17603: Support encrypt config queryWithCipherColumn with column levels
Posted by GitBox <gi...@apache.org>.
zhaoguhong commented on code in PR #17603:
URL: https://github.com/apache/shardingsphere/pull/17603#discussion_r880018956
##########
shardingsphere-features/shardingsphere-encrypt/shardingsphere-encrypt-core/src/test/java/org/apache/shardingsphere/encrypt/rule/EncryptRuleTest.java:
##########
@@ -143,12 +143,34 @@ public void assertFindPlainColumn() {
@Test
public void assertIsQueryWithCipherColumn() {
- EncryptColumnRuleConfiguration encryptColumnConfig = new EncryptColumnRuleConfiguration("encrypt_column", "encrypt_cipher", "", "", "test_encryptor");
- EncryptTableRuleConfiguration tableConfig = new EncryptTableRuleConfiguration("t_encrypt", Collections.singletonList(encryptColumnConfig), true);
+ // global(true) -> table(null) -> column(null)
Review Comment:
ok
--
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
[GitHub] [shardingsphere] strongduanmu merged pull request #17603: Support encrypt config queryWithCipherColumn with column levels
Posted by GitBox <gi...@apache.org>.
strongduanmu merged PR #17603:
URL: https://github.com/apache/shardingsphere/pull/17603
--
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
[GitHub] [shardingsphere] zhaoguhong commented on pull request #17603: Support encrypt config queryWithCipherColumn with column levels
Posted by GitBox <gi...@apache.org>.
zhaoguhong commented on PR #17603:
URL: https://github.com/apache/shardingsphere/pull/17603#issuecomment-1138209060
> Hi @zhaoguhong After this PR is completed, the encrypt feature supports `queryWithCipherColumn` configuration at the column level, and users can control encryption and decryption in a more fine-grained manner. Also, is it possible to continue with the task of adding test cases, examples, and documentation?
ok
--
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
[GitHub] [shardingsphere] zhaoguhong commented on pull request #17603: Support encrypt config queryWithCipherColumn with column levels
Posted by GitBox <gi...@apache.org>.
zhaoguhong commented on PR #17603:
URL: https://github.com/apache/shardingsphere/pull/17603#issuecomment-1128604564
> Please resolve the ci failure and edit the PR title for search convenience. @zhaoguhong
Ok, now it's ready
--
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
[GitHub] [shardingsphere] zhaoguhong commented on a diff in pull request #17603: Support encrypt config queryWithCipherColumn with column levels
Posted by GitBox <gi...@apache.org>.
zhaoguhong commented on code in PR #17603:
URL: https://github.com/apache/shardingsphere/pull/17603#discussion_r875489643
##########
shardingsphere-features/shardingsphere-encrypt/shardingsphere-encrypt-core/src/main/java/org/apache/shardingsphere/encrypt/rule/EncryptColumn.java:
##########
@@ -64,4 +71,5 @@ public Optional<String> getAssistedQueryColumn() {
public Optional<String> getPlainColumn() {
return Strings.isNullOrEmpty(plainColumn) ? Optional.empty() : Optional.of(plainColumn);
}
+
Review Comment:
ok
--
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
[GitHub] [shardingsphere] cheese8 commented on a diff in pull request #17603: Support encrypt config queryWithCipherColumn with column levels
Posted by GitBox <gi...@apache.org>.
cheese8 commented on code in PR #17603:
URL: https://github.com/apache/shardingsphere/pull/17603#discussion_r875821678
##########
shardingsphere-features/shardingsphere-encrypt/shardingsphere-encrypt-core/src/main/java/org/apache/shardingsphere/encrypt/rule/EncryptTable.java:
##########
@@ -48,7 +48,7 @@ public EncryptTable(final EncryptTableRuleConfiguration config, final Map<String
columns.put(each.getLogicColumn(), new EncryptColumn(getEncryptColumnDataType(each.getLogicDataType(), dataTypes), each.getCipherColumn(),
getEncryptColumnDataType(each.getCipherDataType(), dataTypes), each.getAssistedQueryColumn(), getEncryptColumnDataType(each.getAssistedQueryDataType(),
dataTypes),
- each.getPlainColumn(), getEncryptColumnDataType(each.getPlainDataType(), dataTypes), each.getEncryptorName()));
+ each.getPlainColumn(), getEncryptColumnDataType(each.getPlainDataType(), dataTypes), each.getEncryptorName(), each.getQueryWithCipherColumn()));
Review Comment:
In addition, could you add some test to ensure that the `isQueryWithCipherColumn ` on column compare to the `isQueryWithCipherColumn ` on table, the performance was not reduced.
--
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
[GitHub] [shardingsphere] zhaoguhong commented on a diff in pull request #17603: Support encrypt config queryWithCipherColumn with column levels
Posted by GitBox <gi...@apache.org>.
zhaoguhong commented on code in PR #17603:
URL: https://github.com/apache/shardingsphere/pull/17603#discussion_r875592477
##########
shardingsphere-features/shardingsphere-encrypt/shardingsphere-encrypt-api/src/main/java/org/apache/shardingsphere/encrypt/api/config/rule/EncryptColumnRuleConfiguration.java:
##########
@@ -45,7 +45,19 @@ public final class EncryptColumnRuleConfiguration {
private final String encryptorName;
+ private final Boolean queryWithCipherColumn;
+
public EncryptColumnRuleConfiguration(final String logicColumn, final String cipherColumn, final String assistedQueryColumn, final String plainColumn, final String encryptorName) {
- this(logicColumn, null, cipherColumn, null, assistedQueryColumn, null, plainColumn, null, encryptorName);
+ this(logicColumn, null, cipherColumn, null, assistedQueryColumn, null, plainColumn, null, encryptorName, null);
+ }
+
+ public EncryptColumnRuleConfiguration(final String logicColumn, final String cipherColumn, final String assistedQueryColumn, final String plainColumn, final String encryptorName,
+ final Boolean queryWithCipherColumn) {
+ this(logicColumn, null, cipherColumn, null, assistedQueryColumn, null, plainColumn, null, encryptorName, queryWithCipherColumn);
+ }
+
+ public EncryptColumnRuleConfiguration(final String logicColumn, final String logicDataType, final String cipherColumn, final String cipherDataType, final String assistedQueryColumn,
Review Comment:
ok
--
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
[GitHub] [shardingsphere] zhaoguhong commented on a diff in pull request #17603: Support encrypt config queryWithCipherColumn with column levels
Posted by GitBox <gi...@apache.org>.
zhaoguhong commented on code in PR #17603:
URL: https://github.com/apache/shardingsphere/pull/17603#discussion_r880018830
##########
shardingsphere-features/shardingsphere-encrypt/shardingsphere-encrypt-core/src/test/java/org/apache/shardingsphere/encrypt/rule/EncryptRuleTest.java:
##########
@@ -143,12 +143,34 @@ public void assertFindPlainColumn() {
@Test
public void assertIsQueryWithCipherColumn() {
- EncryptColumnRuleConfiguration encryptColumnConfig = new EncryptColumnRuleConfiguration("encrypt_column", "encrypt_cipher", "", "", "test_encryptor");
- EncryptTableRuleConfiguration tableConfig = new EncryptTableRuleConfiguration("t_encrypt", Collections.singletonList(encryptColumnConfig), true);
+ // global(true) -> table(null) -> column(null)
+ EncryptColumnRuleConfiguration encryptColumnConfig = new EncryptColumnRuleConfiguration("encrypt_column", "encrypt_cipher", "", "", "test_encryptor", null);
+ EncryptTableRuleConfiguration tableConfig = new EncryptTableRuleConfiguration("t_encrypt", Collections.singletonList(encryptColumnConfig), null);
AlgorithmProvidedEncryptRuleConfiguration ruleConfig = new AlgorithmProvidedEncryptRuleConfiguration(
Collections.singleton(tableConfig), Collections.singletonMap("test_encryptor", new CoreEncryptAlgorithmFixture()), true);
EncryptRule actual = new EncryptRule(ruleConfig, Collections.emptyMap());
- assertTrue(actual.isQueryWithCipherColumn("t_encrypt"));
+ assertTrue(actual.isQueryWithCipherColumn("t_encrypt", "encrypt_column"));
+
+ // global(true) -> table(false) -> column(null)
+ encryptColumnConfig = new EncryptColumnRuleConfiguration("encrypt_column", "encrypt_cipher", "", "", "test_encryptor", null);
+ tableConfig = new EncryptTableRuleConfiguration("t_encrypt", Collections.singletonList(encryptColumnConfig), false);
+ ruleConfig = new AlgorithmProvidedEncryptRuleConfiguration(Collections.singleton(tableConfig), Collections.singletonMap("test_encryptor", new CoreEncryptAlgorithmFixture()), true);
+ actual = new EncryptRule(ruleConfig, Collections.emptyMap());
+ assertFalse(actual.isQueryWithCipherColumn("t_encrypt", "encrypt_column"));
+
+ // global(true) -> table(false) -> column(true)
Review Comment:
ok
##########
shardingsphere-features/shardingsphere-encrypt/shardingsphere-encrypt-core/src/test/java/org/apache/shardingsphere/encrypt/rule/EncryptRuleTest.java:
##########
@@ -143,12 +143,34 @@ public void assertFindPlainColumn() {
@Test
public void assertIsQueryWithCipherColumn() {
- EncryptColumnRuleConfiguration encryptColumnConfig = new EncryptColumnRuleConfiguration("encrypt_column", "encrypt_cipher", "", "", "test_encryptor");
- EncryptTableRuleConfiguration tableConfig = new EncryptTableRuleConfiguration("t_encrypt", Collections.singletonList(encryptColumnConfig), true);
+ // global(true) -> table(null) -> column(null)
+ EncryptColumnRuleConfiguration encryptColumnConfig = new EncryptColumnRuleConfiguration("encrypt_column", "encrypt_cipher", "", "", "test_encryptor", null);
+ EncryptTableRuleConfiguration tableConfig = new EncryptTableRuleConfiguration("t_encrypt", Collections.singletonList(encryptColumnConfig), null);
AlgorithmProvidedEncryptRuleConfiguration ruleConfig = new AlgorithmProvidedEncryptRuleConfiguration(
Collections.singleton(tableConfig), Collections.singletonMap("test_encryptor", new CoreEncryptAlgorithmFixture()), true);
EncryptRule actual = new EncryptRule(ruleConfig, Collections.emptyMap());
- assertTrue(actual.isQueryWithCipherColumn("t_encrypt"));
+ assertTrue(actual.isQueryWithCipherColumn("t_encrypt", "encrypt_column"));
+
+ // global(true) -> table(false) -> column(null)
Review Comment:
ok
--
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
[GitHub] [shardingsphere] zhaoguhong commented on pull request #17603: Support encrypt config queryWithCipherColumn with column levels
Posted by GitBox <gi...@apache.org>.
zhaoguhong commented on PR #17603:
URL: https://github.com/apache/shardingsphere/pull/17603#issuecomment-1135352614
> Is the query with cipher column has a default value?
The global level defaults is true
--
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