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/12 03:15:41 UTC
[GitHub] imay closed pull request #299: Fix failed cases in regression test
imay closed pull request #299: Fix failed cases in regression test
URL: https://github.com/apache/incubator-doris/pull/299
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/be/src/exprs/cast_functions.cpp b/be/src/exprs/cast_functions.cpp
index 7898149c..065307a3 100644
--- a/be/src/exprs/cast_functions.cpp
+++ b/be/src/exprs/cast_functions.cpp
@@ -204,17 +204,16 @@ StringVal CastFunctions::cast_to_string_val(FunctionContext* ctx, const DateTime
return sv;
}
-#if 0
-
-StringVal CastFunctions::CastToStringVal(FunctionContext* ctx, const StringVal& val) {
+StringVal CastFunctions::cast_to_string_val(FunctionContext* ctx, const StringVal& val) {
if (val.is_null) return StringVal::null();
StringVal sv;
sv.ptr = val.ptr;
sv.len = val.len;
- AnyValUtil::TruncateIfNecessary(ctx->GetReturnType(), &sv);
+ AnyValUtil::TruncateIfNecessary(ctx->get_return_type(), &sv);
return sv;
}
+#if 0
StringVal CastFunctions::CastToChar(FunctionContext* ctx, const StringVal& val) {
if (val.is_null) return StringVal::null();
diff --git a/be/src/exprs/cast_functions.h b/be/src/exprs/cast_functions.h
index e5f68ca8..8a3b612b 100644
--- a/be/src/exprs/cast_functions.h
+++ b/be/src/exprs/cast_functions.h
@@ -115,8 +115,8 @@ class CastFunctions {
static StringVal cast_to_string_val(FunctionContext* context, const FloatVal& val);
static StringVal cast_to_string_val(FunctionContext* context, const DoubleVal& val);
static StringVal cast_to_string_val(FunctionContext* context, const DateTimeVal& val);
+ static StringVal cast_to_string_val(FunctionContext* context, const StringVal& val);
#if 0
- static StringVal CastToStringVal(FunctionContext* context, const StringVal& val);
static StringVal CastToChar(FunctionContext* context, const StringVal& val);
#endif
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 43c9e27f..bc18b9ea 100644
--- a/fe/src/main/java/org/apache/doris/analysis/ArithmeticExpr.java
+++ b/fe/src/main/java/org/apache/doris/analysis/ArithmeticExpr.java
@@ -261,7 +261,7 @@ public void analyzeImpl(Analyzer analyzer) throws AnalysisException {
type = castBinaryOp(commonType);
fn = getBuiltinFunction(analyzer, fnName, collectChildReturnTypes(),
- Function.CompareMode.IS_SUPERTYPE_OF);
+ Function.CompareMode.IS_IDENTICAL);
if (fn == null) {
Preconditions.checkState(false, String.format(
"No match for '%s' with operand types %s and %s", toSql(), t1, t2));
diff --git a/fe/src/main/java/org/apache/doris/analysis/OrderByElement.java b/fe/src/main/java/org/apache/doris/analysis/OrderByElement.java
index 8d041bcb..736682a1 100644
--- a/fe/src/main/java/org/apache/doris/analysis/OrderByElement.java
+++ b/fe/src/main/java/org/apache/doris/analysis/OrderByElement.java
@@ -23,12 +23,11 @@
import org.apache.doris.common.AnalysisException;
import com.google.common.collect.Lists;
-
/**
* Combination of expr and ASC/DESC, and nulls ordering.
*/
public class OrderByElement {
- private Expr expr;
+ private Expr expr;
private final boolean isAsc;
// Represents the NULLs ordering specified: true when "NULLS FIRST", false when
@@ -72,7 +71,8 @@ public OrderByElement clone() {
for (int i = 0; i < src.size(); ++i) {
OrderByElement element = src.get(i);
OrderByElement reverseElement =
- new OrderByElement(element.getExpr().clone(), !element.isAsc, !element.nullsFirstParam);
+ new OrderByElement(element.getExpr().clone(), !element.isAsc,
+ Boolean.valueOf(!nullsFirst(element.nullsFirstParam, element.isAsc)));
result.add(reverseElement);
}
diff --git a/fe/src/main/java/org/apache/doris/rewrite/FoldConstantsRule.java b/fe/src/main/java/org/apache/doris/rewrite/FoldConstantsRule.java
index 201e2204..3c779d6a 100644
--- a/fe/src/main/java/org/apache/doris/rewrite/FoldConstantsRule.java
+++ b/fe/src/main/java/org/apache/doris/rewrite/FoldConstantsRule.java
@@ -39,6 +39,12 @@
import java.util.Collection;
import java.util.List;
import java.util.Objects;
+import java.util.Set;
+
+import com.google.common.base.Preconditions;
+import com.google.common.collect.ImmutableMultimap;
+import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Sets;
/**
* This rule replaces a constant Expr with its equivalent LiteralExpr by evaluating the
@@ -58,14 +64,19 @@
public class FoldConstantsRule implements ExprRewriteRule {
public static ExprRewriteRule INSTANCE = new FoldConstantsRule();
- private Multimap<String, FEFunctionInvoker> functions = ArrayListMultimap.create();
+ private ImmutableMultimap<String, FEFunctionInvoker> functions;
+ // For most build-in functions, it will return NullLiteral when params contain NullLiteral.
+ // But a few functions need to handle NullLiteral differently, such as "if". It need to add
+ // an attribute to LiteralExpr to mark null and check the attribute to decide whether to
+ // replace the result with NullLiteral when function finished. It leaves to be realized.
+ // TODO chenhao16.
+ private ImmutableSet<String> nonNullResultWithNullParamFunctions;
@Override
public Expr apply(Expr expr, Analyzer analyzer) throws AnalysisException {
- if (functions.isEmpty()) {
+ if (functions == null) {
registerFunctions();
}
-
// Avoid calling Expr.isConstant() because that would lead to repeated traversals
// of the Expr tree. Assumes the bottom-up application of this rule. Constant
// children should have been folded at this point.
@@ -103,6 +114,22 @@ private Expr simplify(Expr constExpr) throws AnalysisException {
|| constExpr instanceof FunctionCallExpr
|| constExpr instanceof CastExpr) {
Function fn = constExpr.getFn();
+ // unused cast
+ if (fn == null) {
+ Preconditions.checkState((constExpr instanceof CastExpr)
+ && constExpr.getChildren().size() == 1);
+ return constExpr.getChild(0);
+ }
+
+ // null
+ if (!nonNullResultWithNullParamFunctions.contains(fn.getFunctionName().getFunction())) {
+ for (Expr e : constExpr.getChildren()) {
+ if (e instanceof NullLiteral) {
+ return new NullLiteral();
+ }
+ }
+ }
+
List<ScalarType> argTypes = new ArrayList<>();
for (Type type : fn.getArgs()) {
argTypes.add((ScalarType) type);
@@ -139,7 +166,6 @@ private FEFunctionInvoker getFunction(FEFunctionSignature signature) {
if (!Arrays.equals(argTypes1, argTypes2)) {
continue;
}
-
return invoker;
}
return null;
@@ -148,10 +174,11 @@ private FEFunctionInvoker getFunction(FEFunctionSignature signature) {
private synchronized void registerFunctions() {
// double checked locking pattern
// functions only need to init once
- if (!functions.isEmpty()) {
+ if (functions != null) {
return;
}
-
+ ImmutableMultimap.Builder<String, FEFunctionInvoker> mapBuilder =
+ new ImmutableMultimap.Builder<String, FEFunctionInvoker>();
Class clazz = FEFunctions.class;
for (Method method : clazz.getDeclaredMethods()) {
FEFunction annotation = method.getAnnotation(FEFunction.class);
@@ -162,12 +189,18 @@ private synchronized void registerFunctions() {
for (String type : annotation.argTypes()) {
argTypes.add(ScalarType.createType(type));
}
-
- FEFunctionSignature signature = new FEFunctionSignature(name,
+ FEFunctionSignature signature = new FEFunctionSignature(name,
argTypes.toArray(new ScalarType[argTypes.size()]), returnType);
- functions.put(name, new FEFunctionInvoker(method, signature));
+ mapBuilder.put(name, new FEFunctionInvoker(method, signature));
}
}
+ this.functions = mapBuilder.build();
+
+ // Functions that need to handle null.
+ ImmutableSet.Builder<String> setBuilder =
+ new ImmutableSet.Builder<String>();
+ setBuilder.add("if");
+ this.nonNullResultWithNullParamFunctions = setBuilder.build();
}
public static class FEFunctionInvoker {
@@ -190,7 +223,7 @@ public FEFunctionSignature getSignature() {
public LiteralExpr invoke(List<Expr> args) throws AnalysisException {
try {
return (LiteralExpr) method.invoke(null, args.toArray());
- } catch (InvocationTargetException | IllegalAccessException e) {
+ } catch (InvocationTargetException | IllegalAccessException | IllegalArgumentException e) {
throw new AnalysisException(e.getLocalizedMessage());
}
}
----------------------------------------------------------------
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