Skip to content

[fix](decimal) Compare DecimalV2 fractional values correctly#65266

Closed
mrhhsg wants to merge 1 commit into
apache:branch-4.0from
mrhhsg:fix/decimalv2-predicate-result
Closed

[fix](decimal) Compare DecimalV2 fractional values correctly#65266
mrhhsg wants to merge 1 commit into
apache:branch-4.0from
mrhhsg:fix/decimalv2-predicate-result

Conversation

@mrhhsg

@mrhhsg mrhhsg commented Jul 6, 2026

Copy link
Copy Markdown
Member

What problem does this PR solve?

Issue Number: None

Related PR: 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.

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)

  • 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

### 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
@mrhhsg
mrhhsg requested a review from morningman as a code owner July 6, 2026 10:30
@hello-stephen

Copy link
Copy Markdown
Contributor

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR.

Please clearly describe your PR:

  1. What problem was fixed (it's best to include specific error reporting information). How it was fixed.
  2. Which behaviors were modified. What was the previous behavior, what is it now, why was it modified, and what possible impacts might there be.
  3. What features were added. Why was this function added?
  4. Which code was refactored and why was this part of the code refactored?
  5. Which functions were optimized and what is the difference before and after the optimization?

@mrhhsg

mrhhsg commented Jul 6, 2026

Copy link
Copy Markdown
Member Author

/review

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.md is 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 --check passed for the fetched base/head range. Full FE/BE unit tests were not run because thirdparty/installed and thirdparty/installed/bin/protoc are missing in this runner.
  • Subagent conclusions: optimizer-rewrite and tests-session-config both reported the same candidate; it was merged as MAIN-001, OR-DUP-001 was treated as a duplicate, and convergence round 1 ended with both agents replying NO_NEW_VALUABLE_FINDINGS.

d -> true,
o -> o.isLiteral() && o.getDataType().isDecimalV2Type(),
compareExpressions)) {
return target.getDataType();

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

@mrhhsg
mrhhsg marked this pull request as draft July 6, 2026 14:52
@mrhhsg mrhhsg closed this Jul 7, 2026
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants