From 6ef21f08893bb01581b5addc2e439f3de309df94 Mon Sep 17 00:00:00 2001 From: RH211-sys <233094961+RH211-sys@users.noreply.github.com> Date: Tue, 15 Sep 2026 22:55:20 +0800 Subject: [PATCH 1/3] [feature](function) Support gamma scalar function ### What problem does this PR solve? Related issue: [#48203](https://github.com/apache/doris/issues/48203) Problem Summary: Add the `gamma` scalar function, which generalizes the factorial to real numbers: `gamma(n)` is `(n - 1)!` for a positive integer `n`, and `gamma(0.5)` is `sqrt(pi)`. - BE: `gamma` is registered in `be/src/exprs/function/math.cpp` on top of `std::tgamma`. The poles are mapped to NULL instead of the value the C library produces: `gamma(0)` and every negative integer return NULL, and so does negative infinity, which the BE classifies as a negative integer pole. A NaN argument returns NaN, and positive infinity or an argument large enough to overflow a double (`gamma(172)` and above) returns Infinity. - FE: `Gamma` (unary, `ExplicitlyCastableSignature`, `AlwaysNullable`, `PropagateNullLiteral`), the Nereids visitor entry, and the builtin scalar function registration. - FE constant folding: `NumericArithmetic.gamma`, so that a folded `gamma()` produces the same value as the BE. commons-math3's `Gamma.gamma` saturates to Infinity at 165, where `std::tgamma` still returns a finite 3.29e293 (it stays finite up to 171), so positive inputs are evaluated as `exp(logGamma(x))`. Negative non-integers have no overflow problem and use `Gamma.gamma`. ### Release note Add the `gamma` scalar function. `gamma(n)` is `(n - 1)!` for a positive integer `n`; `gamma(0)` and the negative integers return NULL. ### Check List (For Author) - Test - [x] Regression test - [x] Unit Test - [x] Manual test (add detailed scripts or steps below) - [ ] No need to test or manual test. Explain why: - Behavior changed: - [x] No. - [ ] Yes. - Does this need documentation? - [ ] No. - [x] Yes. ### Check List (For Reviewer who merge this PR) - [ ] Confirm the release note - [ ] Confirm test cases - [ ] Confirm document - [ ] Add branch pick label Manual test: - `select gamma(5), gamma(0.5), gamma(-1.5), gamma(0), gamma(-1), gamma(-3);` returns `24.000000000000004, 1.772453850905516, 2.3632718012073544, NULL, NULL, NULL`, and the same values are returned with `set debug_skip_fold_constant=true`, i.e. constant folding and BE execution agree. - `gamma(cast('nan' as double))` is NaN, `gamma(cast('inf' as double))` is Infinity, `gamma(cast('-inf' as double))` is NULL, `gamma(165)` is 3.287218585534318E293, `gamma(171)` is 7.257415615308056E306 and `gamma(172)` is Infinity. - `explain select gamma(v) from t where gamma(v) > 10;` still plans an olap scan with the predicate and the projection applied. --- be/src/exprs/function/math.cpp | 11 ++ be/test/exprs/function/function_math_test.cpp | 33 +++++ .../doris/catalog/BuiltinScalarFunctions.java | 2 + .../executable/NumericArithmetic.java | 40 +++++ .../expressions/functions/scalar/Gamma.java | 78 ++++++++++ .../visitor/ScalarFunctionVisitor.java | 5 + .../math_functions/test_gamma.out | 138 ++++++++++++++++++ .../math_functions/test_gamma.groovy | 59 ++++++++ 8 files changed, 366 insertions(+) create mode 100644 fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/Gamma.java create mode 100644 regression-test/data/query_p0/sql_functions/math_functions/test_gamma.out create mode 100644 regression-test/suites/query_p0/sql_functions/math_functions/test_gamma.groovy diff --git a/be/src/exprs/function/math.cpp b/be/src/exprs/function/math.cpp index d25cca3a90106e..4342abdd708fc8 100644 --- a/be/src/exprs/function/math.cpp +++ b/be/src/exprs/function/math.cpp @@ -369,6 +369,16 @@ struct SqrtName { using FunctionSqrt = FunctionMathUnaryAlwayNullable>; +struct GammaName { + static constexpr auto name = "gamma"; + // https://dev.mysql.com/doc/refman/8.4/en/mathematical-functions.html#function_gamma + static constexpr bool is_invalid_input(Float64 x) { + return x == 0.0 || (x < 0.0 && x == std::floor(x)); + } +}; +using FunctionGamma = + FunctionMathUnaryAlwayNullable>; + struct CbrtName { static constexpr auto name = "cbrt"; }; @@ -962,6 +972,7 @@ void register_function_math(SimpleFunctionFactory& factory) { factory.register_function(); factory.register_alias("sqrt", "dsqrt"); factory.register_function(); + factory.register_function(); factory.register_function(); factory.register_function(); factory.register_function(); diff --git a/be/test/exprs/function/function_math_test.cpp b/be/test/exprs/function/function_math_test.cpp index cf1b3a442ea686..da592071108df7 100644 --- a/be/test/exprs/function/function_math_test.cpp +++ b/be/test/exprs/function/function_math_test.cpp @@ -184,6 +184,39 @@ TEST(MathFunctionTest, cbrt_test) { static_cast(check_function(func_name, input_types, data_set)); } +TEST(MathFunctionTest, gamma_test) { + std::string func_name = "gamma"; // gamma(x): x > 0, and a negative non-integer x + + InputTypeSet input_types = {PrimitiveType::TYPE_DOUBLE}; + // Gamma(n) is (n - 1)! for a positive integer n, but std::tgamma does not return every + // factorial exactly: 5 comes back as 24.000000000000004, so the expectation carries the ulp + // the implementation actually produces rather than the mathematical integer. Gamma(0.5) is + // sqrt(pi) and the half-integer rows are its multiples (sqrt(pi)/2 at 1.5, 3*sqrt(pi)/4 at + // 2.5); the values at -0.5, -1.5 and -2.5 come from the reflection formula. 0 and the negative + // integers are poles and must come back NULL, as MySQL returns, and a NULL input stays NULL. + DataSet data_set = {{{1.0}, 1.0}, + {{2.0}, 1.0}, + {{3.0}, 2.0}, + {{4.0}, 6.0}, + {{5.0}, 24.000000000000004}, + {{10.0}, 362880.00000000047}, + {{0.5}, 1.7724538509055161}, + {{1.5}, 0.88622692545275805}, + {{2.5}, 1.329340388179137}, + {{-0.5}, -3.5449077018110318}, + {{-1.5}, 2.3632718012073544}, + {{-2.5}, -0.94530872048294179}, + {{0.0}, Null()}, + {{-1.0}, Null()}, + {{-2.0}, Null()}, + {{-3.0}, Null()}, + {{-10.0}, Null()}, + {{Null()}, Null()}}; + + static_cast( + check_function_all_arg_comb(func_name, input_types, data_set)); +} + TEST(MathFunctionTest, cot_test) { std::string func_name = "cot"; diff --git a/fe/fe-core/src/main/java/org/apache/doris/catalog/BuiltinScalarFunctions.java b/fe/fe-core/src/main/java/org/apache/doris/catalog/BuiltinScalarFunctions.java index af2826d7b116f1..55895fd2e13742 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/catalog/BuiltinScalarFunctions.java +++ b/fe/fe-core/src/main/java/org/apache/doris/catalog/BuiltinScalarFunctions.java @@ -230,6 +230,7 @@ import org.apache.doris.nereids.trees.expressions.functions.scalar.FromSecond; import org.apache.doris.nereids.trees.expressions.functions.scalar.FromUnixtime; import org.apache.doris.nereids.trees.expressions.functions.scalar.G; +import org.apache.doris.nereids.trees.expressions.functions.scalar.Gamma; import org.apache.doris.nereids.trees.expressions.functions.scalar.Gcd; import org.apache.doris.nereids.trees.expressions.functions.scalar.GetFormat; import org.apache.doris.nereids.trees.expressions.functions.scalar.GetVariantType; @@ -823,6 +824,7 @@ public class BuiltinScalarFunctions implements FunctionHelper { scalar(FromIso8601Date.class, "from_iso8601_date"), scalar(FromUnixtime.class, "from_unixtime"), scalar(G.class, "g"), + scalar(Gamma.class, "gamma"), scalar(Gcd.class, "gcd"), scalar(GetFormat.class, "get_format"), scalar(GetVariantType.class, "variant_type"), diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/executable/NumericArithmetic.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/executable/NumericArithmetic.java index 507d521e3103d2..493e134de15f88 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/executable/NumericArithmetic.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/executable/NumericArithmetic.java @@ -40,6 +40,7 @@ import org.apache.doris.nereids.types.DoubleType; import org.apache.doris.nereids.types.FloatType; +import org.apache.commons.math3.special.Gamma; import org.apache.commons.math3.util.ArithmeticUtils; import org.apache.commons.math3.util.FastMath; @@ -755,6 +756,45 @@ public static Expression factorial(BigIntLiteral first) { return new BigIntLiteral(ArithmeticUtils.factorial((int) value)); } + /** + * gamma + * + *

The BE computes this with std::tgamma and maps the poles to NULL, so this + * evaluation reproduces that outcome rather than the raw library behaviour: + * commons-math3 returns NaN for everything that is not finite, while std::tgamma + * returns an infinity at zero, at a large enough argument and at positive infinity. + * + *

-Infinity is the one input where the two classifications differ in a way that + * matters: the BE sees it as a negative integer, hence a pole, and yields NULL. + * + *

Positive and negative inputs need different routes through commons-math3. + * Gamma.gamma saturates to an infinity well before std::tgamma does - it already + * overflows at 165, while std::tgamma still returns a finite 3.29e293 there - so + * positive inputs go through exp(logGamma(x)), which stays finite across the range and + * agrees with std::tgamma to the last place. logGamma is not defined for negative + * inputs, but that half has no overflow problem, so Gamma.gamma is used there. + */ + @ExecFunction(name = "gamma") + public static Expression gamma(DoubleLiteral first) { + double x = first.getValue(); + if (Double.isNaN(x)) { + return new DoubleLiteral(Double.NaN); + } + if (Double.isInfinite(x)) { + // +inf overflows to itself; -inf is treated as a negative integer, i.e. a pole. + return x > 0 ? new DoubleLiteral(Double.POSITIVE_INFINITY) + : new NullLiteral(DoubleType.INSTANCE); + } + // Gamma has a pole at zero and at every negative integer, where the BE yields NULL. + if (x == 0.0 || (x < 0.0 && x == Math.floor(x))) { + return new NullLiteral(DoubleType.INSTANCE); + } + if (x > 0.0) { + return new DoubleLiteral(Math.exp(Gamma.logGamma(x))); + } + return new DoubleLiteral(Gamma.gamma(x)); + } + /** * gcd */ diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/Gamma.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/Gamma.java new file mode 100644 index 00000000000000..63fa4e1af1dfac --- /dev/null +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/scalar/Gamma.java @@ -0,0 +1,78 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package org.apache.doris.nereids.trees.expressions.functions.scalar; + +import org.apache.doris.catalog.FunctionSignature; +import org.apache.doris.nereids.trees.expressions.Expression; +import org.apache.doris.nereids.trees.expressions.functions.AlwaysNullable; +import org.apache.doris.nereids.trees.expressions.functions.ExplicitlyCastableSignature; +import org.apache.doris.nereids.trees.expressions.functions.PropagateNullLiteral; +import org.apache.doris.nereids.trees.expressions.shape.UnaryExpression; +import org.apache.doris.nereids.trees.expressions.visitor.ExpressionVisitor; +import org.apache.doris.nereids.types.DoubleType; + +import com.google.common.base.Preconditions; +import com.google.common.collect.ImmutableList; + +import java.util.List; + +/** + * ScalarFunction 'gamma'. This class is generated by GenerateFunction. + * + *

Gamma(x) generalises the factorial to real numbers, so gamma(n) is (n - 1)! for a + * positive integer n. It has a pole at zero and at every negative integer, where this + * function returns NULL instead of the NaN that the mathematical definition would produce. + */ +public class Gamma extends ScalarFunction + implements UnaryExpression, ExplicitlyCastableSignature, AlwaysNullable, PropagateNullLiteral { + + public static final List SIGNATURES = ImmutableList.of( + FunctionSignature.ret(DoubleType.INSTANCE).args(DoubleType.INSTANCE) + ); + + /** + * constructor with 1 argument. + */ + public Gamma(Expression arg) { + super("gamma", arg); + } + + /** constructor for withChildren and reuse signature */ + private Gamma(ScalarFunctionParams functionParams) { + super(functionParams); + } + + /** + * withChildren. + */ + @Override + public Gamma withChildren(List children) { + Preconditions.checkArgument(children.size() == 1); + return new Gamma(getFunctionParams(children)); + } + + @Override + public List getSignatures() { + return SIGNATURES; + } + + @Override + public R accept(ExpressionVisitor visitor, C context) { + return visitor.visitGamma(this, context); + } +} diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/visitor/ScalarFunctionVisitor.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/visitor/ScalarFunctionVisitor.java index 26930ccd27a84a..69b5a5ccc153b6 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/visitor/ScalarFunctionVisitor.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/visitor/ScalarFunctionVisitor.java @@ -244,6 +244,7 @@ import org.apache.doris.nereids.trees.expressions.functions.scalar.FromIso8601Date; import org.apache.doris.nereids.trees.expressions.functions.scalar.FromUnixtime; import org.apache.doris.nereids.trees.expressions.functions.scalar.G; +import org.apache.doris.nereids.trees.expressions.functions.scalar.Gamma; import org.apache.doris.nereids.trees.expressions.functions.scalar.Gcd; import org.apache.doris.nereids.trees.expressions.functions.scalar.GetFormat; import org.apache.doris.nereids.trees.expressions.functions.scalar.GetVariantType; @@ -1562,6 +1563,10 @@ default R visitG(G g, C context) { return visitScalarFunction(g, context); } + default R visitGamma(Gamma gamma, C context) { + return visitScalarFunction(gamma, context); + } + default R visitGcd(Gcd gcd, C context) { return visitScalarFunction(gcd, context); } diff --git a/regression-test/data/query_p0/sql_functions/math_functions/test_gamma.out b/regression-test/data/query_p0/sql_functions/math_functions/test_gamma.out new file mode 100644 index 00000000000000..a1d8fbca0d2f0a --- /dev/null +++ b/regression-test/data/query_p0/sql_functions/math_functions/test_gamma.out @@ -0,0 +1,138 @@ +-- This file is automatically generated. You should know what you did if you want to edit this +-- !empty_nullable -- + +-- !empty_not_nullable -- + +-- !all_null -- +\N +\N +\N + +-- !nullable -- +\N +\N +\N +\N +\N +\N +\N +-0.9453087204829418 +-3.5449077018110318 +0.886226925452758 +1 +1 +1.772453850905516 +2 +2.3632718012073544 +24.000000000000004 +362880.00000000047 +6 +7.257415615308056e+306 +Infinity +Infinity +Infinity +NaN + +-- !not_nullable -- +\N +\N +\N +\N +\N +\N +-0.9453087204829418 +-3.5449077018110318 +0.886226925452758 +1 +1 +1.772453850905516 +2 +2.3632718012073544 +24.000000000000004 +362880.00000000047 +6 +7.257415615308056e+306 +9.875044200833234e+202 +Infinity +Infinity +Infinity +NaN + +-- !nullable_no_null -- +\N +\N +\N +\N +\N +\N +-0.9453087204829418 +-3.5449077018110318 +0.886226925452758 +1 +1 +1.772453850905516 +2 +2.3632718012073544 +24.000000000000004 +362880.00000000047 +6 +7.257415615308056e+306 +9.875044200833234e+202 +Infinity +Infinity +Infinity +NaN + +-- !const_nullable -- +\N +\N +\N +\N +\N +\N +\N +\N +\N +\N +\N +\N +\N +\N +\N +\N +\N +\N +\N +\N +\N +\N +\N + +-- !const_not_nullable -- +1.772453850905516 +1.772453850905516 +1.772453850905516 +1.772453850905516 +1.772453850905516 +1.772453850905516 +1.772453850905516 +1.772453850905516 +1.772453850905516 +1.772453850905516 +1.772453850905516 +1.772453850905516 +1.772453850905516 +1.772453850905516 +1.772453850905516 +1.772453850905516 +1.772453850905516 +1.772453850905516 +1.772453850905516 +1.772453850905516 +1.772453850905516 +1.772453850905516 +1.772453850905516 + +-- !const_nullable_no_null -- +1.772453850905516 + diff --git a/regression-test/suites/query_p0/sql_functions/math_functions/test_gamma.groovy b/regression-test/suites/query_p0/sql_functions/math_functions/test_gamma.groovy new file mode 100644 index 00000000000000..6af59e08b76683 --- /dev/null +++ b/regression-test/suites/query_p0/sql_functions/math_functions/test_gamma.groovy @@ -0,0 +1,59 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +suite("test_gamma") { + sql " drop table if exists test_gamma" + sql """ + create table test_gamma ( + k0 int, + a double not null, + b double null + ) + DISTRIBUTED BY HASH(k0) + PROPERTIES + ( + "replication_num" = "1" + ); + """ + + order_qt_empty_nullable "select gamma(b) from test_gamma" + order_qt_empty_not_nullable "select gamma(a) from test_gamma" + + sql "insert into test_gamma values (1, 1, null), (1, 1, null), (1, 1, null)" + order_qt_all_null "select gamma(b) from test_gamma" + + sql "truncate table test_gamma" + sql """ insert into test_gamma values + (1, 1, 1), (2, 2, 2), (3, 3, 3), (4, 4, 4), (5, 5, 5), (6, 10, 10), + (7, 0, 0), (8, -0, -0), (9, -1, -1), (10, -2, -2), (11, -3, -3), + (12, 0.5, 0.5), (13, -0.5, -0.5), (14, 1.5, 1.5), (15, -1.5, -1.5), (16, -2.5, -2.5), + (17, 171, 171), (18, 172, 172), (19, 1e308, 1e308), + (20, cast('nan' as double), cast('nan' as double)), + (21, cast('inf' as double), cast('inf' as double)), + (22, cast('-inf' as double), cast('-inf' as double)), + (23, 123, null); + """ + + order_qt_nullable "select gamma(b) from test_gamma" + order_qt_not_nullable "select gamma(a) from test_gamma" + order_qt_nullable_no_null "select gamma(nullable(a)) from test_gamma" + order_qt_const_nullable "select gamma(NULL) from test_gamma" + order_qt_const_not_nullable "select gamma(0.5) from test_gamma" + order_qt_const_nullable_no_null "select gamma(nullable(0.5))" + + testFoldConst """ select gamma(0), gamma(-1), gamma(-2), gamma(0.5), gamma(5), gamma(-2.5), gamma(171), gamma(172), gamma(cast('nan' as double)), gamma(cast('inf' as double)), gamma(cast('-inf' as double)) """ +} From 855f0c9343936e3e0253649bc118b5ed3792bcba Mon Sep 17 00:00:00 2001 From: RH211-sys <233094961+RH211-sys@users.noreply.github.com> Date: Mon, 21 Sep 2026 20:24:24 +0800 Subject: [PATCH 2/3] [fix](function) Drop the FE constant folding of gamma ### What problem does this PR solve? Related issue: [#48203](https://github.com/apache/doris/issues/48203) Problem Summary: The FE folded `gamma()` through commons-math3 while the BE computes it with `std::tgamma`. The two are independent implementations that cannot be kept aligned, so a folded constant could differ from the value the BE produces. Measured on the Doris build environment: - `gamma(-1000.5)` is NaN from the FE and -0.0 from the BE, a different classification; - `gamma(-150.5)` is 0.0 from the FE and -4.4784476581511713e-264 from the BE; - `gamma(10)` is 362879.9999999998 from the FE and 362880.00000000047 from the BE, about 11 ulp. `gamma` feeds neither partition nor bucket pruning, so the folding buys nothing. Remove it (`NumericArithmetic.gamma` and the commons-math3 import it needed) and let the BE stay the only implementation. There is then a single evaluation path: the default session, a session with `debug_skip_fold_constant=true`, a session with `enable_fold_constant_by_be=true` and a query that reads `gamma` of a table column all return identical values for the boundary inputs below (0, -0.0, negative integers, +/-infinity, NaN, 165/170/171/171.5/171.8/172/1e308, the smallest subnormal and the smallest normal, 1e-300, and large negative non-integers). Also in this commit: - The negative-zero row of the regression suite was written as the integer `-0`, which is stored as +0.0 and silently duplicated the row holding 0.0. It now uses `cast('-0.0' as double)` and is guarded by a `signbit` assertion, so a wrong test value fails instead of being recorded in the generated .out file. The suite also gains the boundary classes it was missing: inputs whose reciprocal overflows (1e-300, 5e-324, 2.2250738585072014e-308), the overflow onset (171.5 is finite, 171.8 is Infinity) and large negative non-integers that underflow to -0.0 (-1000.5) or to a subnormal (-171.5). - The near-zero rows hold what libm's tgamma returns, about 1e-14 relative away from the correctly rounded value, which is inside the 1e-8 relative tolerance the framework applies to DOUBLE cells. The two properties that tolerance cannot see are asserted directly: the sign of a result that underflows to zero, and a subnormal result that must not collapse to zero. - `testFoldConst` passes by construction now that nothing is folded, so it is kept as a guard for a folding that might come back, and its last two columns are BOOLEAN (`signbit(gamma(-1000.5))`, `gamma(-171.5) > 0`). checkCell compares BOOLEAN cells exactly, while its double path would not notice a folding that flipped the sign of an underflowed zero (0.0 against -0.0 divides by a zero magnitude) or let a subnormal collapse to zero (the decimal-place fallback accepts it). - `math.cpp` cited a MySQL `gamma` anchor, but MySQL has no `gamma` function. The comment now states the convention actually followed: a domain error such as a pole returns NULL, as `sqrt(-1)` and `ln(0)` do, and only overflow returns Infinity. - The BE unit test also pins -0.0 as a pole. - The new FE statements are covered by `GammaTest`, which exercises the signature, the nullability, the implicit cast of the argument and the visitor dispatch through `withChildren`. This is what `check_coverage_fe` reports as uncovered. ### Release note None ### Check List (For Author) - Test - [x] Regression test - [x] Unit Test - [x] Manual test (the boundary matrix described above) - [ ] No need to test or manual test. Explain why: - Behavior changed: - [x] Yes: `gamma()` is no longer constant folded in the FE, it is computed by the BE like every other invocation. The values returned to the client are unchanged. - Does this need documentation? - [x] Yes: https://github.com/apache/doris-website/pull/4140 --- be/src/exprs/function/math.cpp | 4 +- be/test/exprs/function/function_math_test.cpp | 6 +- .../executable/NumericArithmetic.java | 40 ----------- .../functions/scalar/GammaTest.java | 72 +++++++++++++++++++ .../math_functions/test_gamma.out | 35 +++++++++ .../math_functions/test_gamma.groovy | 41 ++++++++++- 6 files changed, 152 insertions(+), 46 deletions(-) create mode 100644 fe/fe-core/src/test/java/org/apache/doris/nereids/trees/expressions/functions/scalar/GammaTest.java diff --git a/be/src/exprs/function/math.cpp b/be/src/exprs/function/math.cpp index 4342abdd708fc8..13e71e623c6bec 100644 --- a/be/src/exprs/function/math.cpp +++ b/be/src/exprs/function/math.cpp @@ -371,7 +371,9 @@ using FunctionSqrt = struct GammaName { static constexpr auto name = "gamma"; - // https://dev.mysql.com/doc/refman/8.4/en/mathematical-functions.html#function_gamma + // gamma has a pole at zero (negative zero included) and at every negative integer. Those + // are domain errors, and like the other math functions here they are reported as NULL + // instead of the infinity or NaN the C library returns; only overflow becomes Infinity. static constexpr bool is_invalid_input(Float64 x) { return x == 0.0 || (x < 0.0 && x == std::floor(x)); } diff --git a/be/test/exprs/function/function_math_test.cpp b/be/test/exprs/function/function_math_test.cpp index da592071108df7..0f5366a9248171 100644 --- a/be/test/exprs/function/function_math_test.cpp +++ b/be/test/exprs/function/function_math_test.cpp @@ -192,8 +192,9 @@ TEST(MathFunctionTest, gamma_test) { // factorial exactly: 5 comes back as 24.000000000000004, so the expectation carries the ulp // the implementation actually produces rather than the mathematical integer. Gamma(0.5) is // sqrt(pi) and the half-integer rows are its multiples (sqrt(pi)/2 at 1.5, 3*sqrt(pi)/4 at - // 2.5); the values at -0.5, -1.5 and -2.5 come from the reflection formula. 0 and the negative - // integers are poles and must come back NULL, as MySQL returns, and a NULL input stays NULL. + // 2.5); the values at -0.5, -1.5 and -2.5 come from the reflection formula. 0 (negative + // zero included) and the negative integers are poles, so they are domain errors and come + // back as NULL, and a NULL input stays NULL. DataSet data_set = {{{1.0}, 1.0}, {{2.0}, 1.0}, {{3.0}, 2.0}, @@ -207,6 +208,7 @@ TEST(MathFunctionTest, gamma_test) { {{-1.5}, 2.3632718012073544}, {{-2.5}, -0.94530872048294179}, {{0.0}, Null()}, + {{-0.0}, Null()}, {{-1.0}, Null()}, {{-2.0}, Null()}, {{-3.0}, Null()}, diff --git a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/executable/NumericArithmetic.java b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/executable/NumericArithmetic.java index 493e134de15f88..507d521e3103d2 100644 --- a/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/executable/NumericArithmetic.java +++ b/fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/executable/NumericArithmetic.java @@ -40,7 +40,6 @@ import org.apache.doris.nereids.types.DoubleType; import org.apache.doris.nereids.types.FloatType; -import org.apache.commons.math3.special.Gamma; import org.apache.commons.math3.util.ArithmeticUtils; import org.apache.commons.math3.util.FastMath; @@ -756,45 +755,6 @@ public static Expression factorial(BigIntLiteral first) { return new BigIntLiteral(ArithmeticUtils.factorial((int) value)); } - /** - * gamma - * - *

The BE computes this with std::tgamma and maps the poles to NULL, so this - * evaluation reproduces that outcome rather than the raw library behaviour: - * commons-math3 returns NaN for everything that is not finite, while std::tgamma - * returns an infinity at zero, at a large enough argument and at positive infinity. - * - *

-Infinity is the one input where the two classifications differ in a way that - * matters: the BE sees it as a negative integer, hence a pole, and yields NULL. - * - *

Positive and negative inputs need different routes through commons-math3. - * Gamma.gamma saturates to an infinity well before std::tgamma does - it already - * overflows at 165, while std::tgamma still returns a finite 3.29e293 there - so - * positive inputs go through exp(logGamma(x)), which stays finite across the range and - * agrees with std::tgamma to the last place. logGamma is not defined for negative - * inputs, but that half has no overflow problem, so Gamma.gamma is used there. - */ - @ExecFunction(name = "gamma") - public static Expression gamma(DoubleLiteral first) { - double x = first.getValue(); - if (Double.isNaN(x)) { - return new DoubleLiteral(Double.NaN); - } - if (Double.isInfinite(x)) { - // +inf overflows to itself; -inf is treated as a negative integer, i.e. a pole. - return x > 0 ? new DoubleLiteral(Double.POSITIVE_INFINITY) - : new NullLiteral(DoubleType.INSTANCE); - } - // Gamma has a pole at zero and at every negative integer, where the BE yields NULL. - if (x == 0.0 || (x < 0.0 && x == Math.floor(x))) { - return new NullLiteral(DoubleType.INSTANCE); - } - if (x > 0.0) { - return new DoubleLiteral(Math.exp(Gamma.logGamma(x))); - } - return new DoubleLiteral(Gamma.gamma(x)); - } - /** * gcd */ diff --git a/fe/fe-core/src/test/java/org/apache/doris/nereids/trees/expressions/functions/scalar/GammaTest.java b/fe/fe-core/src/test/java/org/apache/doris/nereids/trees/expressions/functions/scalar/GammaTest.java new file mode 100644 index 00000000000000..c3c67c026f9f54 --- /dev/null +++ b/fe/fe-core/src/test/java/org/apache/doris/nereids/trees/expressions/functions/scalar/GammaTest.java @@ -0,0 +1,72 @@ +// Licensed to the Apache Software Foundation (ASF) under one +// or more contributor license agreements. See the NOTICE file +// distributed with this work for additional information +// regarding copyright ownership. The ASF licenses this file +// to you under the Apache License, Version 2.0 (the +// "License"); you may not use this file except in compliance +// with the License. You may obtain a copy of the License at +// +// http://www.apache.org/licenses/LICENSE-2.0 +// +// Unless required by applicable law or agreed to in writing, +// software distributed under the License is distributed on an +// "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY +// KIND, either express or implied. See the License for the +// specific language governing permissions and limitations +// under the License. + +package org.apache.doris.nereids.trees.expressions.functions.scalar; + +import org.apache.doris.catalog.FunctionSignature; +import org.apache.doris.nereids.rules.expression.ExpressionRewriteTestHelper; +import org.apache.doris.nereids.trees.expressions.Expression; +import org.apache.doris.nereids.trees.expressions.literal.DoubleLiteral; +import org.apache.doris.nereids.trees.expressions.visitor.DefaultExpressionRewriter; +import org.apache.doris.nereids.types.DoubleType; + +import com.google.common.collect.ImmutableList; +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +public class GammaTest extends ExpressionRewriteTestHelper { + + @Test + public void testSignatureAndNullability() { + Gamma gamma = new Gamma(new DoubleLiteral(2.5)); + + Assertions.assertEquals(DoubleType.INSTANCE, gamma.getDataType()); + Assertions.assertEquals(1, gamma.getSignatures().size()); + FunctionSignature signature = gamma.getSignatures().get(0); + Assertions.assertEquals(DoubleType.INSTANCE, signature.returnType); + Assertions.assertEquals(ImmutableList.of(DoubleType.INSTANCE), signature.argumentsTypes); + // gamma has poles at zero and at every negative integer, so the result stays nullable + // however non-nullable the argument is + Assertions.assertTrue(gamma.nullable()); + Assertions.assertEquals("gamma(2.5)", gamma.toSql()); + } + + @Test + public void testAnalyzedArgumentIsCastToDouble() { + Expression analyzed = typeCoercion(PARSER.parseExpression("gamma(5)")); + + Assertions.assertTrue(analyzed instanceof Gamma); + Assertions.assertEquals(DoubleType.INSTANCE, analyzed.getDataType()); + Assertions.assertEquals(DoubleType.INSTANCE, analyzed.child(0).getDataType()); + Assertions.assertTrue(analyzed.nullable()); + } + + @Test + public void testAcceptRebuildsThroughWithChildren() { + Gamma gamma = new Gamma(new DoubleLiteral(2.5)); + + Expression rewritten = gamma.accept(new DefaultExpressionRewriter() { + @Override + public Expression visitDoubleLiteral(DoubleLiteral doubleLiteral, Void context) { + return new DoubleLiteral(doubleLiteral.getValue() + 1.0); + } + }, null); + + Assertions.assertEquals(new Gamma(new DoubleLiteral(3.5)), rewritten); + Assertions.assertEquals(DoubleType.INSTANCE, rewritten.getDataType()); + } +} diff --git a/regression-test/data/query_p0/sql_functions/math_functions/test_gamma.out b/regression-test/data/query_p0/sql_functions/math_functions/test_gamma.out index a1d8fbca0d2f0a..9b22ef3987b430 100644 --- a/regression-test/data/query_p0/sql_functions/math_functions/test_gamma.out +++ b/regression-test/data/query_p0/sql_functions/math_functions/test_gamma.out @@ -16,18 +16,25 @@ \N \N \N +-0 -0.9453087204829418 -3.5449077018110318 0.886226925452758 1 1 1.772453850905516 +1.93162654317124e-310 2 2.3632718012073544 24.000000000000004 362880.00000000047 +4.4942328371556665e+307 6 7.257415615308056e+306 +9.483367566824735e+307 +9.999999999999763e+299 +Infinity +Infinity Infinity Infinity Infinity @@ -40,19 +47,26 @@ NaN \N \N \N +-0 -0.9453087204829418 -3.5449077018110318 0.886226925452758 1 1 1.772453850905516 +1.93162654317124e-310 2 2.3632718012073544 24.000000000000004 362880.00000000047 +4.4942328371556665e+307 6 7.257415615308056e+306 +9.483367566824735e+307 9.875044200833234e+202 +9.999999999999763e+299 +Infinity +Infinity Infinity Infinity Infinity @@ -65,19 +79,26 @@ NaN \N \N \N +-0 -0.9453087204829418 -3.5449077018110318 0.886226925452758 1 1 1.772453850905516 +1.93162654317124e-310 2 2.3632718012073544 24.000000000000004 362880.00000000047 +4.4942328371556665e+307 6 7.257415615308056e+306 +9.483367566824735e+307 9.875044200833234e+202 +9.999999999999763e+299 +Infinity +Infinity Infinity Infinity Infinity @@ -107,6 +128,13 @@ NaN \N \N \N +\N +\N +\N +\N +\N +\N +\N -- !const_not_nullable -- 1.772453850905516 @@ -132,6 +160,13 @@ NaN 1.772453850905516 1.772453850905516 1.772453850905516 +1.772453850905516 +1.772453850905516 +1.772453850905516 +1.772453850905516 +1.772453850905516 +1.772453850905516 +1.772453850905516 -- !const_nullable_no_null -- 1.772453850905516 diff --git a/regression-test/suites/query_p0/sql_functions/math_functions/test_gamma.groovy b/regression-test/suites/query_p0/sql_functions/math_functions/test_gamma.groovy index 6af59e08b76683..c6241930109d72 100644 --- a/regression-test/suites/query_p0/sql_functions/math_functions/test_gamma.groovy +++ b/regression-test/suites/query_p0/sql_functions/math_functions/test_gamma.groovy @@ -39,15 +39,40 @@ suite("test_gamma") { sql "truncate table test_gamma" sql """ insert into test_gamma values (1, 1, 1), (2, 2, 2), (3, 3, 3), (4, 4, 4), (5, 5, 5), (6, 10, 10), - (7, 0, 0), (8, -0, -0), (9, -1, -1), (10, -2, -2), (11, -3, -3), + (7, 0, 0), (8, cast('-0.0' as double), cast('-0.0' as double)), (9, -1, -1), (10, -2, -2), (11, -3, -3), (12, 0.5, 0.5), (13, -0.5, -0.5), (14, 1.5, 1.5), (15, -1.5, -1.5), (16, -2.5, -2.5), (17, 171, 171), (18, 172, 172), (19, 1e308, 1e308), (20, cast('nan' as double), cast('nan' as double)), (21, cast('inf' as double), cast('inf' as double)), (22, cast('-inf' as double), cast('-inf' as double)), - (23, 123, null); + (23, 123, null), + (24, 1e-300, 1e-300), (25, 5e-324, 5e-324), (26, 2.2250738585072014e-308, 2.2250738585072014e-308), + (27, 171.5, 171.5), (28, 171.8, 171.8), (29, -1000.5, -1000.5), (30, -171.5, -171.5); """ + // k0 = 8 must really hold a negative zero. Spelled as the plain integer "-0" it is stored + // as +0.0 and silently duplicates k0 = 7, so the negative-zero input would never be + // covered while the generated .out file still looks plausible. This assertion guards the + // test input rather than a query result, which is why it is not a qt_sql block. + def negativeZero = sql "select signbit(a), signbit(b) from test_gamma where k0 = 8" + assertTrue(negativeZero.size() == 1 && negativeZero[0][0] && negativeZero[0][1], + "k0 = 8 must store -0.0 in both a and b, but got ${negativeZero}") + + // The near-zero rows hold whatever libm's tgamma returns rather than the correctly rounded + // value: gamma(1e-300) is 9.999999999999763e299 instead of 1e300 and gamma(2^-1022) is + // 4.4942328371556665e307 instead of 2^1022, about 1e-14 relative away. That is amplified by + // |ln gamma(x)| ~ 700 and is still far inside the 1e-8 relative tolerance the framework + // applies to DOUBLE cells, so those rows do not pin one libm version. Two properties are not + // visible to that tolerance and are asserted here instead: the sign of a result that + // underflows to zero (0.0 and -0.0 compare equal) and a subnormal result that must not + // collapse to zero (which the framework's decimal-place fallback would accept). + def underflowSign = sql "select signbit(gamma(a)) from test_gamma where k0 = 29" + assertTrue(underflowSign.size() == 1 && underflowSign[0][0], + "gamma(-1000.5) must underflow to a signed zero, but got ${underflowSign}") + def subnormalResult = sql "select gamma(a) > 0, gamma(a) < 2.2250738585072014e-308 from test_gamma where k0 = 30" + assertTrue(subnormalResult.size() == 1 && subnormalResult[0][0] && subnormalResult[0][1], + "gamma(-171.5) must stay a positive subnormal, but got ${subnormalResult}") + order_qt_nullable "select gamma(b) from test_gamma" order_qt_not_nullable "select gamma(a) from test_gamma" order_qt_nullable_no_null "select gamma(nullable(a)) from test_gamma" @@ -55,5 +80,15 @@ suite("test_gamma") { order_qt_const_not_nullable "select gamma(0.5) from test_gamma" order_qt_const_nullable_no_null "select gamma(nullable(0.5))" - testFoldConst """ select gamma(0), gamma(-1), gamma(-2), gamma(0.5), gamma(5), gamma(-2.5), gamma(171), gamma(172), gamma(cast('nan' as double)), gamma(cast('inf' as double)), gamma(cast('-inf' as double)) """ + // Both settings evaluate gamma in the BE now that the FE folding is gone, so this passes by + // construction; it guards the folding that used to live here. A boundary classified differently + // (NULL, NaN or an infinity against a finite value, as gamma(-1000.5) would be) always fails + // here, but two numeric divergences slip through the tolerance checkCell applies to DOUBLE + // cells: it divides by the magnitude of the real cell, so -0.0 against 0.0 reports no error, + // and its decimal-place fallback accepts a subnormal result rounded to 0.0. The last two + // columns are therefore BOOLEAN, which checkCell compares exactly: signbit(gamma(-1000.5)) + // catches a flipped sign, and gamma(-171.5) > 0 a result that underflowed to zero. A last-place + // difference in the value itself is tolerated on purpose, the same reason the folding was + // dropped. + testFoldConst """ select gamma(0), gamma(-1), gamma(-2), gamma(0.5), gamma(5), gamma(-2.5), gamma(171), gamma(172), gamma(cast('nan' as double)), gamma(cast('inf' as double)), gamma(cast('-inf' as double)), gamma(171.5), gamma(171.8), gamma(1e-300), gamma(5e-324), gamma(2.2250738585072014e-308), gamma(-1000.5), gamma(-171.5), signbit(gamma(-1000.5)), gamma(-171.5) > 0 """ } From 1197c62327b83898337267e8be15898977072c08 Mon Sep 17 00:00:00 2001 From: RH211-sys <233094961+RH211-sys@users.noreply.github.com> Date: Tue, 22 Sep 2026 14:01:34 +0800 Subject: [PATCH 3/3] [test](function) Cover the gamma arity guard for the increment coverage gate ### What problem does this PR solve? Related issue: [#48203](https://github.com/apache/doris/issues/48203) Problem Summary: `check_coverage_fe` requires the increment line coverage to be 100% and failed with `Gamma.java 90.00% (9/10)`, even though every executable line of the class was reached. The `Preconditions.checkArgument(children.size() == 1)` in `withChildren` is the only branch in the class. The line itself executes, but its failing direction never does, and the coverage portal reports a partially covered line as uncovered - locally the same run reports `LINE_MISSED=0` while `BRANCH_MISSED=1` and `INSTRUCTION_MISSED=1`. Exercise the failing direction too, so the line is fully covered. Verified with `run-fe-ut.sh --run --coverage org.apache.doris.nereids.trees.expressions.functions.scalar.GammaTest`: Gamma now reports instruction 0/48 missed, branch 0/2 missed, line 0/10 missed, and the four tests of GammaTest pass. ### Release note None --- .../trees/expressions/functions/scalar/GammaTest.java | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/fe/fe-core/src/test/java/org/apache/doris/nereids/trees/expressions/functions/scalar/GammaTest.java b/fe/fe-core/src/test/java/org/apache/doris/nereids/trees/expressions/functions/scalar/GammaTest.java index c3c67c026f9f54..4f36cb8e5e3a20 100644 --- a/fe/fe-core/src/test/java/org/apache/doris/nereids/trees/expressions/functions/scalar/GammaTest.java +++ b/fe/fe-core/src/test/java/org/apache/doris/nereids/trees/expressions/functions/scalar/GammaTest.java @@ -69,4 +69,15 @@ public Expression visitDoubleLiteral(DoubleLiteral doubleLiteral, Void context) Assertions.assertEquals(new Gamma(new DoubleLiteral(3.5)), rewritten); Assertions.assertEquals(DoubleType.INSTANCE, rewritten.getDataType()); } + + @Test + public void testWithChildrenRejectsWrongArity() { + Gamma gamma = new Gamma(new DoubleLiteral(2.5)); + + // The arity guard is the only branch in this class, so the failing direction has to be + // exercised as well: a partially covered line counts as uncovered for the increment + // coverage gate. + Assertions.assertThrows(IllegalArgumentException.class, + () -> gamma.withChildren(ImmutableList.of(new DoubleLiteral(1.0), new DoubleLiteral(2.0)))); + } }