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 2020/09/10 03:04:36 UTC
[GitHub] [shardingsphere] tristaZero commented on a change in pull request #7353: Improve the Statement definition
tristaZero commented on a change in pull request #7353:
URL: https://github.com/apache/shardingsphere/pull/7353#discussion_r486032788
##########
File path: shardingsphere-features/shardingsphere-encrypt/shardingsphere-encrypt-rewrite/src/main/java/org/apache/shardingsphere/encrypt/rewrite/condition/EncryptConditionEngine.java
##########
@@ -81,50 +102,72 @@
private Collection<EncryptCondition> createEncryptConditions(final SQLStatementContext sqlStatementContext, final AndPredicate andPredicate) {
Collection<EncryptCondition> result = new LinkedList<>();
Collection<Integer> stopIndexes = new HashSet<>();
- for (PredicateSegment predicate : andPredicate.getPredicates()) {
+ for (ExpressionSegment predicate : andPredicate.getPredicates()) {
if (stopIndexes.add(predicate.getStopIndex())) {
createEncryptCondition(sqlStatementContext, predicate).ifPresent(result::add);
}
}
return result;
}
- private Optional<EncryptCondition> createEncryptCondition(final SQLStatementContext sqlStatementContext, final PredicateSegment predicateSegment) {
- Optional<String> tableName = sqlStatementContext.getTablesContext().findTableName(predicateSegment.getColumn(), schemaMetaData);
- return tableName.isPresent() && encryptRule.findEncryptor(tableName.get(), predicateSegment.getColumn().getIdentifier().getValue()).isPresent()
- ? createEncryptCondition(predicateSegment, tableName.get()) : Optional.empty();
+ private Optional<EncryptCondition> createEncryptCondition(final SQLStatementContext sqlStatementContext, final ExpressionSegment expression) {
+ ColumnSegment column = null;
+ if (expression instanceof BinaryOperationExpression && ((BinaryOperationExpression) expression).getLeft() instanceof ColumnSegment) {
+ column = (ColumnSegment) ((BinaryOperationExpression) expression).getLeft();
+ } else if (expression instanceof InExpression && ((InExpression) expression).getLeft() instanceof ColumnSegment) {
+ column = (ColumnSegment) ((InExpression) expression).getLeft();
+ } else if (expression instanceof BetweenExpression && ((BetweenExpression) expression).getLeft() instanceof ColumnSegment) {
+ column = (ColumnSegment) ((BetweenExpression) expression).getLeft();
+ }
+ if (null == column) {
+ return Optional.empty();
+ }
+ Optional<String> tableName = sqlStatementContext.getTablesContext().findTableName(column, schemaMetaData);
+ return tableName.isPresent() && encryptRule.findEncryptor(tableName.get(), column.getIdentifier().getValue()).isPresent()
+ ? createEncryptCondition(expression, tableName.get()) : Optional.empty();
}
- private Optional<EncryptCondition> createEncryptCondition(final PredicateSegment predicateSegment, final String tableName) {
- if (predicateSegment.getRightValue() instanceof PredicateCompareRightValue) {
- PredicateCompareRightValue compareRightValue = (PredicateCompareRightValue) predicateSegment.getRightValue();
- return isSupportedOperator(compareRightValue.getOperator()) ? createCompareEncryptCondition(tableName, predicateSegment, compareRightValue) : Optional.empty();
+ private Optional<EncryptCondition> createEncryptCondition(final ExpressionSegment expression, final String tableName) {
+ if (expression instanceof BinaryOperationExpression) {
+ String operator = ((BinaryOperationExpression) expression).getOperator();
Review comment:
A concrete class for `operator` is recommended.
##########
File path: shardingsphere-features/shardingsphere-shadow/shardingsphere-shadow-common/src/main/java/org/apache/shardingsphere/shadow/condition/ShadowConditionEngine.java
##########
@@ -75,10 +98,18 @@
}
private Optional<ShadowCondition> createShadowCondition(final AndPredicate andPredicate) {
- for (PredicateSegment predicate : andPredicate.getPredicates()) {
+ for (ExpressionSegment predicate : andPredicate.getPredicates()) {
+ ColumnSegment column = null;
+ if (predicate instanceof BinaryOperationExpression && ((BinaryOperationExpression) predicate).getLeft() instanceof ColumnSegment) {
+ column = (ColumnSegment) ((BinaryOperationExpression) predicate).getLeft();
+ } else if (predicate instanceof InExpression && ((InExpression) predicate).getLeft() instanceof ColumnSegment) {
+ column = (ColumnSegment) ((InExpression) predicate).getLeft();
Review comment:
The similar handling appears many times, so do you think it is possible to exact a function?
##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-binder/src/main/java/org/apache/shardingsphere/sql/parser/binder/segment/insert/values/expression/DerivedParameterMarkerExpressionSegment.java
##########
@@ -31,4 +31,9 @@
public DerivedParameterMarkerExpressionSegment(final int parameterMarkerIndex) {
super(0, 0, parameterMarkerIndex);
}
+
+ @Override
+ public String getText() {
+ return null;
Review comment:
Why do we need this interface?
##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-dialect/shardingsphere-sql-parser-mysql/src/main/java/org/apache/shardingsphere/sql/parser/mysql/visitor/MySQLVisitor.java
##########
@@ -255,123 +254,185 @@ public final ASTNode visitExpr(final ExprContext ctx) {
if (null != ctx.booleanPrimary()) {
return visit(ctx.booleanPrimary());
}
+ if (null != ctx.LP_()) {
+ return visit(ctx.expr(0));
+ }
+ if (null != ctx.XOR()) {
+ BinaryOperationExpression result = new BinaryOperationExpression();
+ result.setStartIndex(ctx.start.getStartIndex());
+ result.setStopIndex(ctx.stop.getStopIndex());
+ result.setLeft((ExpressionSegment) visit(ctx.expr(0)));
+ result.setRight((ExpressionSegment) visit(ctx.expr(1)));
+ result.setOperator("XOR");
+ String text = ctx.start.getInputStream().getText(new Interval(ctx.start.getStartIndex(), ctx.stop.getStopIndex()));
+ result.setText(text);
+ return result;
+ }
if (null != ctx.logicalOperator()) {
- return new PredicateBuildUtils(visit(ctx.expr(0)), visit(ctx.expr(1)), ctx.logicalOperator().getText()).mergePredicate();
+ BinaryOperationExpression result = new BinaryOperationExpression();
+ result.setStartIndex(ctx.start.getStartIndex());
+ result.setStopIndex(ctx.stop.getStopIndex());
+ result.setLeft((ExpressionSegment) visit(ctx.expr(0)));
+ result.setRight((ExpressionSegment) visit(ctx.expr(1)));
+ result.setOperator(ctx.logicalOperator().getText());
+ String text = ctx.start.getInputStream().getText(new Interval(ctx.start.getStartIndex(), ctx.stop.getStopIndex()));
+ result.setText(text);
+ return result;
}
- // TODO deal with XOR
- return visit(ctx.expr().get(0));
+ NotExpression result = new NotExpression();
+ result.setStartIndex(ctx.start.getStartIndex());
+ result.setStopIndex(ctx.stop.getStopIndex());
+ result.setExpression((ExpressionSegment) visit(ctx.expr(0)));
+ String text = ctx.start.getInputStream().getText(new Interval(ctx.start.getStartIndex(), ctx.stop.getStopIndex()));
+ result.setText(text);
+ return result;
}
@Override
public final ASTNode visitBooleanPrimary(final BooleanPrimaryContext ctx) {
+ if (null != ctx.IS()) {
+ BinaryOperationExpression result = new BinaryOperationExpression();
+ result.setStartIndex(ctx.start.getStartIndex());
+ result.setStopIndex(ctx.stop.getStopIndex());
+ result.setLeft((ExpressionSegment) visit(ctx.booleanPrimary()));
+ result.setRight(new LiteralExpressionSegment(ctx.IS().getSymbol().getStopIndex() + 1, ctx.stop.getStopIndex(), new Interval(ctx.IS().getSymbol().getStopIndex() + 1,
+ ctx.stop.getStopIndex())));
+ result.setOperator("IS");
+ String text = ctx.start.getInputStream().getText(new Interval(ctx.start.getStartIndex(), ctx.stop.getStopIndex()));
+ result.setText(text);
+ return result;
+ }
if (null != ctx.comparisonOperator() || null != ctx.SAFE_EQ_()) {
return createCompareSegment(ctx);
}
- if (null != ctx.predicate()) {
- return visit(ctx.predicate());
- }
- if (null != ctx.subquery()) {
- return new SubquerySegment(ctx.getStart().getStartIndex(), ctx.getStop().getStopIndex(), (SelectStatement) visit(ctx.subquery()));
- }
- //TODO deal with IS NOT? (TRUE | FALSE | UNKNOWN | NULL)
- String text = ctx.start.getInputStream().getText(new Interval(ctx.start.getStartIndex(), ctx.stop.getStopIndex()));
- return new CommonExpressionSegment(ctx.getStart().getStartIndex(), ctx.getStop().getStopIndex(), text);
+ return visit(ctx.predicate());
}
private ASTNode createCompareSegment(final BooleanPrimaryContext ctx) {
- ASTNode leftValue = visit(ctx.booleanPrimary());
- if (!(leftValue instanceof ColumnSegment)) {
- return leftValue;
- }
- PredicateRightValue rightValue = (PredicateRightValue) createPredicateRightValue(ctx);
- return new PredicateSegment(ctx.getStart().getStartIndex(), ctx.getStop().getStopIndex(), (ColumnSegment) leftValue, rightValue);
- }
-
- private ASTNode createPredicateRightValue(final BooleanPrimaryContext ctx) {
- if (null != ctx.subquery()) {
- return new SubquerySegment(ctx.getStart().getStartIndex(), ctx.getStop().getStopIndex(), (SelectStatement) visit(ctx.subquery()));
- }
- ASTNode rightValue = visit(ctx.predicate());
- return createPredicateRightValue(ctx, rightValue);
- }
-
- private ASTNode createPredicateRightValue(final BooleanPrimaryContext ctx, final ASTNode rightValue) {
- if (rightValue instanceof ColumnSegment) {
- return rightValue;
+ BinaryOperationExpression result = new BinaryOperationExpression();
+ result.setStartIndex(ctx.start.getStartIndex());
+ result.setStopIndex(ctx.stop.getStopIndex());
+ result.setLeft((ExpressionSegment) visit(ctx.booleanPrimary()));
+ if (null != ctx.predicate()) {
+ result.setRight((ExpressionSegment) visit(ctx.predicate()));
+ } else {
+ result.setRight((ExpressionSegment) visit(ctx.subquery()));
}
- return rightValue instanceof SubquerySegment ? new PredicateCompareRightValue(ctx.subquery().start.getStartIndex(), ctx.subquery().stop.getStopIndex(),
- ctx.comparisonOperator().getText(), new SubqueryExpressionSegment((SubquerySegment) rightValue))
- : new PredicateCompareRightValue(ctx.predicate().start.getStartIndex(), ctx.predicate().stop.getStopIndex(), ctx.comparisonOperator().getText(),
- (ExpressionSegment) rightValue);
+ String operator = null != ctx.SAFE_EQ_() ? ctx.SAFE_EQ_().getText() : ctx.comparisonOperator().getText();
+ result.setOperator(operator);
+ String text = ctx.start.getInputStream().getText(new Interval(ctx.start.getStartIndex(), ctx.stop.getStopIndex()));
+ result.setText(text);
+ return result;
}
@Override
public final ASTNode visitPredicate(final PredicateContext ctx) {
- if (null != ctx.IN() && null == ctx.NOT()) {
+ if (null != ctx.IN()) {
return createInSegment(ctx);
}
- if (null != ctx.BETWEEN() && null == ctx.NOT()) {
+ if (null != ctx.BETWEEN()) {
return createBetweenSegment(ctx);
}
- if (1 == ctx.children.size()) {
- return visit(ctx.bitExpr(0));
+ if (null != ctx.LIKE()) {
+ return createBinaryOperationExpressionFromLike(ctx);
}
- return visitRemainPredicate(ctx);
+ if (null != ctx.REGEXP()) {
+ return createBinaryOperationExpressionFromRegexp(ctx);
+ }
+ return visit(ctx.bitExpr(0));
}
- private PredicateSegment createInSegment(final PredicateContext ctx) {
- ColumnSegment column = (ColumnSegment) visit(ctx.bitExpr(0));
- PredicateInRightValue predicateInRightValue = null != ctx.subquery() ? new PredicateInRightValue(ctx.subquery().start.getStartIndex(), ctx.subquery().stop.getStopIndex(),
- getExpressionSegments(ctx))
- : new PredicateInRightValue(ctx.LP_().getSymbol().getStartIndex(), ctx.RP_().getSymbol().getStopIndex(), getExpressionSegments(ctx));
- return new PredicateSegment(ctx.getStart().getStartIndex(), ctx.getStop().getStopIndex(), column, predicateInRightValue);
+ private InExpression createInSegment(final PredicateContext ctx) {
+ InExpression result = new InExpression();
+ result.setStartIndex(ctx.start.getStartIndex());
+ result.setStopIndex(ctx.stop.getStopIndex());
+ result.setLeft((ExpressionSegment) visit(ctx.bitExpr(0)));
+ if (null != ctx.subquery()) {
+ result.setRight(new SubqueryExpressionSegment(new SubquerySegment(ctx.subquery().start.getStartIndex(), ctx.subquery().stop.getStopIndex(), (SelectStatement) visit(ctx.subquery()))));
+ } else {
+ ListExpression listExpression = new ListExpression();
+ listExpression.setStartIndex(ctx.LP_().getSymbol().getStartIndex());
+ listExpression.setStopIndex(ctx.RP_().getSymbol().getStopIndex());
+ String text = ctx.start.getInputStream().getText(new Interval(ctx.LP_().getSymbol().getStartIndex(), ctx.RP_().getSymbol().getStopIndex()));
+ listExpression.setText(text);
+ for (ExprContext each : ctx.expr()) {
+ listExpression.getItems().add((ExpressionSegment) visit(each));
+ }
+ result.setRight(listExpression);
+ }
+ Boolean operator = null != ctx.NOT() ? true : false;
+ result.setNot(operator);
+ String text = ctx.start.getInputStream().getText(new Interval(ctx.start.getStartIndex(), ctx.stop.getStopIndex()));
+ result.setText(text);
+ return result;
}
- private Collection<ExpressionSegment> getExpressionSegments(final PredicateContext ctx) {
- Collection<ExpressionSegment> result = new LinkedList<>();
- if (null != ctx.subquery()) {
- SubqueryContext subquery = ctx.subquery();
- result.add(new SubqueryExpressionSegment(new SubquerySegment(subquery.getStart().getStartIndex(), subquery.getStop().getStopIndex(), (SelectStatement) visit(ctx.subquery()))));
- return result;
- }
- for (ExprContext each : ctx.expr()) {
- result.add((ExpressionSegment) visit(each));
- }
+ private BinaryOperationExpression createBinaryOperationExpressionFromLike(final PredicateContext ctx) {
+ BinaryOperationExpression result = new BinaryOperationExpression();
+ result.setStartIndex(ctx.start.getStartIndex());
+ result.setStopIndex(ctx.stop.getStopIndex());
+ result.setLeft((ExpressionSegment) visit(ctx.bitExpr(0)));
+ String operator;
+ if (null != ctx.SOUNDS()) {
+
Review comment:
Please remove it.
##########
File path: shardingsphere-sql-parser/shardingsphere-sql-parser-binder/src/main/java/org/apache/shardingsphere/sql/parser/binder/segment/table/TablesContext.java
##########
@@ -44,6 +45,18 @@ public TablesContext(final SimpleTableSegment tableSegment) {
this(null == tableSegment ? Collections.emptyList() : Collections.singletonList(tableSegment));
}
+ public TablesContext(final Collection<SimpleTableSegment> tableSegments) {
+ Map<String, SimpleTableSegment> tableMaps = new HashMap<>(1, 1);
+ Collection<SimpleTableSegment> realtables = new LinkedList<>();
+ for (SimpleTableSegment each : tableSegments) {
+ if (!tableMaps.containsKey(each.getTableName().getIdentifier().getValue())) {
+ tableMaps.put(each.getTableName().getIdentifier().getValue(), each);
+ realtables.add(each);
+ }
+ }
+ this.tables = realtables;
Review comment:
`realtable`? I suppose we do not have this concept. Do you mean `actualTable`?
----------------------------------------------------------------
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