Skip to content

fix: coerce operand types in Interval mul/div/intersect/union/contains#22027

Open
adriangb wants to merge 1 commit intoapache:mainfrom
adriangb:fix-interval-decimal-mismatch
Open

fix: coerce operand types in Interval mul/div/intersect/union/contains#22027
adriangb wants to merge 1 commit intoapache:mainfrom
adriangb:fix-interval-decimal-mismatch

Conversation

@adriangb
Copy link
Copy Markdown
Contributor

@adriangb adriangb commented May 5, 2026

Which issue does this PR close?

  • Closes #.

Rationale for this change

Interval::mul, Interval::div, Interval::intersect, Interval::union, and Interval::contains all asserted that both operands had identical data types. This causes internal errors during interval propagation for ordinary SQL queries that mix Decimal128 precisions/scales — for example dividing numeric (which DataFusion maps to Decimal128(38, 10)) by a BIGINT column or count(*) (coerced to Decimal128(20, 0)):

Internal error: Assertion failed: dt.clone() == rhs_type.clone()
  (left: Decimal128(38, 10), right: Decimal128(20, 0)):
  Intervals must have the same data type for division

Interval::add and Interval::sub already used BinaryTypeCoercer to find a common arithmetic type; this PR brings mul and div in line. Once mul/div coerce, the result of an arithmetic op fed into intersect (e.g. by the CP solver in cp_solver.rs) may have a different type than the child interval, so intersect/union/contains are also relaxed to coerce via comparison_coercion (matching the existing pattern in Interval::contains_value).

Reproducer (datafusion-cli, before this PR)

CREATE TABLE t(c1 BIGINT) AS VALUES (1::bigint), (2::bigint), (10::bigint);

SELECT
  c1,
  CASE WHEN c1 = 0 THEN 100.0
       ELSE ROUND((1.0 - (c1::numeric / c1)) * 100, 2)
  END AS rate
FROM t
WHERE (1.0 - (c1::numeric / c1)) * 100 < 95.0;

fails with the internal-error message above. After this PR the query returns rows.

What changes are included in this PR?

  • Replace the type-equality asserts in Interval::mul and Interval::div with a new coerce_operands helper that uses BinaryTypeCoercer::get_result_type and casts both intervals to the common type when they differ.
  • Replace the type-equality asserts in Interval::intersect, Interval::union, and Interval::contains with a new coerce_for_comparison helper that uses comparison_coercion and casts both intervals to the common type when they differ.
  • Update the doc comments on the affected methods to document the new coercion behavior.

The same-type fast path is preserved (no allocation/cast when both intervals already share a type), so this should be a no-op for existing call sites.

Are these changes tested?

Yes:

  • New unit test test_mul_div_mismatched_operand_types in interval_arithmetic.rs exercising Decimal128(38, 10) / Decimal128(20, 0) and Decimal128 × Int64 for both mul and div.
  • New regression query in decimal.slt covering the numeric / bigint shape from the reproducer above through the optimizer's interval-propagation path. Without this fix the SLT query fails with an internal error; with it, it returns rows.
  • Existing datafusion-expr-common, datafusion-physical-expr, datafusion-physical-plan, and full sqllogictests suites all pass.

Are there any user-facing changes?

Queries that previously hit Internal error: Intervals must have the same data type ... (or the equivalent for intersect/union/contains) during interval/stats propagation will now succeed. No public API changes — these methods kept their signatures; the documented preconditions are relaxed.

🤖 Generated with Claude Code

…`/`contains`

Previously these methods asserted that both intervals shared an identical
data type, causing internal errors during interval propagation for queries
like `numeric / count(*)` where the operands end up as different
`Decimal128` precisions/scales (e.g. `Decimal128(38, 10) / Decimal128(20, 0)`).

`Interval::add` and `Interval::sub` already used `BinaryTypeCoercer` to find
a common arithmetic type. This change brings `mul`/`div` in line, and
similarly relaxes `intersect`/`union`/`contains` to coerce via
`comparison_coercion`, since CP-solver propagation feeds the result of an
arithmetic op into `intersect` with a child interval whose type may differ.

Adds a unit test in `interval_arithmetic.rs` for mismatched `Decimal128`
mul/div, and an end-to-end `decimal.slt` regression covering the
`numeric / bigint` shape from the failing customer query.

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@adriangb adriangb force-pushed the fix-interval-decimal-mismatch branch from 86c7d43 to 097259d Compare May 5, 2026 19:33
@adriangb
Copy link
Copy Markdown
Contributor Author

adriangb commented May 5, 2026

@pepijnve @neilconway any interest in reviewing this fix?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

logical-expr Logical plan and expressions sqllogictest SQL Logic Tests (.slt)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant