github-actions[bot] commented on code in PR #64622:
URL: https://github.com/apache/doris/pull/64622#discussion_r3432278752
##########
fe/fe-core/src/main/java/org/apache/doris/nereids/trees/expressions/functions/SearchSignatureForRound.java:
##########
@@ -18,26 +18,81 @@
package org.apache.doris.nereids.trees.expressions.functions;
import org.apache.doris.catalog.FunctionSignature;
+import org.apache.doris.nereids.CascadesContext;
+import org.apache.doris.nereids.rules.expression.ExpressionRewriteContext;
+import org.apache.doris.nereids.rules.expression.rules.FoldConstantRuleOnFE;
+import org.apache.doris.nereids.trees.expressions.Cast;
import org.apache.doris.nereids.trees.expressions.Expression;
+import org.apache.doris.nereids.trees.expressions.literal.IntegerLikeLiteral;
import org.apache.doris.nereids.types.DoubleType;
import org.apache.doris.nereids.types.IntegerType;
+import org.apache.doris.nereids.types.coercion.Int32OrLessType;
+import org.apache.doris.qe.ConnectContext;
+import java.util.ArrayList;
import java.util.List;
/**
- * if argument 0 is float or double, we should return double signature for
round like function.
+ * Signature search for round-like functions (round, round_bankers, ceil,
floor, truncate).
*/
public interface SearchSignatureForRound extends ExplicitlyCastableSignature {
+
+ int DOUBLE_DECIMAL_RESULT_MAX_SCALE = 15;
+
@Override
default FunctionSignature searchSignature(List<FunctionSignature>
signatures) {
List<Expression> arguments = getArguments();
if (arguments.get(0).getDataType().isFloatLikeType()) {
if (arguments.size() == 1) {
return
FunctionSignature.ret(DoubleType.INSTANCE).args(DoubleType.INSTANCE);
} else if (arguments.size() == 2) {
+ if (arguments.get(0).getDataType().isDoubleType()
+ && isOptedIntoDecimalReroute()
+ && isNonNegativeIntegerLiteralAtMost(arguments.get(1),
+ DOUBLE_DECIMAL_RESULT_MAX_SCALE)) {
+ return ExplicitlyCastableSignature.super.searchSignature(
+ withoutFloatLikeReturnTypes(signatures));
+ }
return
FunctionSignature.ret(DoubleType.INSTANCE).args(DoubleType.INSTANCE,
IntegerType.INSTANCE);
}
}
return ExplicitlyCastableSignature.super.searchSignature(signatures);
}
+
+ static boolean isOptedIntoDecimalReroute() {
+ ConnectContext ctx = ConnectContext.get();
+ return ctx != null &&
ctx.getSessionVariable().roundDoubleReturnsDecimalForConstScale;
+ }
+
+ /**
+ * True iff scale folds to an integer literal in the closed range [0,
maxValue].
+ */
+ static boolean isNonNegativeIntegerLiteralAtMost(Expression scale, int
maxValue) {
+ Expression folded = scale;
+ if (!folded.isLiteral() && !folded.isSlot()) {
+ ExpressionRewriteContext ctx = new
ExpressionRewriteContext(CascadesContext.initTempContext());
+ folded = FoldConstantRuleOnFE.evaluate(folded, ctx);
+ }
+ Expression unwrapped = folded;
+ if (unwrapped instanceof Cast && unwrapped.child(0).isLiteral()
+ && unwrapped.child(0).getDataType() instanceof
Int32OrLessType) {
+ unwrapped = unwrapped.child(0);
+ }
+ if (unwrapped instanceof IntegerLikeLiteral) {
+ int value = ((IntegerLikeLiteral) unwrapped).getIntValue();
Review Comment:
This range check is not exact for widened integer literals. A plan like:
```text
Project(round(d, 4294967298))
Scan(d: DOUBLE)
```
has a BIGINT scale literal, but `IntegerLikeLiteral.getIntValue()` delegates
to `Number.intValue()`, so `4294967298` becomes `2`. With
`round_double_returns_decimal_for_const_scale=true`,
`isNonNegativeIntegerLiteralAtMost` returns true, `searchSignature` drops the
DOUBLE signatures, and the expression is routed through the DECIMAL path even
though the actual scale is not in `[0, 15]`. The same can happen for LARGEINT
values or negative values whose low 32 bits look small and positive. This is
distinct from the already raised ordinary `scale > 15` issue because the cap
now works for `17`, but can still be bypassed by integer-literal overflow.
Please do an exact range check before converting to `int` (or restrict this
branch to `Int32OrLessType`, matching `ComputePrecisionForRound`) and add a
negative test with a BIGINT/LARGEINT scale outside the allowed range.
--
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]
