From bb3a0fd30e2bbe58f181b9b37bbee867b6e3245a Mon Sep 17 00:00:00 2001
From: starocean999 <40539150+starocean999@users.noreply.github.com>
Date: Fri, 24 May 2024 02:24:40 +0800
Subject: [PATCH] [fix](nereids)should use nereids expr's nullable info when
 call Expr's toThrift method (#35274)

---
 .../java/org/apache/doris/analysis/Expr.java  |   8 +-
 .../glue/translator/ExpressionTranslator.java | 134 +++++++++++++-----
 2 files changed, 104 insertions(+), 38 deletions(-)

diff --git a/fe/fe-core/src/main/java/org/apache/doris/analysis/Expr.java b/fe/fe-core/src/main/java/org/apache/doris/analysis/Expr.java
index c31bacbf87..9adf1bf13e 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/analysis/Expr.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/analysis/Expr.java
@@ -97,6 +97,8 @@ public abstract class Expr extends TreeNode<Expr> implements ParseNode, Cloneabl
 
     public static final float FUNCTION_CALL_COST = 10;
 
+    protected Optional<Boolean> nullableFromNereids = Optional.empty();
+
     // returns true if an Expr is a non-analytic aggregate.
     private static final com.google.common.base.Predicate<Expr> IS_AGGREGATE_PREDICATE =
             new com.google.common.base.Predicate<Expr>() {
@@ -998,7 +1000,7 @@ public abstract class Expr extends TreeNode<Expr> implements ParseNode, Cloneabl
             }
         }
         msg.output_scale = getOutputScale();
-        msg.setIsNullable(isNullable());
+        msg.setIsNullable(nullableFromNereids.isPresent() ? nullableFromNereids.get() : isNullable());
         toThrift(msg);
         container.addToNodes(msg);
         for (Expr child : children) {
@@ -2603,5 +2605,9 @@ public abstract class Expr extends TreeNode<Expr> implements ParseNode, Cloneabl
     public boolean isZeroLiteral() {
         return this instanceof LiteralExpr && ((LiteralExpr) this).isZero();
     }
+
+    public void setNullableFromNereids(boolean nullable) {
+        nullableFromNereids = Optional.of(nullable);
+    }
 }
 
diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/glue/translator/ExpressionTranslator.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/glue/translator/ExpressionTranslator.java
index 6c7a1bd82c..2482bb50e3 100644
--- a/fe/fe-core/src/main/java/org/apache/doris/nereids/glue/translator/ExpressionTranslator.java
+++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/glue/translator/ExpressionTranslator.java
@@ -140,47 +140,57 @@ public class ExpressionTranslator extends DefaultExpressionVisitor<Expr, PlanTra
 
     @Override
     public Expr visitEqualTo(EqualTo equalTo, PlanTranslatorContext context) {
-        return new BinaryPredicate(Operator.EQ,
+        BinaryPredicate eq = new BinaryPredicate(Operator.EQ,
                 equalTo.child(0).accept(this, context),
                 equalTo.child(1).accept(this, context),
                 equalTo.getDataType().toCatalogDataType(),
                 NullableMode.DEPEND_ON_ARGUMENT);
+        eq.setNullableFromNereids(equalTo.nullable());
+        return eq;
     }
 
     @Override
     public Expr visitGreaterThan(GreaterThan greaterThan, PlanTranslatorContext context) {
-        return new BinaryPredicate(Operator.GT,
+        BinaryPredicate gt = new BinaryPredicate(Operator.GT,
                 greaterThan.child(0).accept(this, context),
                 greaterThan.child(1).accept(this, context),
                 greaterThan.getDataType().toCatalogDataType(),
                 NullableMode.DEPEND_ON_ARGUMENT);
+        gt.setNullableFromNereids(greaterThan.nullable());
+        return gt;
     }
 
     @Override
     public Expr visitGreaterThanEqual(GreaterThanEqual greaterThanEqual, PlanTranslatorContext context) {
-        return new BinaryPredicate(Operator.GE,
+        BinaryPredicate ge = new BinaryPredicate(Operator.GE,
                 greaterThanEqual.child(0).accept(this, context),
                 greaterThanEqual.child(1).accept(this, context),
                 greaterThanEqual.getDataType().toCatalogDataType(),
                 NullableMode.DEPEND_ON_ARGUMENT);
+        ge.setNullableFromNereids(greaterThanEqual.nullable());
+        return ge;
     }
 
     @Override
     public Expr visitLessThan(LessThan lessThan, PlanTranslatorContext context) {
-        return new BinaryPredicate(Operator.LT,
+        BinaryPredicate lt = new BinaryPredicate(Operator.LT,
                 lessThan.child(0).accept(this, context),
                 lessThan.child(1).accept(this, context),
                 lessThan.getDataType().toCatalogDataType(),
                 NullableMode.DEPEND_ON_ARGUMENT);
+        lt.setNullableFromNereids(lessThan.nullable());
+        return lt;
     }
 
     @Override
     public Expr visitLessThanEqual(LessThanEqual lessThanEqual, PlanTranslatorContext context) {
-        return new BinaryPredicate(Operator.LE,
+        BinaryPredicate le = new BinaryPredicate(Operator.LE,
                 lessThanEqual.child(0).accept(this, context),
                 lessThanEqual.child(1).accept(this, context),
                 lessThanEqual.getDataType().toCatalogDataType(),
                 NullableMode.DEPEND_ON_ARGUMENT);
+        le.setNullableFromNereids(lessThanEqual.nullable());
+        return le;
     }
 
     private OlapTable getOlapTableFromSlotDesc(SlotDescriptor slotDesc) {
@@ -247,23 +257,23 @@ public class ExpressionTranslator extends DefaultExpressionVisitor<Expr, PlanTra
         }
 
         MatchPredicate.Operator op = match.op();
-        return new MatchPredicate(op,
-            match.left().accept(this, context),
-            match.right().accept(this, context),
-            match.getDataType().toCatalogDataType(),
-            NullableMode.DEPEND_ON_ARGUMENT,
-            invertedIndexParser,
-            invertedIndexParserMode,
-            invertedIndexCharFilter);
+        MatchPredicate matchPredicate = new MatchPredicate(op, match.left().accept(this, context),
+                match.right().accept(this, context), match.getDataType().toCatalogDataType(),
+                NullableMode.DEPEND_ON_ARGUMENT, invertedIndexParser, invertedIndexParserMode,
+                invertedIndexCharFilter);
+        matchPredicate.setNullableFromNereids(match.nullable());
+        return matchPredicate;
     }
 
     @Override
     public Expr visitNullSafeEqual(NullSafeEqual nullSafeEqual, PlanTranslatorContext context) {
-        return new BinaryPredicate(Operator.EQ_FOR_NULL,
+        BinaryPredicate eq = new BinaryPredicate(Operator.EQ_FOR_NULL,
                 nullSafeEqual.child(0).accept(this, context),
                 nullSafeEqual.child(1).accept(this, context),
                 nullSafeEqual.getDataType().toCatalogDataType(),
                 NullableMode.ALWAYS_NOT_NULLABLE);
+        eq.setNullableFromNereids(nullSafeEqual.nullable());
+        return eq;
     }
 
     @Override
@@ -274,23 +284,32 @@ public class ExpressionTranslator extends DefaultExpressionVisitor<Expr, PlanTra
                     .map(e -> translate(e, context))
                     .collect(Collectors.toList());
             boolean allConstant = inPredicate.getOptions().stream().allMatch(Expression::isConstant);
-            return new org.apache.doris.analysis.InPredicate(
+            org.apache.doris.analysis.InPredicate in = new org.apache.doris.analysis.InPredicate(
                     inPredicate.getCompareExpr().accept(this, context),
                     inList, true, allConstant);
+            in.setNullableFromNereids(inPredicate.nullable());
+            return in;
         } else if (not.child() instanceof EqualTo) {
             EqualTo equalTo = (EqualTo) not.child();
-            return new BinaryPredicate(Operator.NE,
+            BinaryPredicate ne = new BinaryPredicate(Operator.NE,
                     equalTo.child(0).accept(this, context),
                     equalTo.child(1).accept(this, context),
                     equalTo.getDataType().toCatalogDataType(),
                     NullableMode.DEPEND_ON_ARGUMENT);
+            ne.setNullableFromNereids(equalTo.nullable());
+            return ne;
         } else if (not.child() instanceof InSubquery || not.child() instanceof Exists) {
             return new BoolLiteral(true);
         } else if (not.child() instanceof IsNull) {
-            return new IsNullPredicate(((IsNull) not.child()).child().accept(this, context), true, true);
+            IsNullPredicate isNull = new IsNullPredicate(
+                    ((IsNull) not.child()).child().accept(this, context), true, true);
+            isNull.setNullableFromNereids(not.child().nullable());
+            return isNull;
         } else {
-            return new CompoundPredicate(CompoundPredicate.Operator.NOT,
+            CompoundPredicate cp = new CompoundPredicate(CompoundPredicate.Operator.NOT,
                     not.child(0).accept(this, context), null);
+            cp.setNullableFromNereids(not.nullable());
+            return cp;
         }
     }
 
@@ -329,18 +348,22 @@ public class ExpressionTranslator extends DefaultExpressionVisitor<Expr, PlanTra
 
     @Override
     public Expr visitAnd(And and, PlanTranslatorContext context) {
-        return new org.apache.doris.analysis.CompoundPredicate(
+        org.apache.doris.analysis.CompoundPredicate cp = new org.apache.doris.analysis.CompoundPredicate(
                 org.apache.doris.analysis.CompoundPredicate.Operator.AND,
                 and.child(0).accept(this, context),
                 and.child(1).accept(this, context));
+        cp.setNullableFromNereids(and.nullable());
+        return cp;
     }
 
     @Override
     public Expr visitOr(Or or, PlanTranslatorContext context) {
-        return new org.apache.doris.analysis.CompoundPredicate(
+        org.apache.doris.analysis.CompoundPredicate cp = new org.apache.doris.analysis.CompoundPredicate(
                 org.apache.doris.analysis.CompoundPredicate.Operator.OR,
                 or.child(0).accept(this, context),
                 or.child(1).accept(this, context));
+        cp.setNullableFromNereids(or.nullable());
+        return cp;
     }
 
     @Override
@@ -357,14 +380,18 @@ public class ExpressionTranslator extends DefaultExpressionVisitor<Expr, PlanTra
         if (defaultValue.isPresent()) {
             elseExpr = defaultValue.get().accept(this, context);
         }
-        return new CaseExpr(caseWhenClauses, elseExpr);
+        CaseExpr caseExpr = new CaseExpr(caseWhenClauses, elseExpr);
+        caseExpr.setNullableFromNereids(caseWhen.nullable());
+        return caseExpr;
     }
 
     @Override
     public Expr visitCast(Cast cast, PlanTranslatorContext context) {
         // left child of cast is expression, right child of cast is target type
-        return new CastExpr(cast.getDataType().toCatalogDataType(),
+        CastExpr castExpr = new CastExpr(cast.getDataType().toCatalogDataType(),
                 cast.child().accept(this, context), null);
+        castExpr.setNullableFromNereids(cast.nullable());
+        return castExpr;
     }
 
     @Override
@@ -373,9 +400,11 @@ public class ExpressionTranslator extends DefaultExpressionVisitor<Expr, PlanTra
                 .map(e -> e.accept(this, context))
                 .collect(Collectors.toList());
         boolean allConstant = inPredicate.getOptions().stream().allMatch(Expression::isConstant);
-        return new org.apache.doris.analysis.InPredicate(
+        org.apache.doris.analysis.InPredicate in = new org.apache.doris.analysis.InPredicate(
                 inPredicate.getCompareExpr().accept(this, context),
                 inList, false, allConstant);
+        in.setNullableFromNereids(inPredicate.nullable());
+        return in;
     }
 
     @Override
@@ -419,6 +448,7 @@ public class ExpressionTranslator extends DefaultExpressionVisitor<Expr, PlanTra
         FunctionCallExpr functionCallExpr =
                 new FunctionCallExpr(catalogFunction, windowFnParams, windowFnParams, isMergeFn, catalogArguments);
         functionCallExpr.setIsAnalyticFnCall(true);
+        functionCallExpr.setNullableFromNereids(function.nullable());
         return functionCallExpr;
 
     }
@@ -428,7 +458,9 @@ public class ExpressionTranslator extends DefaultExpressionVisitor<Expr, PlanTra
         Expr func = lambda.getLambdaFunction().accept(this, context);
         List<Expr> arguments = lambda.getLambdaArguments().stream().map(e -> e.accept(this, context))
                 .collect(Collectors.toList());
-        return new LambdaFunctionExpr(func, lambda.getLambdaArgumentNames(), arguments);
+        LambdaFunctionExpr functionExpr = new LambdaFunctionExpr(func, lambda.getLambdaArgumentNames(), arguments);
+        functionExpr.setNullableFromNereids(lambda.nullable());
+        return functionExpr;
     }
 
     @Override
@@ -471,7 +503,10 @@ public class ExpressionTranslator extends DefaultExpressionVisitor<Expr, PlanTra
         // create catalog FunctionCallExpr without analyze again
         Expr lambdaBody = visitLambda(lambda, context);
         arguments.set(0, lambdaBody);
-        return new LambdaFunctionCallExpr(catalogFunction, new FunctionParams(false, arguments));
+        LambdaFunctionCallExpr functionCallExpr =
+                new LambdaFunctionCallExpr(catalogFunction, new FunctionParams(false, arguments));
+        functionCallExpr.setNullableFromNereids(arrayMap.nullable());
+        return functionCallExpr;
     }
 
     @Override
@@ -494,11 +529,17 @@ public class ExpressionTranslator extends DefaultExpressionVisitor<Expr, PlanTra
                 function.getDataType().toCatalogDataType(), function.hasVarArguments(),
                 "", TFunctionBinaryType.BUILTIN, true, true, nullableMode);
 
+        FunctionCallExpr functionCallExpr;
         // create catalog FunctionCallExpr without analyze again
         if (function instanceof HighOrderFunction) {
-            return new LambdaFunctionCallExpr(catalogFunction, new FunctionParams(false, arguments));
+            functionCallExpr = new LambdaFunctionCallExpr(catalogFunction,
+                    new FunctionParams(false, arguments));
+        } else {
+            functionCallExpr =
+                    new FunctionCallExpr(catalogFunction, new FunctionParams(false, arguments));
         }
-        return new FunctionCallExpr(catalogFunction, new FunctionParams(false, arguments));
+        functionCallExpr.setNullableFromNereids(function.nullable());
+        return functionCallExpr;
     }
 
     @Override
@@ -538,7 +579,9 @@ public class ExpressionTranslator extends DefaultExpressionVisitor<Expr, PlanTra
                 "", TFunctionBinaryType.BUILTIN, true, true, nullableMode);
 
         // create catalog FunctionCallExpr without analyze again
-        return new FunctionCallExpr(catalogFunction, new FunctionParams(false, arguments));
+        FunctionCallExpr functionCallExpr = new FunctionCallExpr(catalogFunction, new FunctionParams(false, arguments));
+        functionCallExpr.setNullableFromNereids(function.nullable());
+        return functionCallExpr;
     }
 
     @Override
@@ -549,18 +592,21 @@ public class ExpressionTranslator extends DefaultExpressionVisitor<Expr, PlanTra
         } else if (binaryArithmetic instanceof AlwaysNotNullable) {
             nullableMode = NullableMode.ALWAYS_NOT_NULLABLE;
         }
-        return new ArithmeticExpr(binaryArithmetic.getLegacyOperator(),
+        ArithmeticExpr arithmeticExpr = new ArithmeticExpr(binaryArithmetic.getLegacyOperator(),
                 binaryArithmetic.child(0).accept(this, context),
                 binaryArithmetic.child(1).accept(this, context),
                 binaryArithmetic.getDataType().toCatalogDataType(), nullableMode);
+        arithmeticExpr.setNullableFromNereids(binaryArithmetic.nullable());
+        return arithmeticExpr;
     }
 
     @Override
     public Expr visitUnaryArithmetic(UnaryArithmetic unaryArithmetic, PlanTranslatorContext context) {
-        return new ArithmeticExpr(unaryArithmetic.getLegacyOperator(),
+        ArithmeticExpr arithmeticExpr = new ArithmeticExpr(unaryArithmetic.getLegacyOperator(),
                 unaryArithmetic.child().accept(this, context), null,
                 unaryArithmetic.getDataType().toCatalogDataType(), NullableMode.DEPEND_ON_ARGUMENT);
-
+        arithmeticExpr.setNullableFromNereids(unaryArithmetic.nullable());
+        return arithmeticExpr;
     }
 
     @Override
@@ -571,9 +617,13 @@ public class ExpressionTranslator extends DefaultExpressionVisitor<Expr, PlanTra
         if (arithmetic.children().stream().anyMatch(e -> e.getDataType().isDateV2LikeType())) {
             nullableMode = NullableMode.DEPEND_ON_ARGUMENT;
         }
-        return new TimestampArithmeticExpr(arithmetic.getFuncName(), arithmetic.getOp(),
+        TimestampArithmeticExpr timestampArithmeticExpr = new TimestampArithmeticExpr(
+                arithmetic.getFuncName(), arithmetic.getOp(),
                 arithmetic.left().accept(this, context), arithmetic.right().accept(this, context),
-                arithmetic.getTimeUnit().toString(), arithmetic.getDataType().toCatalogDataType(), nullableMode);
+                arithmetic.getTimeUnit().toString(), arithmetic.getDataType().toCatalogDataType(),
+                nullableMode);
+        timestampArithmeticExpr.setNullableFromNereids(arithmetic.nullable());
+        return timestampArithmeticExpr;
     }
 
     @Override
@@ -583,7 +633,9 @@ public class ExpressionTranslator extends DefaultExpressionVisitor<Expr, PlanTra
 
     @Override
     public Expr visitIsNull(IsNull isNull, PlanTranslatorContext context) {
-        return new IsNullPredicate(isNull.child().accept(this, context), false, true);
+        IsNullPredicate isNullPredicate = new IsNullPredicate(isNull.child().accept(this, context), false, true);
+        isNullPredicate.setNullableFromNereids(isNull.nullable());
+        return isNullPredicate;
     }
 
     @Override
@@ -639,7 +691,10 @@ public class ExpressionTranslator extends DefaultExpressionVisitor<Expr, PlanTra
                 TFunctionBinaryType.BUILTIN, true, true,
                 function.nullable() ? NullableMode.ALWAYS_NULLABLE : NullableMode.ALWAYS_NOT_NULLABLE);
 
-        return new FunctionCallExpr(catalogFunction, new FunctionParams(function.isDistinct(), arguments));
+        FunctionCallExpr functionCallExpr = new FunctionCallExpr(catalogFunction,
+                new FunctionParams(function.isDistinct(), arguments));
+        functionCallExpr.setNullableFromNereids(function.nullable());
+        return functionCallExpr;
     }
 
     @Override
@@ -647,7 +702,9 @@ public class ExpressionTranslator extends DefaultExpressionVisitor<Expr, PlanTra
         FunctionParams exprs = new FunctionParams(udf.children().stream()
                 .map(expression -> expression.accept(this, context))
                 .collect(Collectors.toList()));
-        return new FunctionCallExpr(udf.getCatalogFunction(), exprs);
+        FunctionCallExpr functionCallExpr = new FunctionCallExpr(udf.getCatalogFunction(), exprs);
+        functionCallExpr.setNullableFromNereids(udf.nullable());
+        return functionCallExpr;
     }
 
     @Override
@@ -655,7 +712,9 @@ public class ExpressionTranslator extends DefaultExpressionVisitor<Expr, PlanTra
         FunctionParams exprs = new FunctionParams(udaf.isDistinct(), udaf.children().stream()
                 .map(expression -> expression.accept(this, context))
                 .collect(Collectors.toList()));
-        return new FunctionCallExpr(udaf.getCatalogFunction(), exprs);
+        FunctionCallExpr functionCallExpr = new FunctionCallExpr(udaf.getCatalogFunction(), exprs);
+        functionCallExpr.setNullableFromNereids(udaf.nullable());
+        return functionCallExpr;
     }
 
     // TODO: Supports for `distinct`
@@ -698,6 +757,7 @@ public class ExpressionTranslator extends DefaultExpressionVisitor<Expr, PlanTra
         FunctionCallExpr functionCallExpr = new FunctionCallExpr(
                 catalogFunction, fnParams, aggFnParams, isMergeFn, currentPhaseCatalogArguments);
         functionCallExpr.setOrderByElements(orderByElements);
+        functionCallExpr.setNullableFromNereids(function.nullable());
         return functionCallExpr;
     }