Skip to content

Commit

Permalink
[fix](Nereids) change numeric arithmatic function boundary behavior t…
Browse files Browse the repository at this point in the history 
…o match with be execution behavior (#47966)

### What problem does this PR solve?

Related PR: #40744， #47228

Problem Summary:
When numeric function input or output out of boundary when fe folding
constant, it would throw an exception before. Now
we change it to match with be execution result, which is NullLiteral
  • Loading branch information
@LiBinfeng-01
LiBinfeng-01 authored and Your Name committed Feb 19, 2025
1 parent 1f858d9 commit ca37c4c
Show file tree
Hide file tree
Showing 8 changed files with 129 additions and 120 deletions.
69 changes: 47 additions & 22 deletions ...va/org/apache/doris/nereids/trees/expressions/functions/executable/NumericArithmetic.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@

package org.apache.doris.nereids.trees.expressions.functions.executable;

import org.apache.doris.nereids.exceptions.NotSupportedException;
import org.apache.doris.nereids.trees.expressions.ExecFunction;
import org.apache.doris.nereids.trees.expressions.Expression;
import org.apache.doris.nereids.trees.expressions.literal.BigIntLiteral;
Expand All @@ -35,6 +34,7 @@
import org.apache.doris.nereids.trees.expressions.literal.TinyIntLiteral;
import org.apache.doris.nereids.trees.expressions.literal.VarcharLiteral;
import org.apache.doris.nereids.types.DecimalV3Type;
import org.apache.doris.nereids.types.DoubleType;

import java.math.BigDecimal;
import java.math.BigInteger;
Expand Down Expand Up @@ -671,7 +671,7 @@ public static Expression coalesce(Literal first, Literal... second) {
/**
* Method to check boundary with options for inclusive or exclusive boundaries
*/
public static void checkInputBoundary(Literal input, double lowerBound, double upperBound,
public static Boolean inputOutOfBound(Literal input, double lowerBound, double upperBound,
boolean isLowerInclusive, boolean isUpperInclusive) {
if (input instanceof DoubleLiteral) {
double inputValue = ((DoubleLiteral) input).getValue();
Expand All @@ -680,15 +680,16 @@ public static void checkInputBoundary(Literal input, double lowerBound, double u
boolean upperCheck = isUpperInclusive ? (inputValue <= upperBound) : (inputValue < upperBound);
// Return true if both checks are satisfied
if (!lowerCheck || !upperCheck) {
throw new NotSupportedException("input " + input.toSql() + " is out of boundary");
return true;
}
}
return false;
}

private static Expression checkOutputBoundary(Literal input) {
if (input instanceof DoubleLiteral) {
if (((DoubleLiteral) input).getValue().isNaN() || ((DoubleLiteral) input).getValue().isInfinite()) {
throw new NotSupportedException(input.toSql() + " result is invalid");
return new NullLiteral(DoubleType.INSTANCE);
}
}
return input;
Expand Down Expand Up @@ -817,7 +818,9 @@ public static Expression exp(DoubleLiteral first) {
*/
@ExecFunction(name = "ln")
public static Expression ln(DoubleLiteral first) {
checkInputBoundary(first, 0.0d, Double.MAX_VALUE, false, true);
if (inputOutOfBound(first, 0.0d, Double.MAX_VALUE, false, true)) {
return new NullLiteral(DoubleType.INSTANCE);
}
return checkOutputBoundary(new DoubleLiteral(Math.log(first.getValue())));
}

Expand All @@ -826,9 +829,9 @@ public static Expression ln(DoubleLiteral first) {
*/
@ExecFunction(name = "log")
public static Expression log(DoubleLiteral first, DoubleLiteral second) {
checkInputBoundary(first, 0.0d, Double.MAX_VALUE, false, true);
if (first.getValue().equals(1.0d)) {
throw new NotSupportedException("the first input of function log can not be 1.0");
if (inputOutOfBound(first, 0.0d, Double.MAX_VALUE, false, true)
|| first.getValue().equals(1.0d)) {
return new NullLiteral(DoubleType.INSTANCE);
}
return checkOutputBoundary(new DoubleLiteral(Math.log(second.getValue()) / Math.log(first.getValue())));
}
Expand All @@ -838,7 +841,9 @@ public static Expression log(DoubleLiteral first, DoubleLiteral second) {
*/
@ExecFunction(name = "log2")
public static Expression log2(DoubleLiteral first) {
checkInputBoundary(first, 0.0d, Double.MAX_VALUE, false, true);
if (inputOutOfBound(first, 0.0d, Double.MAX_VALUE, false, true)) {
return new NullLiteral(DoubleType.INSTANCE);
}
return checkOutputBoundary(new DoubleLiteral(Math.log(first.getValue()) / Math.log(2.0)));
}

Expand All @@ -847,7 +852,9 @@ public static Expression log2(DoubleLiteral first) {
*/
@ExecFunction(name = "log10")
public static Expression log10(DoubleLiteral first) {
checkInputBoundary(first, 0.0d, Double.MAX_VALUE, false, true);
if (inputOutOfBound(first, 0.0d, Double.MAX_VALUE, false, true)) {
return new NullLiteral(DoubleType.INSTANCE);
}
return checkOutputBoundary(new DoubleLiteral(Math.log10(first.getValue())));
}

Expand All @@ -856,7 +863,9 @@ public static Expression log10(DoubleLiteral first) {
*/
@ExecFunction(name = "sqrt")
public static Expression sqrt(DoubleLiteral first) {
checkInputBoundary(first, 0.0d, Double.MAX_VALUE, true, true);
if (inputOutOfBound(first, 0.0d, Double.MAX_VALUE, true, true)) {
return new NullLiteral(DoubleType.INSTANCE);
}
return checkOutputBoundary(new DoubleLiteral(Math.sqrt(first.getValue())));
}

Expand All @@ -865,9 +874,9 @@ public static Expression sqrt(DoubleLiteral first) {
*/
@ExecFunction(name = "power")
public static Expression power(DoubleLiteral first, DoubleLiteral second) {
checkInputBoundary(second, Double.NEGATIVE_INFINITY, Double.POSITIVE_INFINITY, false, false);
if (first.getValue() < 0 && second.getValue() % 1 != 0) {
throw new NotSupportedException("input pair of function power can not be negative number and non-integer");
if (inputOutOfBound(second, Double.NEGATIVE_INFINITY, Double.POSITIVE_INFINITY, false, false)
|| (first.getValue() < 0 && second.getValue() % 1 != 0)) {
return new NullLiteral(DoubleType.INSTANCE);
}
return checkOutputBoundary(new DoubleLiteral(Math.pow(first.getValue(), second.getValue())));
}
Expand All @@ -877,7 +886,9 @@ public static Expression power(DoubleLiteral first, DoubleLiteral second) {
*/
@ExecFunction(name = "sin")
public static Expression sin(DoubleLiteral first) {
checkInputBoundary(first, Double.NEGATIVE_INFINITY, Double.POSITIVE_INFINITY, false, false);
if (inputOutOfBound(first, Double.NEGATIVE_INFINITY, Double.POSITIVE_INFINITY, false, false)) {
return new NullLiteral(DoubleType.INSTANCE);
}
return checkOutputBoundary(new DoubleLiteral(Math.sin(first.getValue())));
}

Expand All @@ -886,7 +897,9 @@ public static Expression sin(DoubleLiteral first) {
*/
@ExecFunction(name = "cos")
public static Expression cos(DoubleLiteral first) {
checkInputBoundary(first, Double.NEGATIVE_INFINITY, Double.POSITIVE_INFINITY, false, false);
if (inputOutOfBound(first, Double.NEGATIVE_INFINITY, Double.POSITIVE_INFINITY, false, false)) {
return new NullLiteral(DoubleType.INSTANCE);
}
return checkOutputBoundary(new DoubleLiteral(Math.cos(first.getValue())));
}

Expand All @@ -895,7 +908,9 @@ public static Expression cos(DoubleLiteral first) {
*/
@ExecFunction(name = "tan")
public static Expression tan(DoubleLiteral first) {
checkInputBoundary(first, Double.NEGATIVE_INFINITY, Double.POSITIVE_INFINITY, false, false);
if (inputOutOfBound(first, Double.NEGATIVE_INFINITY, Double.POSITIVE_INFINITY, false, false)) {
return new NullLiteral(DoubleType.INSTANCE);
}
return checkOutputBoundary(new DoubleLiteral(Math.tan(first.getValue())));
}

Expand All @@ -904,7 +919,9 @@ public static Expression tan(DoubleLiteral first) {
*/
@ExecFunction(name = "asin")
public static Expression asin(DoubleLiteral first) {
checkInputBoundary(first, -1.0, 1.0, true, true);
if (inputOutOfBound(first, -1.0, 1.0, true, true)) {
return new NullLiteral(DoubleType.INSTANCE);
}
return checkOutputBoundary(new DoubleLiteral(Math.asin(first.getValue())));
}

Expand All @@ -913,7 +930,9 @@ public static Expression asin(DoubleLiteral first) {
*/
@ExecFunction(name = "acos")
public static Expression acos(DoubleLiteral first) {
checkInputBoundary(first, -1.0, 1.0, true, true);
if (inputOutOfBound(first, -1.0, 1.0, true, true)) {
return new NullLiteral(DoubleType.INSTANCE);
}
return checkOutputBoundary(new DoubleLiteral(Math.acos(first.getValue())));
}

Expand Down Expand Up @@ -1049,7 +1068,9 @@ public static Expression dexp(DoubleLiteral first) {
*/
@ExecFunction(name = "dlog1")
public static Expression dlog1(DoubleLiteral first) {
checkInputBoundary(first, 0.0d, Double.MAX_VALUE, false, true);
if (inputOutOfBound(first, 0.0d, Double.MAX_VALUE, false, true)) {
return new NullLiteral(DoubleType.INSTANCE);
}
return checkOutputBoundary(new DoubleLiteral(Math.log1p(first.getValue())));
}

Expand All @@ -1058,7 +1079,9 @@ public static Expression dlog1(DoubleLiteral first) {
*/
@ExecFunction(name = "dlog10")
public static Expression dlog10(DoubleLiteral first) {
checkInputBoundary(first, 0.0d, Double.MAX_VALUE, false, true);
if (inputOutOfBound(first, 0.0d, Double.MAX_VALUE, false, true)) {
return new NullLiteral(DoubleType.INSTANCE);
}
return checkOutputBoundary(new DoubleLiteral(Math.log10(first.getValue())));
}

Expand All @@ -1067,7 +1090,9 @@ public static Expression dlog10(DoubleLiteral first) {
*/
@ExecFunction(name = "dsqrt")
public static Expression dsqrt(DoubleLiteral first) {
checkInputBoundary(first, 0.0d, Double.MAX_VALUE, false, true);
if (inputOutOfBound(first, 0.0d, Double.MAX_VALUE, false, true)) {
return new NullLiteral(DoubleType.INSTANCE);
}
return checkOutputBoundary(new DoubleLiteral(Math.sqrt(first.getValue())));
}

Expand Down
87 changes: 37 additions & 50 deletions fe/fe-core/src/test/java/org/apache/doris/nereids/rules/expression/FoldConstantTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
import org.apache.doris.common.Config;
import org.apache.doris.nereids.analyzer.UnboundRelation;
import org.apache.doris.nereids.exceptions.AnalysisException;
import org.apache.doris.nereids.exceptions.NotSupportedException;
import org.apache.doris.nereids.parser.NereidsParser;
import org.apache.doris.nereids.rules.analysis.ExpressionAnalyzer;
import org.apache.doris.nereids.rules.expression.rules.FoldConstantRule;
Expand Down Expand Up @@ -379,93 +378,81 @@ void testleFoldNumeric() {
Exp exp = new Exp(new DoubleLiteral(0d));
rewritten = executor.rewrite(exp, context);
Assertions.assertEquals(new DoubleLiteral(1.0), rewritten);
Assertions.assertThrows(NotSupportedException.class, () -> {
Exp exExp = new Exp(new DoubleLiteral(1000d));
executor.rewrite(exExp, context);
}, "infinite result is invalid");
Expression exExp = new Exp(new DoubleLiteral(1000d));
rewritten = executor.rewrite(exExp, context);
Assertions.assertEquals(new NullLiteral(DoubleType.INSTANCE), rewritten);

Ln ln = new Ln(new DoubleLiteral(1d));
rewritten = executor.rewrite(ln, context);
Assertions.assertEquals(new DoubleLiteral(0.0), rewritten);
Assertions.assertThrows(NotSupportedException.class, () -> {
Ln exExp = new Ln(new DoubleLiteral(0.0d));
executor.rewrite(exExp, context);
}, "input 0.0 is out of boundary");
Assertions.assertThrows(NotSupportedException.class, () -> {
Ln exExp = new Ln(new DoubleLiteral(-1d));
executor.rewrite(exExp, context);
}, "input -1 is out of boundary");

Assertions.assertThrows(NotSupportedException.class, () -> {
Log exExp = new Log(new DoubleLiteral(1.0d), new DoubleLiteral(1.0d));
executor.rewrite(exExp, context);
}, "the first input of function log can not be 1.0");
exExp = new Ln(new DoubleLiteral(0.0d));
rewritten = executor.rewrite(exExp, context);
Assertions.assertEquals(new NullLiteral(DoubleType.INSTANCE), rewritten);

exExp = new Ln(new DoubleLiteral(-1d));
rewritten = executor.rewrite(exExp, context);
Assertions.assertEquals(new NullLiteral(DoubleType.INSTANCE), rewritten);
exExp = new Log(new DoubleLiteral(1.0d), new DoubleLiteral(1.0d));
rewritten = executor.rewrite(exExp, context);
Assertions.assertEquals(new NullLiteral(DoubleType.INSTANCE), rewritten);

Sqrt sqrt = new Sqrt(new DoubleLiteral(16d));
rewritten = executor.rewrite(sqrt, context);
Assertions.assertEquals(new DoubleLiteral(4d), rewritten);
sqrt = new Sqrt(new DoubleLiteral(0d));
rewritten = executor.rewrite(sqrt, context);
Assertions.assertEquals(new DoubleLiteral(0d), rewritten);
Assertions.assertThrows(NotSupportedException.class, () -> {
Sqrt exExp = new Sqrt(new DoubleLiteral(-1d));
executor.rewrite(exExp, context);
}, "input -1 is out of boundary");
exExp = new Sqrt(new DoubleLiteral(-1d));
rewritten = executor.rewrite(exExp, context);
Assertions.assertEquals(new NullLiteral(DoubleType.INSTANCE), rewritten);

Power power = new Power(new DoubleLiteral(2d), new DoubleLiteral(3));
rewritten = executor.rewrite(power, context);
Assertions.assertEquals(new DoubleLiteral(8d), rewritten);
Assertions.assertThrows(NotSupportedException.class, () -> {
Power exExp = new Power(new DoubleLiteral(2d), new DoubleLiteral(10000d));
executor.rewrite(exExp, context);
}, "infinite result is invalid");
Assertions.assertThrows(NotSupportedException.class, () -> {
Power exExp = new Power(new DoubleLiteral(-1d), new DoubleLiteral(1.1d));
executor.rewrite(exExp, context);
}, "input pair of function power can not be negative number and non-integer");
exExp = new Power(new DoubleLiteral(2d), new DoubleLiteral(10000d));
rewritten = executor.rewrite(exExp, context);
Assertions.assertEquals(new NullLiteral(DoubleType.INSTANCE), rewritten);
exExp = new Power(new DoubleLiteral(-1d), new DoubleLiteral(1.1d));
rewritten = executor.rewrite(exExp, context);
Assertions.assertEquals(new NullLiteral(DoubleType.INSTANCE), rewritten);

Sin sin = new Sin(new DoubleLiteral(Math.PI / 2));
rewritten = executor.rewrite(sin, context);
Assertions.assertEquals(new DoubleLiteral(1d), rewritten);
sin = new Sin(new DoubleLiteral(0d));
rewritten = executor.rewrite(sin, context);
Assertions.assertEquals(new DoubleLiteral(0d), rewritten);
Assertions.assertThrows(NotSupportedException.class, () -> {
Sin exExp = new Sin(new DoubleLiteral(Double.POSITIVE_INFINITY));
executor.rewrite(exExp, context);
}, "input infinity is out of boundary");
exExp = new Sin(new DoubleLiteral(Double.POSITIVE_INFINITY));
rewritten = executor.rewrite(exExp, context);
Assertions.assertEquals(new NullLiteral(DoubleType.INSTANCE), rewritten);

Cos cos = new Cos(new DoubleLiteral(0d));
rewritten = executor.rewrite(cos, context);
Assertions.assertEquals(new DoubleLiteral(1d), rewritten);
Assertions.assertThrows(NotSupportedException.class, () -> {
Cos exExp = new Cos(new DoubleLiteral(Double.POSITIVE_INFINITY));
executor.rewrite(exExp, context);
}, "input infinity is out of boundary");
exExp = new Cos(new DoubleLiteral(Double.POSITIVE_INFINITY));
rewritten = executor.rewrite(exExp, context);
Assertions.assertEquals(new NullLiteral(DoubleType.INSTANCE), rewritten);

Tan tan = new Tan(new DoubleLiteral(0d));
rewritten = executor.rewrite(tan, context);
Assertions.assertEquals(new DoubleLiteral(0d), rewritten);
Assertions.assertThrows(NotSupportedException.class, () -> {
Tan exExp = new Tan(new DoubleLiteral(Double.POSITIVE_INFINITY));
executor.rewrite(exExp, context);
}, "input infinity is out of boundary");
exExp = new Tan(new DoubleLiteral(Double.POSITIVE_INFINITY));
rewritten = executor.rewrite(exExp, context);
Assertions.assertEquals(new NullLiteral(DoubleType.INSTANCE), rewritten);

Asin asin = new Asin(new DoubleLiteral(1d));
rewritten = executor.rewrite(asin, context);
Assertions.assertEquals(new DoubleLiteral(Math.PI / 2), rewritten);
Assertions.assertThrows(NotSupportedException.class, () -> {
Asin exExp = new Asin(new DoubleLiteral(2d));
executor.rewrite(exExp, context);
}, "input 2.0 is out of boundary");
exExp = new Asin(new DoubleLiteral(2d));
rewritten = executor.rewrite(exExp, context);
Assertions.assertEquals(new NullLiteral(DoubleType.INSTANCE), rewritten);

Acos acos = new Acos(new DoubleLiteral(1d));
rewritten = executor.rewrite(acos, context);
Assertions.assertEquals(new DoubleLiteral(0), rewritten);
Assertions.assertThrows(NotSupportedException.class, () -> {
Acos exExp = new Acos(new DoubleLiteral(2d));
executor.rewrite(exExp, context);
}, "input 2.0 is out of boundary");
exExp = new Acos(new DoubleLiteral(2d));
rewritten = executor.rewrite(exExp, context);
Assertions.assertEquals(new NullLiteral(DoubleType.INSTANCE), rewritten);

Sign sign = new Sign(new DoubleLiteral(1d));
rewritten = executor.rewrite(sign, context);
Expand Down
4 changes: 4 additions & 0 deletions regression-test/data/datatype_p0/double/test_double_nan.out
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
-- This file is automatically generated. You should know what you did if you want to edit this
-- !select --
1 \N

-- !select --
\N

4 changes: 4 additions & 0 deletions regression-test/data/datatype_p0/float/test_float_nan.out
View file
Original file line number Diff line number Diff line change
@@ -1,3 +1,7 @@
-- This file is automatically generated. You should know what you did if you want to edit this
-- !select --
1 \N

-- !select --
\N

10 changes: 2 additions & 8 deletions regression-test/suites/datatype_p0/double/test_double_nan.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,16 +20,10 @@ suite("test_double_nan", "datatype_p0") {
sql "DROP TABLE IF EXISTS ${tableName}"
sql "CREATE TABLE if NOT EXISTS ${tableName} (k int, value double) DUPLICATE KEY(k) DISTRIBUTED BY HASH (k) BUCKETS 1 PROPERTIES ('replication_num' = '1');"

test {
sql """insert into ${tableName} select 1, sqrt(-1);"""
exception "errCode"
}
sql """insert into ${tableName} select 1, sqrt(-1);"""

qt_select "select * from ${tableName} order by 1;"
test {
sql "select sqrt(-1);"
exception "errCode"
}
qt_select "select sqrt(-1);"

sql "DROP TABLE IF EXISTS ${tableName}"
}
10 changes: 2 additions & 8 deletions regression-test/suites/datatype_p0/float/test_float_nan.groovy
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,16 +19,10 @@ suite("test_float_nan", "datatype_p0") {
def tableName = "tbl_test_float_nan"
sql "DROP TABLE IF EXISTS ${tableName}"
sql "CREATE TABLE if NOT EXISTS ${tableName} (k int, value float) DUPLICATE KEY(k) DISTRIBUTED BY HASH (k) BUCKETS 1 PROPERTIES ('replication_num' = '1');"
test {
sql """insert into ${tableName} select 1, sqrt(-1);"""
exception "errCode"
}
sql """insert into ${tableName} select 1, sqrt(-1);"""

qt_select "select * from ${tableName} order by 1;"
test {
sql "select sqrt(-1);"
exception "errCode"
}
qt_select "select sqrt(-1);"

sql "DROP TABLE IF EXISTS ${tableName}"
}
Loading

0 comments on commit ca37c4c

Please sign in to comment.