github-actions[bot] commented on code in PR #68029:
URL: https://github.com/apache/doris/pull/68029#discussion_r4058837042
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/executable/NumericArithmetic.java:
##########
@@ -755,6 +756,45 @@ public static Expression factorial(BigIntLiteral first) {
return new BigIntLiteral(ArithmeticUtils.factorial((int) value));
}
+ /**
+ * gamma
+ *
+ * <p>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.
+ *
+ * <p>-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.
+ *
+ * <p>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)
Review Comment:
`gamma` is folded here with Commons Math, but execution uses `std::tgamma`
in BE. These implementations do not agree for all finite doubles: Commons Math
3.6.1 gives `gamma(-150.5) = -0.0` while BE returns about
`-4.478447658150641e-264`, and `gamma(-1000.5) = NaN` while BE returns signed
zero; near the positive subnormal overflow boundary (`0x0.3ffffffffffffp-1022`)
FE produces a finite max-double value while BE returns `Infinity`. Even
ordinary values differ (`gamma(10)` folds to `362879.9999999998` versus BE
`362880.0`). A literal query can therefore change rows, comparisons, and
overflow/NaN classification when constant folding is toggled. Please use a
numerically equivalent implementation/shared boundary or disable folding for
inputs that cannot be proven equivalent, and add parity coverage for these
domains.
##########
be/test/exprs/function/function_math_test.cpp:
##########
@@ -184,6 +184,39 @@ TEST(MathFunctionTest, cbrt_test) {
static_cast<void>(check_function<DataTypeFloat64, true>(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},
Review Comment:
These exact expected values are from the FE `exp(logGamma)` route, not the
registered BE call. On this runner `std::tgamma(5)` is `24.0`, `tgamma(10)` is
`362880.0`, `tgamma(-1.5)` is `2.363271801207355`, `tgamma(-2.5)` is
`-0.9453087204829419`, and `tgamma(171)` is `7.257415615307999e306`, while this
test expects different doubles. `check_function` compares `ColumnFloat64`
values exactly, so the unit test (and the generated regression fixture) is
platform-dependent and can fail. Please regenerate expectations from the BE
implementation or use a documented tolerance/portable oracle.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
