Skip to content

fix: validate cast target type in interval check_support - #25562

Open
shoemoney wants to merge 1 commit into
apache:mainfrom
shoemoney:fix-25532-cast-target-check
Open

shoemoney wants to merge 1 commit into
apache:mainfrom
shoemoney:fix-25532-cast-target-check

Conversation

@shoemoney

Copy link
Copy Markdown

Which issue does this PR close?

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:

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:

} 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:

} 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 #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.

check_support recursed through a CastExpr without inspecting the cast
target type, so an expression casting through a type that interval
arithmetic cannot order (for example Utf8) was reported as analyzable.
Interval::cast_to then converted both endpoints individually and
Interval::try_new rejected the result, failing a valid query with an
internal error about reversed bounds.

Require is_datatype_supported for the cast target before recursing.

Closes apache#25532.
@github-actions github-actions Bot added physical-expr Changes to the physical-expr crates sqllogictest SQL Logic Tests (.slt) labels Sep 21, 2026

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

Thanks @shoemoney , LGTM

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

Labels

physical-expr Changes to the physical-expr crates sqllogictest SQL Logic Tests (.slt)

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Numeric-to-string cast interval inference can reject valid filters with reversed bounds

2 participants