[Bug] DECIMALV2 constant folding: 0 / x folds to NULL instead of 0 (divideDecimal checks wrong operand)

[LuciferYang]

Search before asking

• I had searched in the issues and found no similar issues.

Version

master (Nereids fold-constant divideDecimal).

What's Wrong?

NumericArithmetic.divideDecimal (the DECIMALV2 constant-folding divide function) guards against division by zero by checking the numerator (first) instead of the denominator (second):

public static Expression divideDecimal(DecimalLiteral first, DecimalLiteral second) {
    if (first.getValue().compareTo(BigDecimal.ZERO) == 0) {   // wrong operand
        return new NullLiteral(first.getDataType());
    }
    BigDecimal result = first.getValue().divide(second.getValue());
    return new DecimalLiteral(result);
}

When both operands are constant DECIMALV2 literals (so the fold-constant rule invokes divideDecimal):

• 0 / x folds to NULL instead of 0 — a silent wrong result.
• x / 0 skips the guard and calls BigDecimal.divide(ZERO), throwing ArithmeticException. This is caught by ExpressionEvaluator.invoke and the expression is left unfolded (BE then evaluates it, returning NULL), so it is not a crash, but the FE folding path is incorrect.

The sibling functions divideDouble and divideDecimalV3 both correctly check second.

What You Expected?

0 / x should fold to 0. Division by zero should return NULL (Doris/MySQL semantics), consistent with divideDouble and divideDecimalV3.

How to Reproduce?

Constant-fold a DECIMALV2 division whose numerator literal is 0, with both operands constant DECIMALV2 literals so the Nereids fold-constant rule dispatches to divideDecimal. The folded result is NULL where it should be 0.

Anything Else?

Fix proposed in PR #64675.

Are you willing to submit PR?

• Yes I am willing to submit a PR!

[zclllyybb]

Breakwater-GitHub-Analysis-Slot: slot_88614b96b47d
This content is generated by AI for reference only.

Initial analysis:

This looks like a valid FE/Nereids constant-folding bug on current master. I checked the upstream master code path at commit 90db340515e88e177bc4e6151ba0b9a84cf04d8c:

• NumericArithmetic.divideDecimal(DecimalLiteral first, DecimalLiteral second) currently returns NullLiteral when first is zero. That checks the numerator, unlike divideDouble and divideDecimalV3, which check the denominator.
• ExpressionEvaluator registers NumericArithmetic executable functions and selects overloads by Java parameter assignability, so divide with two DecimalLiteral children can enter this DECIMALV2 overload during folding.
• If the denominator is zero and the numerator is nonzero, BigDecimal.divide(0) raises through reflection; ExpressionEvaluator.invoke catches non-NotSupportedException invocation failures and returns the original expression. That explains why this path is unlikely to crash FE, but the folding implementation is still inconsistent and has a wrong zero-numerator result.

Expected impact:

• A silent wrong folded result for DECIMALV2 constant folding when both operands are constants and the numerator is zero, for example 0 / nonzero_decimal_v2 folding to NULL instead of decimal zero.
• I did not see the same bug in the adjacent double or DECIMALV3 folding paths, because those paths already guard on second.

PR #64675 appears directionally correct: changing the guard to second.getValue().compareTo(BigDecimal.ZERO) == 0 aligns DECIMALV2 with the sibling implementations and preserves NULL for divide by zero. The added direct unit tests for 0 / 5 and 5 / 0 cover the two edge cases in the executable function.

Missing information / suggested next steps:

1. No logs or profile are needed for the initial triage; the reported behavior is directly supported by the current master code.
2. The issue does not include a concrete SQL snippet or EXPLAIN/folded-plan output. The code evidence is enough to validate the bug, but adding a minimal SQL-level repro would make user-facing verification easier.
3. Review and run the PR's FE unit tests/CI. Consider adding a Nereids regression or planner-level assertion if maintainers want coverage of the full fold-constant dispatch path, not only the executable helper method.
4. If older release branches still carry the same DECIMALV2 implementation, check the exact branch code before deciding whether this needs backporting.

[LuciferYang]

The root-cause analysis matches. One addition on reachability, plus a concrete repro.

The DECIMALV2 overload divideDecimal(DecimalLiteral, DecimalLiteral) is only reached when decimal literals are parsed as DecimalV2, which is gated by the FE config enable_decimal_conversion (default true):

• LogicalPlanBuilder.visitDecimalLiteral returns DecimalV3Literal when enable_decimal_conversion = true (the default) → constant folding dispatches to divideDecimalV3, which already guards on the denominator and is correct.
• It returns DecimalLiteral (DecimalV2) only when enable_decimal_conversion = false → folding dispatches to the buggy divideDecimal.

So the wrong result is observable only with the non-default legacy config. Minimal repro:

ADMIN SET FRONTEND CONFIG ("enable_decimal_conversion" = "false");
SELECT 0.0 / 5.0;   -- FE folds this to NULL; expected 0
ADMIN SET FRONTEND CONFIG ("enable_decimal_conversion" = "true");

On the remaining points:

• x / 0: agreed — ExpressionEvaluator.invoke catches the ArithmeticException and returns the expression unfolded, so it is not an FE crash; the value still resolves to NULL (via BE), matching Doris/MySQL divide-by-zero semantics. The genuine defect is the 0 / x → NULL wrong result.
• Tests: PR [fix](nereids) Fix divide-by-zero guard in divideDecimal constant folding #64675 adds direct unit tests on the executable helper for 0 / 5 and 5 / 0. I can also add a FoldConstantRuleOnFE-level test exercising the full dispatch with DecimalV2 literals if that coverage is preferred.
• Backport: the same divideDecimal implementation exists on release branches; I'm happy to provide backports once a committer applies the appropriate branch-pick label.