[fix](decimal) Compare DecimalV2 fractional values correctly#65266
[fix](decimal) Compare DecimalV2 fractional values correctly#65266mrhhsg wants to merge 1 commit into
Conversation
### What problem does this PR solve?
Issue Number: None
Problem Summary: DecimalV2 vectorized comparisons converted DecimalV2Value through integer casts before applying equality and range operators. The cast drops the fractional part, so values such as 5.5555 could compare equal to 5.0 and make DecimalV2 predicates return incorrect rows. Compare DecimalV2 by its full scaled value. Also keep DecimalV2 slot-vs-literal predicates uncast on the slot side during type coercion so simple delete predicates can still be pushed to storage.
### Release note
Fix incorrect DecimalV2 comparison results when fractional parts differ.
### Check List (For Author)
- Test:
- Unit Test: `./run-be-ut.sh --run --filter=VAccurateComparison.*`
- Unit Test: `./run-fe-ut.sh --run org.apache.doris.nereids.util.TypeCoercionUtilsTest`
- Regression test: `doris-local-regression --network 10.26.20.3/24 run -d datatype_p0/decimalv2 -s test_decimalv2_common`
- Build: `BUILD_TYPE=ASAN doris-local-regression --network 10.26.20.3/24 build`
- Format: `build-support/clang-format.sh be/src/vec/core/accurate_comparison.h be/src/vec/core/decimal_comparison.h be/test/vec/core/accurate_comparison_test.cpp`, `build-support/check-format.sh`, `git diff --check`
- Behavior changed: Yes. DecimalV2 comparisons now consider fractional digits correctly.
- Does this need documentation: No
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
|
/review |
There was a problem hiding this comment.
Automated review found one correctness issue.
Critical checkpoints:
- Prompt/context: review prompt and prepared context were read before diff inspection; no AGENTS.md files were listed/found, and
.claude/skills/code-review/SKILL.mdis missing in this checkout. - Scope: GitHub PR metadata and the fetched base/head diff match the five changed files.
- Findings: one inline comment on DecimalV2 slot-vs-literal coercion rounding higher-scale literals.
- Validation:
git diff --checkpassed for the fetched base/head range. Full FE/BE unit tests were not run becausethirdparty/installedandthirdparty/installed/bin/protocare missing in this runner. - Subagent conclusions:
optimizer-rewriteandtests-session-configboth reported the same candidate; it was merged asMAIN-001,OR-DUP-001was treated as a duplicate, and convergence round 1 ended with both agents replyingNO_NEW_VALUABLE_FINDINGS.
| d -> true, | ||
| o -> o.isLiteral() && o.getDataType().isDecimalV2Type(), | ||
| compareExpressions)) { | ||
| return target.getDataType(); |
There was a problem hiding this comment.
This downgrade is too broad for literals that are not exactly representable by the slot type. Returning target.getDataType() here makes both comparison predicates and IN predicates cast the literal down to the column type; DecimalLiteral then rounds to the target scale. For example, with DECIMAL(8,5) c, c = 5.555555 would be evaluated against CAST(5.555555 AS DECIMAL(8,5)) (5.55556) instead of the wider DecimalV2 common scale, so it can match or delete a row that should not satisfy the original predicate. Please only take this slot-preserving path when every literal fits the target range and scale without rounding; otherwise keep the wider common type.
What problem does this PR solve?
Issue Number: None
Related PR: None
Problem Summary: DecimalV2 vectorized comparisons converted
DecimalV2Valuethrough integer casts before applying equality and range operators. The cast drops the fractional part, so values such as5.5555could compare equal to5.0and make DecimalV2 predicates return incorrect rows.This PR compares DecimalV2 by its full scaled value. It also keeps DecimalV2 slot-vs-literal predicates uncast on the slot side during type coercion so simple delete predicates can still be pushed to storage.
Release note
Fix incorrect DecimalV2 comparison results when fractional parts differ.
Check List (For Author)
./run-be-ut.sh --run --filter=VAccurateComparison.*./run-fe-ut.sh --run org.apache.doris.nereids.util.TypeCoercionUtilsTestdoris-local-regression --network 10.26.20.3/24 run -d datatype_p0/decimalv2 -s test_decimalv2_commonBUILD_TYPE=ASAN doris-local-regression --network 10.26.20.3/24 buildbuild-support/clang-format.sh be/src/vec/core/accurate_comparison.h be/src/vec/core/decimal_comparison.h be/test/vec/core/accurate_comparison_test.cpp,build-support/check-format.sh,git diff --check