You are viewing a plain text version of this content. The canonical link for it is here.
Posted to commits@datafusion.apache.org by gi...@apache.org on 2026/10/16 20:43:34 UTC
(datafusion) branch gh-readonly-queue/main/pr-25562-63a2801c1b0b766cc206c2bbebde9c52dac66f60 created (now 37c954c639)
This is an automated email from the ASF dual-hosted git repository.
github-merge-queue[bot] pushed a change to branch gh-readonly-queue/main/pr-25562-63a2801c1b0b766cc206c2bbebde9c52dac66f60
in repository https://gitbox.apache.org/repos/asf/datafusion.git
at 37c954c639 fix: validate cast target type in interval check_support (#25562)
This branch includes the following new commits:
new 37c954c639 fix: validate cast target type in interval check_support (#25562)
The 1 revisions listed above as "new" are entirely new to this
repository and will be described in separate emails. The revisions
listed as "add" were already present in the repository and have only
been added to this reference.
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datafusion.apache.org
For additional commands, e-mail: commits-help@datafusion.apache.org
(datafusion) 01/01: fix: validate cast target type in interval check_support (#25562)
Posted by gi...@apache.org.
This is an automated email from the ASF dual-hosted git repository.
github-merge-queue[bot] pushed a commit to branch gh-readonly-queue/main/pr-25562-63a2801c1b0b766cc206c2bbebde9c52dac66f60
in repository https://gitbox.apache.org/repos/asf/datafusion.git
commit 37c954c639da635178d16f4f311d40cb60eefce7
Author: Jeremy Schoemaker <je...@shoemoney.com>
AuthorDate: Fri Oct 16 20:43:13 2026 +0000
fix: validate cast target type in interval check_support (#25562)
## Which issue does this PR close?
- Closes #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 #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.
---
datafusion/physical-expr/src/intervals/utils.rs | 6 ++++-
datafusion/sqllogictest/test_files/cast.slt | 36 +++++++++++++++++++++++++
2 files changed, 41 insertions(+), 1 deletion(-)
diff --git a/datafusion/physical-expr/src/intervals/utils.rs b/datafusion/physical-expr/src/intervals/utils.rs
index 1090660a6b..e85e7a5d26 100644
--- a/datafusion/physical-expr/src/intervals/utils.rs
+++ b/datafusion/physical-expr/src/intervals/utils.rs
@@ -53,7 +53,11 @@ pub fn check_support(expr: &Arc<dyn PhysicalExpr>, schema: &SchemaRef) -> bool {
false
}
} else if let Some(cast) = expr.downcast_ref::<CastExpr>() {
- check_support(cast.expr(), schema)
+ // The cast target type must itself be one we can reason about. Casting
+ // interval endpoints to a type with a different ordering (e.g. integers
+ // to strings) does not preserve the interval, so such expressions are
+ // left unanalyzed rather than producing bogus bounds.
+ is_datatype_supported(cast.cast_type()) && check_support(cast.expr(), schema)
} else if let Some(negative) = expr.downcast_ref::<NegativeExpr>() {
check_support(negative.arg(), schema)
} else {
diff --git a/datafusion/sqllogictest/test_files/cast.slt b/datafusion/sqllogictest/test_files/cast.slt
index 6d62461235..414ee2f636 100644
--- a/datafusion/sqllogictest/test_files/cast.slt
+++ b/datafusion/sqllogictest/test_files/cast.slt
@@ -332,3 +332,39 @@ NULL NULL
statement ok
DROP TABLE narrow_column;
+
+# Casting integer ids to strings in a view and back to integers in the filter
+# must not use string-ordered interval bounds derived from numeric statistics.
+
+statement ok
+COPY (SELECT column1 AS id FROM (VALUES (2), (10)))
+TO 'test_files/scratch/cast/cast_interval_ids.parquet'
+STORED AS PARQUET;
+
+statement ok
+CREATE EXTERNAL TABLE cast_interval_ids
+STORED AS PARQUET
+LOCATION 'test_files/scratch/cast/cast_interval_ids.parquet';
+
+statement ok
+CREATE VIEW cast_interval_text_ids AS
+SELECT CAST(id AS VARCHAR) AS id FROM cast_interval_ids;
+
+query T
+SELECT id FROM cast_interval_text_ids WHERE CAST(id AS INT) > 1 ORDER BY id;
+----
+10
+2
+
+# Selective filtering through the cast must apply the numeric predicate after
+# casting, not string-ordered interval bounds derived from the statistics.
+query T
+SELECT id FROM cast_interval_text_ids WHERE CAST(id AS INT) > 5;
+----
+10
+
+statement ok
+DROP VIEW cast_interval_text_ids;
+
+statement ok
+DROP TABLE cast_interval_ids;
---------------------------------------------------------------------
To unsubscribe, e-mail: commits-unsubscribe@datafusion.apache.org
For additional commands, e-mail: commits-help@datafusion.apache.org