Skip to content

feat: extend interval analysis support for temporal types - #21520

Merged
berkaysynnada merged 6 commits into
apache:mainfrom
buraksenn:extend-interval-analysis-temporal
Apr 17, 2026
Merged

berkaysynnada merged 6 commits into
apache:mainfrom
buraksenn:extend-interval-analysis-temporal

Conversation

@buraksenn

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

related with #21109 and its PR #21473

Rationale for this change

Temporal types can actually support internal arithmetics and with this change internal arithmetics can now narrow down column boundaries and selectivity instead of falling back to default selectivity of 1.0

What changes are included in this PR?

Extend internal_arithmetic types with date and duration types

Are these changes tested?

Yes adjusted tests

Are there any user-facing changes?

no

@github-actions github-actions Bot added logical-expr Logical plan and expressions physical-expr Changes to the physical-expr crates common Related to common crate physical-plan Changes to the physical-plan crate labels Apr 9, 2026
@github-actions github-actions Bot added the core Core DataFusion crate label Apr 9, 2026
Comment thread datafusion/common/src/scalar/mod.rs Resolved

@berkaysynnada berkaysynnada 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.

LGTM, just two minor issues

@berkaysynnada
berkaysynnada added this pull request to the merge queue Apr 17, 2026
@github-merge-queue
github-merge-queue Bot removed this pull request from the merge queue due to a conflict with the base branch Apr 17, 2026
@berkaysynnada
berkaysynnada added this pull request to the merge queue Apr 17, 2026
Merged via the queue into apache:main with commit 6cd7e83 Apr 17, 2026
35 checks passed
@berkaysynnada
berkaysynnada deleted the extend-interval-analysis-temporal branch April 17, 2026 12:28
Rich-T-kid pushed a commit to Rich-T-kid/datafusion that referenced this pull request Apr 21, 2026
## Which issue does this PR close?
- Closes apache#21111

related with apache#21109 and its PR
apache#21473

## Rationale for this change
Temporal types can actually support internal arithmetics and with this
change internal arithmetics can now narrow down column boundaries and
selectivity instead of falling back to default selectivity of 1.0

## What changes are included in this PR?
Extend internal_arithmetic types with date and duration types

## Are these changes tested?
Yes adjusted tests

## Are there any user-facing changes?
no
alamb pushed a commit to rxdrccq/datafusion that referenced this pull request Oct 16, 2026
## Which issue does this PR close?

- Closes apache#25532.

## Rationale for this change

A view that exposes integer ids as strings, queried with an integer
predicate, fails with an internal error even though every value is
valid:

```sql
CREATE VIEW text_ids AS SELECT CAST(id AS VARCHAR) AS id FROM ids;   -- ids holds 2 and 10
SELECT id FROM text_ids WHERE CAST(id AS INT) > 1;
```

```
Internal error: Assertion failed: interval.lower.is_null() || interval.upper.is_null() || interval.lower <=
interval.upper: Interval's lower bound 2 is greater than the upper bound 10.
```

The cause is in `check_support` in
`datafusion/physical-expr/src/intervals/utils.rs`. On main the
`CastExpr` arm (lines 55 and 56) is:

```rust
} else if let Some(cast) = expr.downcast_ref::<CastExpr>() {
    check_support(cast.expr(), schema)
```

It recurses into the operand and never looks at `cast.cast_type()`.
`is_datatype_supported` (lines 93 to 110) allows the integer, float and
temporal types, and deliberately does not include `Utf8`, but that
allowlist is only ever consulted for columns and literals, never for a
cast target.

So `check_support` returns `true` for the expression above. `filter.rs`
calls `analyze(...)?` inside that branch, with no fallback, and
`Interval::cast_to` in
`datafusion/expr-common/src/interval_arithmetic.rs` converts the two
endpoints independently before handing them to `Interval::try_new`. The
numeric interval `[2, 10]` derived from Parquet statistics becomes
`["2", "10"]`, and under string ordering `"2" > "10"`, so `try_new`
rejects it and the query fails.

One nuance worth stating, because it explains why an obvious test does
not reproduce it. The simple shape `CAST(id AS VARCHAR) = '5'` is
already rejected, because the literal `'5'` is `Utf8` and the existing
literal check catches it. Reproducing the bug needs the unsupported type
to appear only in the middle of a cast chain whose outermost type is
supported, which is exactly the view scenario in the issue: the view
supplies the inner cast to string, and the user predicate supplies the
outer cast back to integer. Nothing in the expression tree is then
string typed at a point the current checks look at.

## What changes are included in this PR?

The `CastExpr` arm now requires the cast target type to be one interval
arithmetic can order before it recurses:

```rust
} else if let Some(cast) = expr.downcast_ref::<CastExpr>() {
    is_datatype_supported(cast.cast_type()) && check_support(cast.expr(), schema)
```

This is strictly analysis disabling. `check_support` is a gate that
decides whether to attempt interval analysis at all, and the change can
only ever turn a `true` into a `false`. It cannot alter any interval
that is computed today, so no currently valid numeric or temporal
interval can regress. The only effect is that an expression casting
through a type outside `is_datatype_supported` falls back to not being
analyzed, which is the behavior these expressions should have had all
along.

This also follows the direction of apache#21520, which last touched this file
and extended the same allowlist for temporal types.

## What is the testing strategy for this PR?

Two unit tests in a new `tests` module in
`datafusion/physical-expr/src/intervals/utils.rs`, covering both the
direct unsupported target and the round trip through an unsupported
type, plus a positive case asserting supported cast targets are still
analyzed. Plus an end to end case in
`datafusion/sqllogictest/test_files/cast.slt` that reproduces the issue
over Parquet, so the statistics derived interval is real rather than
synthetic.

Unit tests, against unmodified source:

```
---- intervals::utils::tests::test_check_support_rejects_unsupported_cast_target stdout ----
thread 'intervals::utils::tests::test_check_support_rejects_unsupported_cast_target' panicked at
datafusion/physical-expr/src/intervals/utils.rs:219:9:
assertion failed: !check_support(&to_utf8, &schema)

test result: FAILED. 1 passed; 1 failed; 0 ignored; 0 measured; 1664 filtered out
```

sqllogictest, against unmodified source:

```
1. query failed: DataFusion error: Internal error: Assertion failed: interval.lower.is_null() || interval.upper.is_null() || interval.lower <=
interval.upper: Interval's lower bound 2 is greater than the upper bound 10.
[SQL] SELECT id FROM cast_interval_text_ids WHERE CAST(id AS INT) > 1 ORDER BY id;
at test_files/cast.slt:283
```

That is the exact error from the issue.

With the fix, `cargo test -p datafusion-physical-expr`:

```
test result: ok. 1677 passed; 0 failed; 2 ignored; 0 measured; 0 filtered out
```

The baseline on `main` for the same crate was 1136 passing in the
`intervals` filter, and 1138 after adding the two new tests, so nothing
existing changed status.

`cargo test --test sqllogictests -- cast.slt` passes, and `cargo fmt
--all -- --check` and `cargo clippy -p datafusion-physical-expr
--all-targets --all-features -- -D warnings` are both clean.

## Are there any user-facing changes?

Yes, in the sense that a query that previously failed with an internal
error now returns the correct rows. There are no API changes.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

common Related to common crate core Core DataFusion crate logical-expr Logical plan and expressions physical-expr Changes to the physical-expr crates physical-plan Changes to the physical-plan crate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

feat: Extend single-value NDV optimization to timestamp and interval types

2 participants