strongduanmu commented on a change in pull request #12504:
URL: https://github.com/apache/shardingsphere/pull/12504#discussion_r713607277
##########
File path:
shardingsphere-features/shardingsphere-encrypt/shardingsphere-encrypt-core/src/main/java/org/apache/shardingsphere/encrypt/rewrite/token/generator/impl/EncryptPredicateColumnTokenGenerator.java
##########
@@ -75,38 +78,54 @@ protected boolean isGenerateSQLTokenForEncrypt(final
SQLStatementContext sqlStat
private Collection<SubstitutableColumnNameToken> generateSQLTokens(final
Collection<ExpressionSegment> predicates, final Map<String, String>
columnTableNames) {
Collection<SubstitutableColumnNameToken> result = new LinkedList<>();
for (ExpressionSegment each : predicates) {
- Optional<ColumnSegment> column = ColumnExtractor.extract(each);
- if (!column.isPresent()) {
+
result.addAll(processColumnSegment(ColumnExtractor.extractAll(each),
columnTableNames));
+ }
+ return result;
+ }
+
+ private List<SubstitutableColumnNameToken> processColumnSegment(final
Collection<Optional<ColumnSegment>> columnSegments, final Map<String, String>
columnTableNames) {
Review comment:
@cheese8 Do you think `generateSQLTokens` is better?
##########
File path:
shardingsphere-features/shardingsphere-encrypt/shardingsphere-encrypt-core/src/main/java/org/apache/shardingsphere/encrypt/rewrite/token/generator/impl/EncryptPredicateColumnTokenGenerator.java
##########
@@ -75,38 +78,54 @@ protected boolean isGenerateSQLTokenForEncrypt(final
SQLStatementContext sqlStat
private Collection<SubstitutableColumnNameToken> generateSQLTokens(final
Collection<ExpressionSegment> predicates, final Map<String, String>
columnTableNames) {
Collection<SubstitutableColumnNameToken> result = new LinkedList<>();
for (ExpressionSegment each : predicates) {
- Optional<ColumnSegment> column = ColumnExtractor.extract(each);
- if (!column.isPresent()) {
+
result.addAll(processColumnSegment(ColumnExtractor.extractAll(each),
columnTableNames));
+ }
+ return result;
+ }
+
+ private List<SubstitutableColumnNameToken> processColumnSegment(final
Collection<Optional<ColumnSegment>> columnSegments, final Map<String, String>
columnTableNames) {
+ List<SubstitutableColumnNameToken> result = new LinkedList<>();
+ for (Optional<ColumnSegment> each : columnSegments) {
+ if (!each.isPresent()) {
continue;
}
- Optional<EncryptTable> encryptTable =
findEncryptTable(columnTableNames, column.get());
- if (!encryptTable.isPresent() ||
!encryptTable.get().findEncryptorName(column.get().getIdentifier().getValue()).isPresent())
{
+ Optional<EncryptTable> encryptTable =
findEncryptTable(columnTableNames, each.get());
+ if (!encryptTable.isPresent() ||
!encryptTable.get().findEncryptorName(each.get().getIdentifier().getValue()).isPresent())
{
continue;
}
- int startIndex = column.get().getOwner().isPresent() ?
column.get().getOwner().get().getStopIndex() + 2 : column.get().getStartIndex();
- int stopIndex = column.get().getStopIndex();
+ int startIndex = each.get().getOwner().isPresent() ?
each.get().getOwner().get().getStopIndex() + 2 : each.get().getStartIndex();
+ int stopIndex = each.get().getStopIndex();
if (!queryWithCipherColumn) {
- Optional<String> plainColumn =
encryptTable.get().findPlainColumn(column.get().getIdentifier().getValue());
+ Optional<String> plainColumn =
encryptTable.get().findPlainColumn(each.get().getIdentifier().getValue());
if (plainColumn.isPresent()) {
result.add(new SubstitutableColumnNameToken(startIndex,
stopIndex, getColumnProjections(plainColumn.get())));
continue;
}
}
- Optional<String> assistedQueryColumn =
encryptTable.get().findAssistedQueryColumn(column.get().getIdentifier().getValue());
+ Optional<String> assistedQueryColumn =
encryptTable.get().findAssistedQueryColumn(each.get().getIdentifier().getValue());
SubstitutableColumnNameToken encryptColumnNameToken =
assistedQueryColumn.map(columnName
-> new SubstitutableColumnNameToken(startIndex, stopIndex,
getColumnProjections(columnName))).orElseGet(()
- -> new SubstitutableColumnNameToken(startIndex, stopIndex,
getColumnProjections(encryptTable.get().getCipherColumn(column.get().getIdentifier().getValue()))));
+ -> new SubstitutableColumnNameToken(startIndex, stopIndex,
getColumnProjections(encryptTable.get().getCipherColumn(each.get().getIdentifier().getValue()))));
result.add(encryptColumnNameToken);
}
return result;
}
private Map<String, String> getColumnTableNames(final SQLStatementContext
sqlStatementContext, final Collection<AndPredicate> andPredicates) {
- Collection<ColumnSegment> columns =
andPredicates.stream().flatMap(each -> each.getPredicates().stream())
- .map(each ->
ColumnExtractor.extract(each).orElse(null)).filter(Objects::nonNull).collect(Collectors.toList());
+ Collection<ColumnSegment> columns = new ArrayList<ColumnSegment>();
+ andPredicates.forEach(each ->
columns.addAll(processExpressionSegment(each.getPredicates())));
return sqlStatementContext.getTablesContext().findTableName(columns,
schema);
}
+ private Collection<ColumnSegment> processExpressionSegment(final
Collection<ExpressionSegment> predicates) {
Review comment:
@cheese8 Please use stream api to replace this method
##########
File path:
shardingsphere-features/shardingsphere-encrypt/shardingsphere-encrypt-core/src/main/java/org/apache/shardingsphere/encrypt/rewrite/token/generator/impl/EncryptProjectionTokenGenerator.java
##########
@@ -88,6 +111,71 @@ protected boolean isGenerateSQLTokenForEncrypt(final
SQLStatementContext sqlStat
return result;
}
+ private List<SubstitutableColumnNameToken> distinct(final
List<SubstitutableColumnNameToken> substitutableColumnNameTokenList) {
+ Map<String, SubstitutableColumnNameToken> distinctMap = new
HashMap<>();
+ for (SubstitutableColumnNameToken each :
substitutableColumnNameTokenList) {
+ String key = each.getStartIndex() + "," + each.getStartIndex();
+ if (!distinctMap.containsKey(key)) {
+ distinctMap.put(key, each);
+ }
+ }
+ return new LinkedList<>(distinctMap.values());
+ }
+
+ private Collection<SubstitutableColumnNameToken>
processJoinTableSegments(final SelectStatementContext selectStatementContext,
Review comment:
@cheese8 `generateSQLTokens` may be better.
##########
File path:
shardingsphere-test/shardingsphere-rewrite-test/src/test/resources/scenario/mix/case/select_for_query_with_cipher.xml
##########
@@ -56,14 +56,14 @@
<rewrite-assertion
id="select_with_sharding_qualified_shorthand_join_table">
<input sql="SELECT b.* FROM t_account a, t_account_detail b where
a.password = b.password" />
- <output sql="SELECT b.* FROM t_account_0 a, t_account_detail_0 b where
a.assisted_query_password = b.password" />
- <output sql="SELECT b.* FROM t_account_1 a, t_account_detail_1 b where
a.assisted_query_password = b.password" />
+ <output sql="SELECT b.* FROM t_account_0 a, t_account_detail_0 b where
a.assisted_query_password = b.assisted_query_password" />
Review comment:
@cheese8 I think this pr is a big change, can we add more test case for
this pr?
##########
File path:
shardingsphere-features/shardingsphere-encrypt/shardingsphere-encrypt-core/src/main/java/org/apache/shardingsphere/encrypt/rewrite/token/generator/impl/EncryptProjectionTokenGenerator.java
##########
@@ -63,10 +74,22 @@ protected boolean isGenerateSQLTokenForEncrypt(final
SQLStatementContext sqlStat
@Override
public Collection<SubstitutableColumnNameToken> generateSQLTokens(final
SelectStatementContext selectStatementContext) {
ProjectionsSegment projectionsSegment =
selectStatementContext.getSqlStatement().getProjections();
- // TODO process multiple tables
- String tableName =
selectStatementContext.getAllTables().iterator().next().getTableName().getIdentifier().getValue();
- return getEncryptRule().findEncryptTable(tableName).map(
- encryptTable -> generateSQLTokens(projectionsSegment, tableName,
selectStatementContext, encryptTable)).orElseGet(Collections::emptyList);
+ Collection<SimpleTableSegment> simpleTableSegments =
selectStatementContext.getAllTables();
+ Map<String, SimpleTableSegment> tableSegmentMap =
selectStatementContext.getTablesContext().getUniqueTables();
+ List<SubstitutableColumnNameToken> substitutableColumnNameTokenList =
new ArrayList<>();
+ for (SimpleTableSegment simpleTableSegment : tableSegmentMap.values())
{
+ String tableName =
simpleTableSegment.getTableName().getIdentifier().getValue();
+ Optional<EncryptTable> encryptTable =
getEncryptRule().findEncryptTable(tableName);
+ if (encryptTable.isPresent()) {
+ Collection<SubstitutableColumnNameToken> sqlTokens =
generateSQLTokens(projectionsSegment, tableName, selectStatementContext,
encryptTable.get());
+ substitutableColumnNameTokenList.addAll(sqlTokens);
+ }
+ }
+ if (selectStatementContext.isContainsJoinQuery()) {
+
substitutableColumnNameTokenList.addAll(processJoinTableSegments(selectStatementContext,
simpleTableSegments));
+ }
+
+ return
Collections.unmodifiableList(distinct(substitutableColumnNameTokenList));
Review comment:
@cheese8 Why use `Collections.unmodifiableList`?
##########
File path:
shardingsphere-sql-parser/shardingsphere-sql-parser-statement/src/main/java/org/apache/shardingsphere/sql/parser/sql/common/util/ColumnExtractor.java
##########
@@ -54,4 +59,33 @@
}
return Optional.empty();
}
+
+ /**
+ * Get left and right value if either value of expression is column
segment.
+ *
+ * @param expression expression segment
+ * @return column segment collection
+ */
+ public static Collection<Optional<ColumnSegment>> extractAll(final
ExpressionSegment expression) {
Review comment:
@cheese8 Please use Collection<ColumnSegment> return value.
##########
File path:
shardingsphere-features/shardingsphere-encrypt/shardingsphere-encrypt-core/src/main/java/org/apache/shardingsphere/encrypt/rewrite/token/generator/impl/EncryptPredicateColumnTokenGenerator.java
##########
@@ -75,38 +78,54 @@ protected boolean isGenerateSQLTokenForEncrypt(final
SQLStatementContext sqlStat
private Collection<SubstitutableColumnNameToken> generateSQLTokens(final
Collection<ExpressionSegment> predicates, final Map<String, String>
columnTableNames) {
Collection<SubstitutableColumnNameToken> result = new LinkedList<>();
for (ExpressionSegment each : predicates) {
- Optional<ColumnSegment> column = ColumnExtractor.extract(each);
- if (!column.isPresent()) {
+
result.addAll(processColumnSegment(ColumnExtractor.extractAll(each),
columnTableNames));
+ }
+ return result;
+ }
+
+ private List<SubstitutableColumnNameToken> processColumnSegment(final
Collection<Optional<ColumnSegment>> columnSegments, final Map<String, String>
columnTableNames) {
+ List<SubstitutableColumnNameToken> result = new LinkedList<>();
+ for (Optional<ColumnSegment> each : columnSegments) {
+ if (!each.isPresent()) {
continue;
}
- Optional<EncryptTable> encryptTable =
findEncryptTable(columnTableNames, column.get());
- if (!encryptTable.isPresent() ||
!encryptTable.get().findEncryptorName(column.get().getIdentifier().getValue()).isPresent())
{
+ Optional<EncryptTable> encryptTable =
findEncryptTable(columnTableNames, each.get());
+ if (!encryptTable.isPresent() ||
!encryptTable.get().findEncryptorName(each.get().getIdentifier().getValue()).isPresent())
{
continue;
}
- int startIndex = column.get().getOwner().isPresent() ?
column.get().getOwner().get().getStopIndex() + 2 : column.get().getStartIndex();
- int stopIndex = column.get().getStopIndex();
+ int startIndex = each.get().getOwner().isPresent() ?
each.get().getOwner().get().getStopIndex() + 2 : each.get().getStartIndex();
+ int stopIndex = each.get().getStopIndex();
if (!queryWithCipherColumn) {
- Optional<String> plainColumn =
encryptTable.get().findPlainColumn(column.get().getIdentifier().getValue());
+ Optional<String> plainColumn =
encryptTable.get().findPlainColumn(each.get().getIdentifier().getValue());
if (plainColumn.isPresent()) {
result.add(new SubstitutableColumnNameToken(startIndex,
stopIndex, getColumnProjections(plainColumn.get())));
continue;
}
}
- Optional<String> assistedQueryColumn =
encryptTable.get().findAssistedQueryColumn(column.get().getIdentifier().getValue());
+ Optional<String> assistedQueryColumn =
encryptTable.get().findAssistedQueryColumn(each.get().getIdentifier().getValue());
SubstitutableColumnNameToken encryptColumnNameToken =
assistedQueryColumn.map(columnName
-> new SubstitutableColumnNameToken(startIndex, stopIndex,
getColumnProjections(columnName))).orElseGet(()
- -> new SubstitutableColumnNameToken(startIndex, stopIndex,
getColumnProjections(encryptTable.get().getCipherColumn(column.get().getIdentifier().getValue()))));
+ -> new SubstitutableColumnNameToken(startIndex, stopIndex,
getColumnProjections(encryptTable.get().getCipherColumn(each.get().getIdentifier().getValue()))));
result.add(encryptColumnNameToken);
}
return result;
}
private Map<String, String> getColumnTableNames(final SQLStatementContext
sqlStatementContext, final Collection<AndPredicate> andPredicates) {
- Collection<ColumnSegment> columns =
andPredicates.stream().flatMap(each -> each.getPredicates().stream())
- .map(each ->
ColumnExtractor.extract(each).orElse(null)).filter(Objects::nonNull).collect(Collectors.toList());
+ Collection<ColumnSegment> columns = new ArrayList<ColumnSegment>();
Review comment:
@cheese8 Can we use stream api to keep the logic concise?
##########
File path:
shardingsphere-features/shardingsphere-encrypt/shardingsphere-encrypt-core/src/main/java/org/apache/shardingsphere/encrypt/rewrite/token/generator/impl/EncryptProjectionTokenGenerator.java
##########
@@ -63,10 +74,22 @@ protected boolean isGenerateSQLTokenForEncrypt(final
SQLStatementContext sqlStat
@Override
public Collection<SubstitutableColumnNameToken> generateSQLTokens(final
SelectStatementContext selectStatementContext) {
ProjectionsSegment projectionsSegment =
selectStatementContext.getSqlStatement().getProjections();
- // TODO process multiple tables
- String tableName =
selectStatementContext.getAllTables().iterator().next().getTableName().getIdentifier().getValue();
- return getEncryptRule().findEncryptTable(tableName).map(
- encryptTable -> generateSQLTokens(projectionsSegment, tableName,
selectStatementContext, encryptTable)).orElseGet(Collections::emptyList);
+ Collection<SimpleTableSegment> simpleTableSegments =
selectStatementContext.getAllTables();
+ Map<String, SimpleTableSegment> tableSegmentMap =
selectStatementContext.getTablesContext().getUniqueTables();
+ List<SubstitutableColumnNameToken> substitutableColumnNameTokenList =
new ArrayList<>();
+ for (SimpleTableSegment simpleTableSegment : tableSegmentMap.values())
{
+ String tableName =
simpleTableSegment.getTableName().getIdentifier().getValue();
Review comment:
If we just want to use tableName, we can invoke
`tableContext.getTableNames()` rather than
`getTablesContext().getUniqueTables()`.
##########
File path:
shardingsphere-features/shardingsphere-encrypt/shardingsphere-encrypt-core/src/main/java/org/apache/shardingsphere/encrypt/rewrite/token/generator/impl/EncryptProjectionTokenGenerator.java
##########
@@ -88,6 +111,71 @@ protected boolean isGenerateSQLTokenForEncrypt(final
SQLStatementContext sqlStat
return result;
}
+ private List<SubstitutableColumnNameToken> distinct(final
List<SubstitutableColumnNameToken> substitutableColumnNameTokenList) {
Review comment:
@cheese8 Can we use set to remove duplicate data?
##########
File path:
shardingsphere-features/shardingsphere-encrypt/shardingsphere-encrypt-core/src/main/java/org/apache/shardingsphere/encrypt/rewrite/token/generator/impl/EncryptProjectionTokenGenerator.java
##########
@@ -88,6 +111,71 @@ protected boolean isGenerateSQLTokenForEncrypt(final
SQLStatementContext sqlStat
return result;
}
+ private List<SubstitutableColumnNameToken> distinct(final
List<SubstitutableColumnNameToken> substitutableColumnNameTokenList) {
+ Map<String, SubstitutableColumnNameToken> distinctMap = new
HashMap<>();
+ for (SubstitutableColumnNameToken each :
substitutableColumnNameTokenList) {
+ String key = each.getStartIndex() + "," + each.getStartIndex();
+ if (!distinctMap.containsKey(key)) {
+ distinctMap.put(key, each);
+ }
+ }
+ return new LinkedList<>(distinctMap.values());
+ }
+
+ private Collection<SubstitutableColumnNameToken>
processJoinTableSegments(final SelectStatementContext selectStatementContext,
+ final Collection<SimpleTableSegment> simpleTableSegments) {
+ List<SubstitutableColumnNameToken> substitutableColumnNameTokenList =
new ArrayList<>();
+ Map<String, String> aliasTable = new
HashMap<>(simpleTableSegments.size());
+ for (SimpleTableSegment simpleTableSegment : simpleTableSegments) {
+ String tableName =
simpleTableSegment.getTableName().getIdentifier().getValue();
+ String alias = tableName;
+ if (simpleTableSegment.getAlias().isPresent()) {
+ alias = simpleTableSegment.getAlias().get();
+ }
+ aliasTable.put(alias, tableName);
+ }
+ TableSegment tableSegment =
selectStatementContext.getSqlStatement().getFrom();
Review comment:
@cheese8 Please use common table extract logic by calling
`TableExtractor`.
##########
File path:
shardingsphere-features/shardingsphere-encrypt/shardingsphere-encrypt-core/src/main/java/org/apache/shardingsphere/encrypt/rewrite/token/generator/impl/EncryptPredicateColumnTokenGenerator.java
##########
@@ -75,38 +78,54 @@ protected boolean isGenerateSQLTokenForEncrypt(final
SQLStatementContext sqlStat
private Collection<SubstitutableColumnNameToken> generateSQLTokens(final
Collection<ExpressionSegment> predicates, final Map<String, String>
columnTableNames) {
Collection<SubstitutableColumnNameToken> result = new LinkedList<>();
for (ExpressionSegment each : predicates) {
- Optional<ColumnSegment> column = ColumnExtractor.extract(each);
- if (!column.isPresent()) {
+
result.addAll(processColumnSegment(ColumnExtractor.extractAll(each),
columnTableNames));
+ }
+ return result;
+ }
+
+ private List<SubstitutableColumnNameToken> processColumnSegment(final
Collection<Optional<ColumnSegment>> columnSegments, final Map<String, String>
columnTableNames) {
+ List<SubstitutableColumnNameToken> result = new LinkedList<>();
Review comment:
@cheese8 If you don't have to use List for operation, the Collection
type is better.
##########
File path:
shardingsphere-features/shardingsphere-encrypt/shardingsphere-encrypt-core/src/main/java/org/apache/shardingsphere/encrypt/rewrite/token/generator/impl/EncryptProjectionTokenGenerator.java
##########
@@ -63,10 +74,22 @@ protected boolean isGenerateSQLTokenForEncrypt(final
SQLStatementContext sqlStat
@Override
public Collection<SubstitutableColumnNameToken> generateSQLTokens(final
SelectStatementContext selectStatementContext) {
ProjectionsSegment projectionsSegment =
selectStatementContext.getSqlStatement().getProjections();
- // TODO process multiple tables
- String tableName =
selectStatementContext.getAllTables().iterator().next().getTableName().getIdentifier().getValue();
- return getEncryptRule().findEncryptTable(tableName).map(
- encryptTable -> generateSQLTokens(projectionsSegment, tableName,
selectStatementContext, encryptTable)).orElseGet(Collections::emptyList);
+ Collection<SimpleTableSegment> simpleTableSegments =
selectStatementContext.getAllTables();
+ Map<String, SimpleTableSegment> tableSegmentMap =
selectStatementContext.getTablesContext().getUniqueTables();
+ List<SubstitutableColumnNameToken> substitutableColumnNameTokenList =
new ArrayList<>();
+ for (SimpleTableSegment simpleTableSegment : tableSegmentMap.values())
{
Review comment:
@cheese8 I think `each` is better.
##########
File path:
shardingsphere-features/shardingsphere-encrypt/shardingsphere-encrypt-core/src/main/java/org/apache/shardingsphere/encrypt/rewrite/token/generator/impl/EncryptProjectionTokenGenerator.java
##########
@@ -88,6 +111,71 @@ protected boolean isGenerateSQLTokenForEncrypt(final
SQLStatementContext sqlStat
return result;
}
+ private List<SubstitutableColumnNameToken> distinct(final
List<SubstitutableColumnNameToken> substitutableColumnNameTokenList) {
+ Map<String, SubstitutableColumnNameToken> distinctMap = new
HashMap<>();
+ for (SubstitutableColumnNameToken each :
substitutableColumnNameTokenList) {
+ String key = each.getStartIndex() + "," + each.getStartIndex();
+ if (!distinctMap.containsKey(key)) {
+ distinctMap.put(key, each);
+ }
+ }
+ return new LinkedList<>(distinctMap.values());
+ }
+
+ private Collection<SubstitutableColumnNameToken>
processJoinTableSegments(final SelectStatementContext selectStatementContext,
+ final Collection<SimpleTableSegment> simpleTableSegments) {
+ List<SubstitutableColumnNameToken> substitutableColumnNameTokenList =
new ArrayList<>();
+ Map<String, String> aliasTable = new
HashMap<>(simpleTableSegments.size());
Review comment:
@cheese8 Can we extract this logic to a private method?
--
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: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
