You are viewing a plain text version of this content. The canonical link for it is here.
Posted to dev@doris.apache.org by GitBox <gi...@apache.org> on 2018/11/06 07:25:42 UTC
[GitHub] imay closed pull request #253: Fix view missed parenthesis bug
imay closed pull request #253: Fix view missed parenthesis bug
URL: https://github.com/apache/incubator-doris/pull/253
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git a/fe/src/main/cup/sql_parser.cup b/fe/src/main/cup/sql_parser.cup
index 24d5e4c3..aecb090a 100644
--- a/fe/src/main/cup/sql_parser.cup
+++ b/fe/src/main/cup/sql_parser.cup
@@ -3139,7 +3139,10 @@ non_pred_expr ::=
| arithmetic_expr:e
{: RESULT = e; :}
| LPAREN non_pred_expr:e RPAREN
- {: RESULT = e; :}
+ {:
+ e.setPrintSqlInParens(true);
+ RESULT = e;
+ :}
/* TODO(zc): add other trim function */
| KW_TRIM:id LPAREN function_params:params RPAREN
{: RESULT = new FunctionCallExpr(new FunctionName(null, id), params); :}
@@ -3365,7 +3368,10 @@ predicate ::=
| like_predicate:p
{: RESULT = p; :}
| LPAREN predicate:p RPAREN
- {: RESULT = p; :}
+ {:
+ p.setPrintSqlInParens(true);
+ RESULT = p;
+ :}
;
comparison_predicate ::=
diff --git a/fe/src/main/java/org/apache/doris/analysis/AnalyticExpr.java b/fe/src/main/java/org/apache/doris/analysis/AnalyticExpr.java
index b813e1b8..a02766d5 100644
--- a/fe/src/main/java/org/apache/doris/analysis/AnalyticExpr.java
+++ b/fe/src/main/java/org/apache/doris/analysis/AnalyticExpr.java
@@ -785,7 +785,7 @@ protected Expr substituteImpl(ExprSubstitutionMap sMap, Analyzer analyzer)
}
@Override
- public String toSql() {
+ public String toSqlImpl() {
if (sqlString != null) {
return sqlString;
}
diff --git a/fe/src/main/java/org/apache/doris/analysis/ArithmeticExpr.java b/fe/src/main/java/org/apache/doris/analysis/ArithmeticExpr.java
index 1d2c59f0..43c9e27f 100644
--- a/fe/src/main/java/org/apache/doris/analysis/ArithmeticExpr.java
+++ b/fe/src/main/java/org/apache/doris/analysis/ArithmeticExpr.java
@@ -150,7 +150,7 @@ public Expr clone() {
}
@Override
- public String toSql() {
+ public String toSqlImpl() {
if (children.size() == 1) {
return op.toString() + " " + getChild(0).toSql();
} else {
diff --git a/fe/src/main/java/org/apache/doris/analysis/BetweenPredicate.java b/fe/src/main/java/org/apache/doris/analysis/BetweenPredicate.java
index 2b0be6f7..939ad167 100644
--- a/fe/src/main/java/org/apache/doris/analysis/BetweenPredicate.java
+++ b/fe/src/main/java/org/apache/doris/analysis/BetweenPredicate.java
@@ -85,7 +85,7 @@ protected void toThrift(TExprNode msg) {
}
@Override
- public String toSql() {
+ public String toSqlImpl() {
String notStr = (isNotBetween) ? "NOT " : "";
return children.get(0).toSql() + " " + notStr + "BETWEEN " +
children.get(1).toSql() + " AND " + children.get(2).toSql();
diff --git a/fe/src/main/java/org/apache/doris/analysis/BinaryPredicate.java b/fe/src/main/java/org/apache/doris/analysis/BinaryPredicate.java
index d51711ee..00b9778d 100644
--- a/fe/src/main/java/org/apache/doris/analysis/BinaryPredicate.java
+++ b/fe/src/main/java/org/apache/doris/analysis/BinaryPredicate.java
@@ -219,7 +219,7 @@ public boolean equals(Object obj) {
}
@Override
- public String toSql() {
+ public String toSqlImpl() {
return getChild(0).toSql() + " " + op.toString() + " " + getChild(1).toSql();
}
diff --git a/fe/src/main/java/org/apache/doris/analysis/BoolLiteral.java b/fe/src/main/java/org/apache/doris/analysis/BoolLiteral.java
index 197663bc..53a8e500 100644
--- a/fe/src/main/java/org/apache/doris/analysis/BoolLiteral.java
+++ b/fe/src/main/java/org/apache/doris/analysis/BoolLiteral.java
@@ -78,7 +78,7 @@ public int compareLiteral(LiteralExpr expr) {
}
@Override
- public String toSql() {
+ public String toSqlImpl() {
return getStringValue();
}
diff --git a/fe/src/main/java/org/apache/doris/analysis/CaseExpr.java b/fe/src/main/java/org/apache/doris/analysis/CaseExpr.java
index 72844133..aae504e8 100644
--- a/fe/src/main/java/org/apache/doris/analysis/CaseExpr.java
+++ b/fe/src/main/java/org/apache/doris/analysis/CaseExpr.java
@@ -103,7 +103,7 @@ public boolean equals(Object obj) {
}
@Override
- public String toSql() {
+ public String toSqlImpl() {
StringBuilder output = new StringBuilder("CASE");
int childIdx = 0;
if (hasCaseExpr) {
diff --git a/fe/src/main/java/org/apache/doris/analysis/CastExpr.java b/fe/src/main/java/org/apache/doris/analysis/CastExpr.java
index bbcc0665..db02c845 100644
--- a/fe/src/main/java/org/apache/doris/analysis/CastExpr.java
+++ b/fe/src/main/java/org/apache/doris/analysis/CastExpr.java
@@ -154,7 +154,7 @@ public Expr clone() {
}
@Override
- public String toSql() {
+ public String toSqlImpl() {
if (isImplicit) {
return getChild(0).toSql();
}
diff --git a/fe/src/main/java/org/apache/doris/analysis/CompoundPredicate.java b/fe/src/main/java/org/apache/doris/analysis/CompoundPredicate.java
index 6e94355a..b847ba11 100644
--- a/fe/src/main/java/org/apache/doris/analysis/CompoundPredicate.java
+++ b/fe/src/main/java/org/apache/doris/analysis/CompoundPredicate.java
@@ -67,7 +67,7 @@ public boolean equals(Object obj) {
}
@Override
- public String toSql() {
+ public String toSqlImpl() {
if (children.size() == 1) {
Preconditions.checkState(op == Operator.NOT);
return "NOT " + getChild(0).toSql();
diff --git a/fe/src/main/java/org/apache/doris/analysis/DateLiteral.java b/fe/src/main/java/org/apache/doris/analysis/DateLiteral.java
index 37b9493d..f98a7fb8 100644
--- a/fe/src/main/java/org/apache/doris/analysis/DateLiteral.java
+++ b/fe/src/main/java/org/apache/doris/analysis/DateLiteral.java
@@ -144,7 +144,7 @@ public int compareLiteral(LiteralExpr expr) {
}
@Override
- public String toSql() {
+ public String toSqlImpl() {
return "'" + getStringValue() + "'";
}
diff --git a/fe/src/main/java/org/apache/doris/analysis/DecimalLiteral.java b/fe/src/main/java/org/apache/doris/analysis/DecimalLiteral.java
index 69433e8d..a2b52afe 100644
--- a/fe/src/main/java/org/apache/doris/analysis/DecimalLiteral.java
+++ b/fe/src/main/java/org/apache/doris/analysis/DecimalLiteral.java
@@ -159,7 +159,7 @@ public int compareLiteral(LiteralExpr expr) {
}
@Override
- public String toSql() {
+ public String toSqlImpl() {
return getStringValue();
}
diff --git a/fe/src/main/java/org/apache/doris/analysis/ExistsPredicate.java b/fe/src/main/java/org/apache/doris/analysis/ExistsPredicate.java
index 6724e39d..24bfda8c 100644
--- a/fe/src/main/java/org/apache/doris/analysis/ExistsPredicate.java
+++ b/fe/src/main/java/org/apache/doris/analysis/ExistsPredicate.java
@@ -59,7 +59,7 @@ protected void toThrift(TExprNode msg) {
@Override
public Expr clone() { return new ExistsPredicate(this); }
- public String toSql() {
+ public String toSqlImpl() {
StringBuilder strBuilder = new StringBuilder();
if (notExists) {
strBuilder.append("NOT ");
diff --git a/fe/src/main/java/org/apache/doris/analysis/Expr.java b/fe/src/main/java/org/apache/doris/analysis/Expr.java
index 27281521..a69c9df3 100644
--- a/fe/src/main/java/org/apache/doris/analysis/Expr.java
+++ b/fe/src/main/java/org/apache/doris/analysis/Expr.java
@@ -232,6 +232,10 @@ public boolean apply(Expr arg) {
// Cached value of IsConstant(), set during analyze() and valid if isAnalyzed_ is true.
private boolean isConstant_;
+ // Flag to indicate whether to wrap this expr's toSql() in parenthesis. Set by parser.
+ // Needed for properly capturing expr precedences in the SQL string.
+ protected boolean printSqlInParens = false;
+
protected Expr() {
super();
type = Type.INVALID;
@@ -252,6 +256,7 @@ protected Expr(Expr other) {
opcode = other.opcode;
isConstant_ = other.isConstant_;
fn = other.fn;
+ printSqlInParens = other.printSqlInParens;
children = Expr.cloneList(other.children);
}
@@ -318,6 +323,14 @@ public Function getFn() {
return fn;
}
+ public boolean getPrintSqlInParens() {
+ return printSqlInParens;
+ }
+
+ public void setPrintSqlInParens(boolean b) {
+ printSqlInParens = b;
+ }
+
/**
* Perform semantic analysis of node and all of its children.
* Throws exception if any errors found.
@@ -781,7 +794,15 @@ public void computeOutputColumn(Analyzer analyzer) {
}
}
- public abstract String toSql();
+ public String toSql() {
+ return (printSqlInParens) ? "(" + toSqlImpl() + ")" : toSqlImpl();
+ }
+
+ /**
+ * Returns a SQL string representing this expr. Subclasses should override this method
+ * instead of toSql() to ensure that parenthesis are properly added around the toSql().
+ */
+ protected abstract String toSqlImpl();
public String toMySql() {
return toSql();
@@ -1363,7 +1384,10 @@ public static Expr pushNegationToOperands(Expr root) {
if (root instanceof CompoundPredicate) {
Expr left = pushNegationToOperands(root.getChild(0));
Expr right = pushNegationToOperands(root.getChild(1));
- return new CompoundPredicate(((CompoundPredicate)root).getOp(), left, right);
+ CompoundPredicate compoundPredicate =
+ new CompoundPredicate(((CompoundPredicate)root).getOp(), left, right);
+ compoundPredicate.setPrintSqlInParens(root.getPrintSqlInParens());
+ return compoundPredicate;
}
return root;
diff --git a/fe/src/main/java/org/apache/doris/analysis/FloatLiteral.java b/fe/src/main/java/org/apache/doris/analysis/FloatLiteral.java
index 296998b1..b84e0f42 100644
--- a/fe/src/main/java/org/apache/doris/analysis/FloatLiteral.java
+++ b/fe/src/main/java/org/apache/doris/analysis/FloatLiteral.java
@@ -121,7 +121,7 @@ public int compareLiteral(LiteralExpr expr) {
}
@Override
- public String toSql() {
+ public String toSqlImpl() {
return getStringValue();
}
diff --git a/fe/src/main/java/org/apache/doris/analysis/FunctionCallExpr.java b/fe/src/main/java/org/apache/doris/analysis/FunctionCallExpr.java
index 70e9e1a2..4150b0da 100644
--- a/fe/src/main/java/org/apache/doris/analysis/FunctionCallExpr.java
+++ b/fe/src/main/java/org/apache/doris/analysis/FunctionCallExpr.java
@@ -171,7 +171,7 @@ public boolean equals(Object obj) {
}
@Override
- public String toSql() {
+ public String toSqlImpl() {
StringBuilder sb = new StringBuilder();
sb.append(fnName).append("(");
if (fnParams.isStar()) {
diff --git a/fe/src/main/java/org/apache/doris/analysis/InPredicate.java b/fe/src/main/java/org/apache/doris/analysis/InPredicate.java
index 3493d3eb..7dd51860 100644
--- a/fe/src/main/java/org/apache/doris/analysis/InPredicate.java
+++ b/fe/src/main/java/org/apache/doris/analysis/InPredicate.java
@@ -223,7 +223,7 @@ protected void toThrift(TExprNode msg) {
}
@Override
- public String toSql() {
+ public String toSqlImpl() {
StringBuilder strBuilder = new StringBuilder();
String notStr = (isNotIn) ? "NOT " : "";
strBuilder.append(getChild(0).toSql() + " " + notStr + "IN (");
diff --git a/fe/src/main/java/org/apache/doris/analysis/InformationFunction.java b/fe/src/main/java/org/apache/doris/analysis/InformationFunction.java
index d4861a01..02c1e710 100644
--- a/fe/src/main/java/org/apache/doris/analysis/InformationFunction.java
+++ b/fe/src/main/java/org/apache/doris/analysis/InformationFunction.java
@@ -73,7 +73,7 @@ protected void toThrift(TExprNode msg) {
}
@Override
- public String toSql() {
+ public String toSqlImpl() {
return funcType + "()";
}
}
diff --git a/fe/src/main/java/org/apache/doris/analysis/IntLiteral.java b/fe/src/main/java/org/apache/doris/analysis/IntLiteral.java
index 5d13b80c..1f5b1df9 100644
--- a/fe/src/main/java/org/apache/doris/analysis/IntLiteral.java
+++ b/fe/src/main/java/org/apache/doris/analysis/IntLiteral.java
@@ -279,7 +279,7 @@ public double getDoubleValue() {
}
@Override
- public String toSql() {
+ public String toSqlImpl() {
return getStringValue();
}
diff --git a/fe/src/main/java/org/apache/doris/analysis/IsNullPredicate.java b/fe/src/main/java/org/apache/doris/analysis/IsNullPredicate.java
index 60c7037c..073a4e04 100644
--- a/fe/src/main/java/org/apache/doris/analysis/IsNullPredicate.java
+++ b/fe/src/main/java/org/apache/doris/analysis/IsNullPredicate.java
@@ -92,7 +92,7 @@ public boolean equals(Object obj) {
}
@Override
- public String toSql() {
+ public String toSqlImpl() {
return getChild(0).toSql() + (isNotNull ? " IS NOT NULL" : " IS NULL");
}
diff --git a/fe/src/main/java/org/apache/doris/analysis/LargeIntLiteral.java b/fe/src/main/java/org/apache/doris/analysis/LargeIntLiteral.java
index f505c7b5..d0162709 100644
--- a/fe/src/main/java/org/apache/doris/analysis/LargeIntLiteral.java
+++ b/fe/src/main/java/org/apache/doris/analysis/LargeIntLiteral.java
@@ -173,7 +173,7 @@ public double getDoubleValue() {
}
@Override
- public String toSql() {
+ public String toSqlImpl() {
return getStringValue();
}
diff --git a/fe/src/main/java/org/apache/doris/analysis/LikePredicate.java b/fe/src/main/java/org/apache/doris/analysis/LikePredicate.java
index d4889131..624657ef 100644
--- a/fe/src/main/java/org/apache/doris/analysis/LikePredicate.java
+++ b/fe/src/main/java/org/apache/doris/analysis/LikePredicate.java
@@ -102,7 +102,7 @@ public boolean equals(Object obj) {
}
@Override
- public String toSql() {
+ public String toSqlImpl() {
return getChild(0).toSql() + " " + op.toString() + " " + getChild(1).toSql();
}
diff --git a/fe/src/main/java/org/apache/doris/analysis/MaxLiteral.java b/fe/src/main/java/org/apache/doris/analysis/MaxLiteral.java
index 0282f024..8eba747e 100644
--- a/fe/src/main/java/org/apache/doris/analysis/MaxLiteral.java
+++ b/fe/src/main/java/org/apache/doris/analysis/MaxLiteral.java
@@ -53,7 +53,7 @@ protected void toThrift(TExprNode msg) {
}
@Override
- public String toSql() {
+ public String toSqlImpl() {
return "MAXVALUE";
}
diff --git a/fe/src/main/java/org/apache/doris/analysis/NullLiteral.java b/fe/src/main/java/org/apache/doris/analysis/NullLiteral.java
index 6179f3fa..ddf58180 100644
--- a/fe/src/main/java/org/apache/doris/analysis/NullLiteral.java
+++ b/fe/src/main/java/org/apache/doris/analysis/NullLiteral.java
@@ -89,7 +89,7 @@ public int compareLiteral(LiteralExpr expr) {
}
@Override
- public String toSql() {
+ public String toSqlImpl() {
return getStringValue();
}
diff --git a/fe/src/main/java/org/apache/doris/analysis/SlotRef.java b/fe/src/main/java/org/apache/doris/analysis/SlotRef.java
index db444b77..26b59081 100644
--- a/fe/src/main/java/org/apache/doris/analysis/SlotRef.java
+++ b/fe/src/main/java/org/apache/doris/analysis/SlotRef.java
@@ -148,7 +148,7 @@ public String debugString() {
}
@Override
- public String toSql() {
+ public String toSqlImpl() {
StringBuilder sb = new StringBuilder();
// if (desc != null) {
diff --git a/fe/src/main/java/org/apache/doris/analysis/StringLiteral.java b/fe/src/main/java/org/apache/doris/analysis/StringLiteral.java
index b8c94306..98029706 100644
--- a/fe/src/main/java/org/apache/doris/analysis/StringLiteral.java
+++ b/fe/src/main/java/org/apache/doris/analysis/StringLiteral.java
@@ -112,7 +112,7 @@ public boolean isMinValue() {
}
@Override
- public String toSql() {
+ public String toSqlImpl() {
return "'" + value + "'";
}
diff --git a/fe/src/main/java/org/apache/doris/analysis/Subquery.java b/fe/src/main/java/org/apache/doris/analysis/Subquery.java
index 16723a08..5dc0ccd7 100644
--- a/fe/src/main/java/org/apache/doris/analysis/Subquery.java
+++ b/fe/src/main/java/org/apache/doris/analysis/Subquery.java
@@ -51,7 +51,7 @@
public QueryStmt getStatement() { return stmt; }
@Override
- public String toSql() { return "(" + stmt.toSql() + ")"; }
+ public String toSqlImpl() { return "(" + stmt.toSql() + ")"; }
/**
* C'tor that initializes a Subquery from a QueryStmt.
diff --git a/fe/src/main/java/org/apache/doris/analysis/SysVariableDesc.java b/fe/src/main/java/org/apache/doris/analysis/SysVariableDesc.java
index 5e072f26..fc4d5343 100644
--- a/fe/src/main/java/org/apache/doris/analysis/SysVariableDesc.java
+++ b/fe/src/main/java/org/apache/doris/analysis/SysVariableDesc.java
@@ -115,7 +115,7 @@ protected void toThrift(TExprNode msg) {
}
@Override
- public String toSql() {
+ public String toSqlImpl() {
StringBuilder sb = new StringBuilder("@@");
if (setType == SetType.GLOBAL) {
sb.append("GLOBAL.");
diff --git a/fe/src/main/java/org/apache/doris/analysis/TableRef.java b/fe/src/main/java/org/apache/doris/analysis/TableRef.java
index 8cffa391..c997d1d8 100644
--- a/fe/src/main/java/org/apache/doris/analysis/TableRef.java
+++ b/fe/src/main/java/org/apache/doris/analysis/TableRef.java
@@ -546,7 +546,7 @@ public String toSql() {
if (usingColNames != null) {
output.append("USING (").append(Joiner.on(", ").join(usingColNames)).append(")");
} else if (onClause != null) {
- output.append("ON (").append(onClause.toSql()).append(")");
+ output.append("ON ").append(onClause.toSql());
}
return output.toString();
}
diff --git a/fe/src/main/java/org/apache/doris/analysis/TimestampArithmeticExpr.java b/fe/src/main/java/org/apache/doris/analysis/TimestampArithmeticExpr.java
index 32f31730..57dd403c 100644
--- a/fe/src/main/java/org/apache/doris/analysis/TimestampArithmeticExpr.java
+++ b/fe/src/main/java/org/apache/doris/analysis/TimestampArithmeticExpr.java
@@ -255,7 +255,7 @@ private TExprOpcode getOpCode() throws AnalysisException {
}
@Override
- public String toSql() {
+ public String toSqlImpl() {
StringBuilder strBuilder = new StringBuilder();
if (funcName != null) {
// Function-call like version.
diff --git a/fe/src/main/java/org/apache/doris/analysis/TupleIsNullPredicate.java b/fe/src/main/java/org/apache/doris/analysis/TupleIsNullPredicate.java
index df0da3c0..489b190d 100644
--- a/fe/src/main/java/org/apache/doris/analysis/TupleIsNullPredicate.java
+++ b/fe/src/main/java/org/apache/doris/analysis/TupleIsNullPredicate.java
@@ -145,7 +145,7 @@ private static boolean requiresNullWrapping(Expr expr, Analyzer analyzer) {
}
@Override
- public String toSql() {
+ public String toSqlImpl() {
return "";
}
}
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on 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
With regards,
Apache Git Services
---------------------------------------------------------------------
To unsubscribe, e-mail: dev-unsubscribe@doris.apache.org
For additional commands, e-mail: dev-help@doris.apache.org