diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/ArithmeticExpr.java b/fe/fe-core/src/main/java/org/apache/doris/analysis/ArithmeticExpr.java index 845b1d60a1276a..bd406ca4044c1a 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/analysis/ArithmeticExpr.java +++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/ArithmeticExpr.java @@ -102,11 +102,9 @@ protected ArithmeticExpr(ArithmeticExpr other) { @Override public String toString() { if (children.size() == 1) { - return op.toString() + " " + getChild(0).accept(ExprToSqlVisitor.INSTANCE, ToSqlParams.WITH_TABLE); + return op.toString() + " " + getChild(0); } else { - return "(" + getChild(0).accept(ExprToSqlVisitor.INSTANCE, ToSqlParams.WITH_TABLE) - + " " + op.toString() - + " " + getChild(1).accept(ExprToSqlVisitor.INSTANCE, ToSqlParams.WITH_TABLE) + ")"; + return "(" + getChild(0) + " " + op.toString() + " " + getChild(1) + ")"; } } diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/ExprToThriftVisitor.java b/fe/fe-core/src/main/java/org/apache/doris/analysis/ExprToThriftVisitor.java index 7161417cc3e8e8..6faacd25f705a7 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/analysis/ExprToThriftVisitor.java +++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/ExprToThriftVisitor.java @@ -26,6 +26,7 @@ import org.apache.doris.common.ErrorCode; import org.apache.doris.common.ErrorReport; import org.apache.doris.qe.ConnectContext; +import org.apache.doris.thrift.TAggregateExpr; import org.apache.doris.thrift.TBoolLiteral; import org.apache.doris.thrift.TCaseExpr; import org.apache.doris.thrift.TColumnRef; @@ -493,7 +494,7 @@ public Void visitFunctionCallExpr(FunctionCallExpr expr, TExprNode msg) { if (aggParams == null) { aggParams = expr.getFnParams(); } - msg.setAggExpr(aggParams.createTAggregateExpr(expr.isMergeAggFn())); + msg.setAggExpr(createTAggregateExprFromFunctionParams(aggParams, expr.isMergeAggFn())); } else { msg.node_type = TExprNodeType.FUNCTION_CALL; } @@ -504,6 +505,20 @@ public Void visitFunctionCallExpr(FunctionCallExpr expr, TExprNode msg) { return null; } + public TAggregateExpr createTAggregateExprFromFunctionParams(FunctionParams functionParams, boolean isMergeAggFn) { + List paramTypes = new ArrayList<>(); + if (functionParams.exprs() != null) { + for (Expr expr : functionParams.exprs()) { + TTypeDesc desc = expr.getType().toThrift(); + desc.setIsNullable(expr.isNullable()); + paramTypes.add(desc); + } + } + TAggregateExpr aggExpr = new TAggregateExpr(isMergeAggFn); + aggExpr.setParamTypes(paramTypes); + return aggExpr; + } + @Override public Void visitLambdaFunctionCallExpr(LambdaFunctionCallExpr expr, TExprNode msg) { msg.node_type = TExprNodeType.LAMBDA_FUNCTION_CALL_EXPR; diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/FunctionParams.java b/fe/fe-core/src/main/java/org/apache/doris/analysis/FunctionParams.java index 0b5b3b706f23f8..d47d041be1a839 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/analysis/FunctionParams.java +++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/FunctionParams.java @@ -20,13 +20,9 @@ package org.apache.doris.analysis; -import org.apache.doris.thrift.TAggregateExpr; -import org.apache.doris.thrift.TTypeDesc; - import com.google.common.base.Preconditions; import com.google.gson.annotations.SerializedName; -import java.util.ArrayList; import java.util.List; import java.util.Objects; @@ -74,20 +70,6 @@ public FunctionParams clone(List children) { return new FunctionParams(isDistinct(), children); } - public TAggregateExpr createTAggregateExpr(boolean isMergeAggFn) { - List paramTypes = new ArrayList<>(); - if (exprs != null) { - for (Expr expr : exprs) { - TTypeDesc desc = expr.getType().toThrift(); - desc.setIsNullable(expr.isNullable()); - paramTypes.add(desc); - } - } - TAggregateExpr aggExpr = new TAggregateExpr(isMergeAggFn); - aggExpr.setParamTypes(paramTypes); - return aggExpr; - } - public boolean isStar() { return isStar; } diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/OrderByElement.java b/fe/fe-core/src/main/java/org/apache/doris/analysis/OrderByElement.java index caa3d3cd9496e6..27b61d59be1094 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/analysis/OrderByElement.java +++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/OrderByElement.java @@ -75,13 +75,14 @@ public static List getOrderByExprs(List src) { return result; } - public String toSql() { + @Override + public String toString() { StringBuilder strBuilder = new StringBuilder(); - strBuilder.append(expr.accept(ExprToSqlVisitor.INSTANCE, ToSqlParams.WITH_TABLE)); + strBuilder.append(expr); strBuilder.append(isAsc ? " ASC" : " DESC"); // When ASC and NULLS LAST or DESC and NULLS FIRST, we do not print NULLS FIRST/LAST - // because it is the default behavior and we want to avoid printing NULLS FIRST/LAST + // because it is the default behavior. we want to avoid printing NULLS FIRST/LAST // whenever possible as it is incompatible with Hive (SQL compatibility with Hive is // important for views). if (nullsFirstParam != null) { @@ -97,11 +98,6 @@ public String toSql() { return strBuilder.toString(); } - @Override - public String toString() { - return toSql(); - } - @Override public boolean equals(Object obj) { if (obj == null) { diff --git a/fe/fe-core/src/main/java/org/apache/doris/planner/AnalyticEvalNode.java b/fe/fe-core/src/main/java/org/apache/doris/planner/AnalyticEvalNode.java index b85c82b58740e5..2ede7ac4e528e4 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/planner/AnalyticEvalNode.java +++ b/fe/fe-core/src/main/java/org/apache/doris/planner/AnalyticEvalNode.java @@ -126,7 +126,7 @@ public String getNodeExplainString(String prefix, TExplainLevel detailLevel) { strings.clear(); for (OrderByElement element : orderByElements) { - strings.add(element.toSql()); + strings.add(orderByElementToSql(element)); } output.append(Joiner.on(", ").join(strings)); @@ -146,6 +146,29 @@ public String getNodeExplainString(String prefix, TExplainLevel detailLevel) { return output.toString(); } + + private String orderByElementToSql(OrderByElement orderByElement) { + StringBuilder strBuilder = new StringBuilder(); + strBuilder.append(orderByElement.getExpr().accept(ExprToSqlVisitor.INSTANCE, ToSqlParams.WITH_TABLE)); + strBuilder.append(orderByElement.getIsAsc() ? " ASC" : " DESC"); + + // When ASC and NULLS LAST or DESC and NULLS FIRST, we do not print NULLS FIRST/LAST + // because it is the default behavior and we want to avoid printing NULLS FIRST/LAST + // whenever possible as it is incompatible with Hive (SQL compatibility with Hive is + // important for views). + if (orderByElement.getNullsFirstParam() != null) { + if (orderByElement.getIsAsc() && orderByElement.getNullsFirstParam()) { + // If ascending, nulls are last by default, so only add if nulls first. + strBuilder.append(" NULLS FIRST"); + } else if (!orderByElement.getIsAsc() && !orderByElement.getNullsFirstParam()) { + // If descending, nulls are first by default, so only add if nulls last. + strBuilder.append(" NULLS LAST"); + } + } + + return strBuilder.toString(); + } + /** * If `partitionExprs` is empty, the result must be output by single instance. *