You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "neilconway (via GitHub)" <gi...@apache.org> on 2026/02/19 02:09:19 UTC
[PR] fix: Prefer numeric in type coercion for comparisons [datafusion]
neilconway opened a new pull request, #20426:
URL: https://github.com/apache/datafusion/pull/20426
## Which issue does this PR close?
- Closes #15161.
## Rationale for this change
In a comparison between a numeric column and a string literal (e.g., `WHERE int_col < '10'`), we previously coerced the numeric column to be a string type. This resulted in doing a lexicographic comparison, which results in incorrect query results.
Instead, we split type coercion into two situations: type coercion for comparisons, where we want string->numeric coercion, and type coercion for places like UNION or IN lists, where DataFusion's traditional behavior has been to tolerate type mismatching by coercing values to strings.
## What changes are included in this PR?
* Update `comparison_coercion` to coerce strings to numerics
* Remove previous `comparison_coercion_numeric` function
* Add a new function, `type_union_coercion`, and use it when appropriate (UNION, CASE, NVL, IN lists, and struct fields)
* Add unit and SLT tests for new coercion behavior
* Update existing SLT tests for changes in coercion behavior
## Are these changes tested?
Yes. New tests added, existing tests pass. (TODO: ClickBench queries need to be fixed.)
## Are there any user-facing changes?
Yes:
* Comparing a numeric column to a string literal will now behave differently (the previous behavior was wrong)
* Comparing a string column with a numeric literal will now attempt to cast the string column to the numeric type (instead of coercing the numeric literal to string and then doing lexicographic comparison); if the column contains any non-numeric values, the query will fail at runtime
* Comparing a string column with a numeric column will now cast the string column to numeric, similar behavior to the previous case
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: Prefer numeric in type coercion for comparisons [datafusion]
Posted by "martin-g (via GitHub)" <gi...@apache.org>.
martin-g commented on code in PR #20426:
URL: https://github.com/apache/datafusion/pull/20426#discussion_r2908005743
##########
datafusion/expr-common/src/signature.rs:
##########
@@ -158,7 +158,7 @@ pub enum Arity {
pub enum TypeSignature {
/// One or more arguments of a common type out of a list of valid types.
///
- /// For functions that take no arguments (e.g. `random()` see [`TypeSignature::Nullary`]).
+ /// For functions that take no arguments (e.g. `random()`), see [`TypeSignature::Nullary`]).
Review Comment:
```suggestion
/// For functions that take no arguments (e.g. `random()`), see [`TypeSignature::Nullary`].
```
##########
datafusion/expr/src/type_coercion/other.rs:
##########
@@ -17,38 +17,58 @@
use arrow::datatypes::DataType;
-use super::binary::comparison_coercion;
+use super::binary::{comparison_coercion, type_union_coercion};
+
+/// Fold `coerce_fn` over `types`, starting from `initial_type`.
+fn fold_coerce(
+ initial_type: &DataType,
+ types: &[DataType],
+ coerce_fn: fn(&DataType, &DataType) -> Option<DataType>,
+) -> Option<DataType> {
+ types
+ .iter()
+ .try_fold(initial_type.clone(), |left_type, right_type| {
+ coerce_fn(&left_type, right_type)
+ })
+}
/// Attempts to coerce the types of `list_types` to be comparable with the
-/// `expr_type`.
-/// Returns the common data type for `expr_type` and `list_types`
+/// `expr_type` for IN list predicates.
+/// Returns the common data type for `expr_type` and `list_types`.
+///
+/// Uses comparison coercion because `x IN (a, b)` is semantically equivalent
+/// to `x = a OR x = b`.
pub fn get_coerce_type_for_list(
expr_type: &DataType,
list_types: &[DataType],
) -> Option<DataType> {
- list_types
- .iter()
- .try_fold(expr_type.clone(), |left_type, right_type| {
- comparison_coercion(&left_type, right_type)
- })
+ fold_coerce(expr_type, list_types, comparison_coercion)
+}
+
+/// Find a common coerceable type for `CASE expr WHEN val1 WHEN val2 ...`
+/// conditions. Returns the common type for `case_type` and all `when_types`.
+///
+/// Uses comparison coercion because `CASE expr WHEN val` is semantically
+/// equivalent to `expr = val`.
+pub fn get_coerce_type_for_case_when(
+ when_types: &[DataType],
+ case_type: &DataType,
+) -> Option<DataType> {
+ fold_coerce(case_type, when_types, comparison_coercion)
}
-/// Find a common coerceable type for all `when_or_then_types` as well
-/// and the `case_or_else_type`, if specified.
-/// Returns the common data type for `when_or_then_types` and `case_or_else_type`
+/// Find a common coerceable type for CASE THEN/ELSE result expressions.
+/// Returns the common data type for `then_types` and `else_type`.
+///
+/// Uses type union coercion because the result branches must be brought to a
+/// common type (like UNION), not compared.
pub fn get_coerce_type_for_case_expression(
- when_or_then_types: &[DataType],
- case_or_else_type: Option<&DataType>,
+ then_types: &[DataType],
+ else_type: Option<&DataType>,
) -> Option<DataType> {
- let case_or_else_type = match case_or_else_type {
- None => when_or_then_types[0].clone(),
- Some(data_type) => data_type.clone(),
+ let (initial_type, remaining) = match else_type {
+ None => (&then_types[0], &then_types[1..]),
Review Comment:
```suggestion
None => then_types.split_first()?,
```
this would be more safe since it returns None if the slice has no items
##########
datafusion/expr-common/src/signature.rs:
##########
@@ -184,38 +184,39 @@ pub enum TypeSignature {
Uniform(usize, Vec<DataType>),
/// One or more arguments with exactly the specified types in order.
///
- /// For functions that take no arguments (e.g. `random()`) use [`TypeSignature::Nullary`].
+ /// For functions that take no arguments (e.g. `random()`), use [`TypeSignature::Nullary`].
Exact(Vec<DataType>),
/// One or more arguments belonging to the [`TypeSignatureClass`], in order.
///
/// [`Coercion`] contains not only the desired type but also the allowed
/// casts. For example, if you expect a function has string type, but you
/// also allow it to be casted from binary type.
///
- /// For functions that take no arguments (e.g. `random()`) see [`TypeSignature::Nullary`].
+ /// For functions that take no arguments (e.g. `random()`), see [`TypeSignature::Nullary`].
Coercible(Vec<Coercion>),
/// One or more arguments coercible to a single, comparable type.
///
/// Each argument will be coerced to a single type using the
- /// coercion rules described in [`comparison_coercion_numeric`].
+ /// coercion rules described in [`comparison_coercion`].
///
/// # Examples
///
/// If the `nullif(1, 2)` function is called with `i32` and `i64` arguments
/// the types will both be coerced to `i64` before the function is invoked.
///
/// If the `nullif('1', 2)` function is called with `Utf8` and `i64` arguments
- /// the types will both be coerced to `Utf8` before the function is invoked.
+ /// the types will both be coerced to `Int64` before the function is invoked
+ /// (numeric is preferred over string).
///
/// Note:
- /// - For functions that take no arguments (e.g. `random()` see [`TypeSignature::Nullary`]).
+ /// - For functions that take no arguments (e.g. `random()`), see [`TypeSignature::Nullary`]).
Review Comment:
```suggestion
/// - For functions that take no arguments (e.g. `random()`), see [`TypeSignature::Nullary`].
```
##########
datafusion/expr-common/src/signature.rs:
##########
@@ -246,7 +247,7 @@ pub enum TypeSignature {
/// For example, if a function is called with (utf8, large_utf8), all
/// arguments will be coerced to `LargeUtf8`
///
- /// For functions that take no arguments (e.g. `random()` use [`TypeSignature::Nullary`]).
+ /// For functions that take no arguments (e.g. `random()`), use [`TypeSignature::Nullary`]).
Review Comment:
```suggestion
/// For functions that take no arguments (e.g. `random()`), use [`TypeSignature::Nullary`].
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: Prefer numeric in type coercion for comparisons [datafusion]
Posted by "adriangb (via GitHub)" <gi...@apache.org>.
adriangb commented on PR #20426:
URL: https://github.com/apache/datafusion/pull/20426#issuecomment-4185171273
Thanks @neilconway !!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: Prefer numeric in type coercion for comparisons [datafusion]
Posted by "neilconway (via GitHub)" <gi...@apache.org>.
neilconway commented on PR #20426:
URL: https://github.com/apache/datafusion/pull/20426#issuecomment-3924284302
TODOs:
* [ ] ClickBench queries are broken, needs investigation
* [ ] We might be able to remove some/all of the special-case logic in `cast_literal_to_type_with_op` (see #15110)
* [ ] Decide whether type union coercion is well-motivated, or if we want to remove it
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: Prefer numeric in type coercion for comparisons [datafusion]
Posted by "cetra3 (via GitHub)" <gi...@apache.org>.
cetra3 commented on code in PR #20426:
URL: https://github.com/apache/datafusion/pull/20426#discussion_r2892288270
##########
datafusion/expr-common/src/type_coercion/binary.rs:
##########
@@ -859,11 +886,15 @@ pub fn try_type_union_resolution_with_struct(
///
/// # Numeric / String comparisons
///
-/// When comparing numeric values and strings, both values will be coerced to
-/// strings. For example when comparing `'2' > 1`, the arguments will be
-/// coerced to `Utf8` for comparison
+/// When comparing numeric values and strings, the string value will be coerced
+/// to the numeric type. For example when comparing `'2' > 1` where `1` is
+/// `Int32`, `'2'` will be coerced to `Int32` for comparison.
+///
+/// For type unification contexts (see [`type_union_coercion`]), use
+/// [`type_union_coercion`] instead, which prefers strings as the safe widening
+/// type.
pub fn comparison_coercion(lhs_type: &DataType, rhs_type: &DataType) -> Option<DataType> {
- if lhs_type.equals_datatype(rhs_type) {
+ if lhs_type == rhs_type {
Review Comment:
Is there a reason for this change? Shouldn't we prefer `equals_datatype` here
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: Prefer numeric in type coercion for comparisons [datafusion]
Posted by "neilconway (via GitHub)" <gi...@apache.org>.
neilconway commented on PR #20426:
URL: https://github.com/apache/datafusion/pull/20426#issuecomment-4184494630
@adriangb Thanks! I merged in the latest main and it seems CI is happy again.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: Prefer numeric in type coercion for comparisons [datafusion]
Posted by "neilconway (via GitHub)" <gi...@apache.org>.
neilconway commented on PR #20426:
URL: https://github.com/apache/datafusion/pull/20426#issuecomment-4007883127
> LGTM but obviously need to work out what to do about those clickbench queries
Thanks for the review, @cetra3 !
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: Prefer numeric in type coercion for comparisons [datafusion]
Posted by "neilconway (via GitHub)" <gi...@apache.org>.
neilconway commented on code in PR #20426:
URL: https://github.com/apache/datafusion/pull/20426#discussion_r2892369063
##########
datafusion/core/tests/sql/unparser.rs:
##########
@@ -107,6 +107,14 @@ struct TestQuery {
/// Collect SQL for Clickbench queries.
fn clickbench_queries() -> Vec<TestQuery> {
+ // q36-q42 compare UInt16 "EventDate" column with date strings like '2013-07-01'.
+ // With numeric-preferring comparison coercion, these fail because a date string
+ // can't be cast to UInt16. These queries use ClickHouse conventions where
+ // EventDate is stored as a day-offset integer.
+ //
+ // TODO: fix this
+ const SKIP_QUERIES: &[&str] = &["q36", "q37", "q38", "q39", "q40", "q41", "q42"];
Review Comment:
I think I have a solution, similar to what we do when we're actually running the queries -- load the data into a `xyz_raw` table and then define a view `xyz` that does the appropriate transform, basically.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: Prefer numeric in type coercion for comparisons [datafusion]
Posted by "cetra3 (via GitHub)" <gi...@apache.org>.
cetra3 commented on code in PR #20426:
URL: https://github.com/apache/datafusion/pull/20426#discussion_r2892298774
##########
datafusion/core/tests/sql/unparser.rs:
##########
@@ -107,6 +107,14 @@ struct TestQuery {
/// Collect SQL for Clickbench queries.
fn clickbench_queries() -> Vec<TestQuery> {
+ // q36-q42 compare UInt16 "EventDate" column with date strings like '2013-07-01'.
+ // With numeric-preferring comparison coercion, these fail because a date string
+ // can't be cast to UInt16. These queries use ClickHouse conventions where
+ // EventDate is stored as a day-offset integer.
+ //
+ // TODO: fix this
+ const SKIP_QUERIES: &[&str] = &["q36", "q37", "q38", "q39", "q40", "q41", "q42"];
Review Comment:
Is there a world where we can make the appropriate changes and get these queries working?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: Prefer numeric in type coercion for comparisons [datafusion]
Posted by "neilconway (via GitHub)" <gi...@apache.org>.
neilconway commented on PR #20426:
URL: https://github.com/apache/datafusion/pull/20426#issuecomment-4027406189
@adriangb @martin-g Thank you for the reviews!
I added a section to the upgrade guide for this change.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: Prefer numeric in type coercion for comparisons [datafusion]
Posted by "neilconway (via GitHub)" <gi...@apache.org>.
neilconway commented on code in PR #20426:
URL: https://github.com/apache/datafusion/pull/20426#discussion_r2908282619
##########
datafusion/core/src/physical_planner.rs:
##########
@@ -3436,16 +3436,16 @@ mod tests {
#[tokio::test]
async fn in_list_types() -> Result<()> {
- // expression: "a in ('a', 1)"
- let list = vec![lit("a"), lit(1i64)];
Review Comment:
That's right; and good point, I updated the test to expicitly check for the type mismatch error.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: Prefer numeric in type coercion for comparisons [datafusion]
Posted by "neilconway (via GitHub)" <gi...@apache.org>.
neilconway commented on PR #20426:
URL: https://github.com/apache/datafusion/pull/20426#issuecomment-3924347326
The [ClickBench queries](https://github.com/apache/datafusion/blob/c699361cb4ae13276ca4bd936d3ed4d3cffe78e6/datafusion/core/tests/sql/unparser.rs#L109) are just weird: they have expressions like `"EventDate" >= '2013-07-01'`, where `EventDate` is a `uint16`. Previously, `EventDate` would be cast to a string and the comparison would have been done lexicographically, which is wrong but didn't fail. Now we'll try to coerce the string literal to a numeric, which fails. So this is intended behavior -- the ClickBench queries need fixing up some other way.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: Prefer numeric in type coercion for comparisons [datafusion]
Posted by "neilconway (via GitHub)" <gi...@apache.org>.
neilconway commented on PR #20426:
URL: https://github.com/apache/datafusion/pull/20426#issuecomment-4184033827
@adriangb @alamb @martin-g Just checking in here -- do we think this is ready to land?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: Prefer numeric in type coercion for comparisons [datafusion]
Posted by "adriangb (via GitHub)" <gi...@apache.org>.
adriangb merged PR #20426:
URL: https://github.com/apache/datafusion/pull/20426
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: Prefer numeric in type coercion for comparisons [datafusion]
Posted by "neilconway (via GitHub)" <gi...@apache.org>.
neilconway commented on PR #20426:
URL: https://github.com/apache/datafusion/pull/20426#issuecomment-4185188406
Thanks for the reviews and discussion! @adriangb @martin-g @alamb @milenkovicm @cetra3 @AlonSpivack
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: Prefer numeric in type coercion for comparisons [datafusion]
Posted by "milenkovicm (via GitHub)" <gi...@apache.org>.
milenkovicm commented on PR #20426:
URL: https://github.com/apache/datafusion/pull/20426#issuecomment-4026321284
I'm not sure if i can give meaningful contribution in this case, ballista is relaying on datafusion for this behaviour.
would we need to capture change of behaviour somewhere, like upgrade guide ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: Prefer numeric in type coercion for comparisons [datafusion]
Posted by "neilconway (via GitHub)" <gi...@apache.org>.
neilconway commented on PR #20426:
URL: https://github.com/apache/datafusion/pull/20426#issuecomment-3924335597
One example of a weird behavior that we get with the new coercion rules proposed here:
```
> create table t1 (a text, b int, c float);
0 row(s) fetched.
Elapsed 0.018 seconds.
> explain select * from t1 where a < 5;
+---------------+-------------------------------+
| plan_type | plan |
+---------------+-------------------------------+
| physical_plan | ┌───────────────────────────┐ |
| | │ FilterExec │ |
| | │ -------------------- │ |
| | │ predicate: │ |
| | │ CAST(a AS Int64) < 5 │ |
| | └─────────────┬─────────────┘ |
| | ┌─────────────┴─────────────┐ |
| | │ DataSourceExec │ |
| | │ -------------------- │ |
| | │ bytes: 0 │ |
| | │ format: memory │ |
| | │ rows: 0 │ |
| | └───────────────────────────┘ |
| | |
+---------------+-------------------------------+
1 row(s) fetched.
Elapsed 0.016 seconds.
```
This follows from the "coerce string to numeric" change in general, but it's a bit weird. I think we should probably reject queries like this entirely, on the basis of a type mismatch.
If we did that and nothing else, we'd break a ton of queries (e.g., anything comparing a numeric column with a string literal). But we could fix that up by treating comparisons with string literals specially (e.g., "if x OP y mismatch in types, check if either x or y is a string literal; if so, try to coerce it to a numeric type"). So that's an alternative approach worth considering.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: Prefer numeric in type coercion for comparisons [datafusion]
Posted by "neilconway (via GitHub)" <gi...@apache.org>.
neilconway commented on PR #20426:
URL: https://github.com/apache/datafusion/pull/20426#issuecomment-4009240397
@alamb @adriangb @comphead FYI, I revised this PR and I think the current version is ready for review. Potentially a candidate for 53, if there's time.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: Prefer numeric in type coercion for comparisons [datafusion]
Posted by "neilconway (via GitHub)" <gi...@apache.org>.
neilconway commented on code in PR #20426:
URL: https://github.com/apache/datafusion/pull/20426#discussion_r2994892779
##########
datafusion/expr-common/src/type_coercion/binary.rs:
##########
@@ -748,7 +747,7 @@ fn type_union_resolution_coercion(
.or_else(|| list_coercion(lhs_type, rhs_type))
.or_else(|| temporal_coercion_nonstrict_timezone(lhs_type, rhs_type))
.or_else(|| string_coercion(lhs_type, rhs_type))
- .or_else(|| numeric_string_coercion(lhs_type, rhs_type))
+ .or_else(|| string_numeric_coercion(lhs_type, rhs_type))
Review Comment:
This is a good and subtle point that should be called out. The previous behavior actually already implemented numeric-preferring coercion in this case: this applied to situations like `VALUES` lists, array literals, or `COALESCE` expressions. So the prior behavior was not super consistent -- we implemented type-union-like behavior in some "type union-y" situations but not in others. Arguably, it would be more self-consistent to change this situation to be string-preferring, for consistency with the rest of this PR, but I think there are two good reasons not to do so:
1. Further change to query behavior
2. I'd like to see us eventually move toward stricter typing behavior, not looser, and this would be a step in the wrong direction that IMO we would eventually want to revert anyway, which would mean another breaking change.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: Prefer numeric in type coercion for comparisons [datafusion]
Posted by "milenkovicm (via GitHub)" <gi...@apache.org>.
milenkovicm commented on PR #20426:
URL: https://github.com/apache/datafusion/pull/20426#issuecomment-4026563388
Maybe sending an email to dev list for visibility would make sense, would it?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: Prefer numeric in type coercion for comparisons [datafusion]
Posted by "AlonSpivack (via GitHub)" <gi...@apache.org>.
AlonSpivack commented on PR #20426:
URL: https://github.com/apache/datafusion/pull/20426#issuecomment-3925402231
Thanks for putting this PR together @neilconway!
Splitting out `type_union_coercion` is definitely the right architectural move.
Regarding your dilemma about `text_col < 5`:
I strongly agree with your intuition that we should **reject these queries entirely** with a clear type-mismatch/planning error, rather than trying to add special-case fallback logic. If a user actually wants to compare a string column to a number, they should be required to add an explicit `CAST`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: Prefer numeric in type coercion for comparisons [datafusion]
Posted by "neilconway (via GitHub)" <gi...@apache.org>.
neilconway commented on code in PR #20426:
URL: https://github.com/apache/datafusion/pull/20426#discussion_r2995497654
##########
datafusion/optimizer/src/analyzer/type_coercion.rs:
##########
Review Comment:
You mean, does this code need to be updated as part of this PR? I'd argue no -- the pre-existing behavior here was strict and not to do type coercion, which IMO makes sense.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: Prefer numeric in type coercion for comparisons [datafusion]
Posted by "neilconway (via GitHub)" <gi...@apache.org>.
neilconway commented on code in PR #20426:
URL: https://github.com/apache/datafusion/pull/20426#discussion_r2995453628
##########
datafusion/expr-common/src/type_coercion/binary.rs:
##########
@@ -1707,7 +1636,8 @@ fn coerce_list_children(lhs_field: &FieldRef, rhs_field: &FieldRef) -> Option<Fi
))
}
-/// Coercion rules for list types.
+/// Coerce two list types by coercing their element types via
+/// [`type_union_resolution`].
fn list_coercion(lhs_type: &DataType, rhs_type: &DataType) -> Option<DataType> {
Review Comment:
Fixed to use `coerce_fn`.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: Prefer numeric in type coercion for comparisons [datafusion]
Posted by "neilconway (via GitHub)" <gi...@apache.org>.
neilconway commented on PR #20426:
URL: https://github.com/apache/datafusion/pull/20426#issuecomment-4135955294
@martin-g Thanks for the great code review!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: Prefer numeric in type coercion for comparisons [datafusion]
Posted by "neilconway (via GitHub)" <gi...@apache.org>.
neilconway commented on code in PR #20426:
URL: https://github.com/apache/datafusion/pull/20426#discussion_r2892311093
##########
datafusion/expr-common/src/type_coercion/binary.rs:
##########
@@ -859,11 +886,15 @@ pub fn try_type_union_resolution_with_struct(
///
/// # Numeric / String comparisons
///
-/// When comparing numeric values and strings, both values will be coerced to
-/// strings. For example when comparing `'2' > 1`, the arguments will be
-/// coerced to `Utf8` for comparison
+/// When comparing numeric values and strings, the string value will be coerced
+/// to the numeric type. For example when comparing `'2' > 1` where `1` is
+/// `Int32`, `'2'` will be coerced to `Int32` for comparison.
+///
+/// For type unification contexts (see [`type_union_coercion`]), use
+/// [`type_union_coercion`] instead, which prefers strings as the safe widening
+/// type.
pub fn comparison_coercion(lhs_type: &DataType, rhs_type: &DataType) -> Option<DataType> {
- if lhs_type.equals_datatype(rhs_type) {
+ if lhs_type == rhs_type {
Review Comment:
Yes, I think `equals_datatype` is right here.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: Prefer numeric in type coercion for comparisons [datafusion]
Posted by "martin-g (via GitHub)" <gi...@apache.org>.
martin-g commented on code in PR #20426:
URL: https://github.com/apache/datafusion/pull/20426#discussion_r2991306254
##########
datafusion/optimizer/src/analyzer/type_coercion.rs:
##########
Review Comment:
Is this check related to the changes in this PR ?
##########
docs/source/library-user-guide/upgrading/53.0.0.md:
##########
Review Comment:
This should probably be moved to 54.0.0.md now
##########
datafusion/core/tests/expr_api/mod.rs:
##########
@@ -342,20 +342,25 @@ fn test_create_physical_expr_nvl2() {
#[tokio::test]
async fn test_create_physical_expr_coercion() {
- // create_physical_expr does apply type coercion and unwrapping in cast
+ // create_physical_expr applies type coercion (and can unwrap/fold
+ // literal casts). Comparison coercion prefers numeric types, so
+ // string/int comparisons cast the string side to the numeric type.
//
- // expect the cast on the literals
- // compare string function to int `id = 1`
- create_expr_test(col("id").eq(lit(1i32)), "id@0 = CAST(1 AS Utf8)");
- create_expr_test(lit(1i32).eq(col("id")), "CAST(1 AS Utf8) = id@0");
- // compare int col to string literal `i = '202410'`
- // Note this casts the column (not the field)
- create_expr_test(col("i").eq(lit("202410")), "CAST(i@1 AS Utf8) = 202410");
- create_expr_test(lit("202410").eq(col("i")), "202410 = CAST(i@1 AS Utf8)");
- // however, when simplified the casts on i should removed
- // https://github.com/apache/datafusion/issues/14944
- create_simplified_expr_test(col("i").eq(lit("202410")), "CAST(i@1 AS Utf8) = 202410");
- create_simplified_expr_test(lit("202410").eq(col("i")), "CAST(i@1 AS Utf8) = 202410");
+ // string column vs int literal: id (Utf8) is cast to Int32
+ create_expr_test(col("id").eq(lit(1i32)), "CAST(id@0 AS Int32) = 1");
+ create_expr_test(lit(1i32).eq(col("id")), "1 = CAST(id@0 AS Int32)");
+ // int column vs string literal: the string literal is cast to Int64
+ create_expr_test(col("i").eq(lit("202410")), "i@1 = CAST(202410 AS Int64)");
+ create_expr_test(lit("202410").eq(col("i")), "CAST(202410 AS Int64) = i@1");
+ // when simplified, the literal cast is constant-folded
Review Comment:
Here it says `the literal cast is constant-folded` but the expected_expr is still a `CAST(...)`, not just a constant.
##########
datafusion/expr-common/src/type_coercion/binary.rs:
##########
@@ -748,7 +747,7 @@ fn type_union_resolution_coercion(
.or_else(|| list_coercion(lhs_type, rhs_type))
.or_else(|| temporal_coercion_nonstrict_timezone(lhs_type, rhs_type))
.or_else(|| string_coercion(lhs_type, rhs_type))
- .or_else(|| numeric_string_coercion(lhs_type, rhs_type))
+ .or_else(|| string_numeric_coercion(lhs_type, rhs_type))
Review Comment:
Shouldn't this be `string_numeric_union_coercion`, i.e. string-preferred coercion for UNION ?
##########
datafusion/sqllogictest/test_files/string_numeric_coercion.slt:
##########
@@ -0,0 +1,496 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+##########
+## Tests for string-numeric comparison coercion
+## Verifies that when comparing a numeric column to a string literal,
+## the comparison is performed numerically (not lexicographically).
+## See: https://github.com/apache/datafusion/issues/15161
+##########
+
+# Setup test data
+statement ok
+CREATE TABLE t_int AS VALUES (1), (5), (325), (499), (1000);
+
+statement ok
+CREATE TABLE t_float AS VALUES (1.5), (5.0), (325.7), (499.9), (1000.1);
+
+# -------------------------------------------------
+# Integer column with comparison operators vs string literals.
+# Ensure that the comparison is done with numeric semantics,
+# not lexicographically.
+# -------------------------------------------------
+
+query I rowsort
+SELECT * FROM t_int WHERE column1 < '5';
+----
+1
+
+query I rowsort
+SELECT * FROM t_int WHERE column1 > '5';
+----
+1000
+325
+499
+
+query I rowsort
+SELECT * FROM t_int WHERE column1 <= '5';
+----
+1
+5
+
+query I rowsort
+SELECT * FROM t_int WHERE column1 >= '5';
+----
+1000
+325
+499
+5
+
+query I rowsort
+SELECT * FROM t_int WHERE column1 = '5';
+----
+5
+
+query I rowsort
+SELECT * FROM t_int WHERE column1 != '5';
+----
+1
+1000
+325
+499
+
+query I rowsort
+SELECT * FROM t_int WHERE column1 < '10';
+----
+1
+5
+
+query I rowsort
+SELECT * FROM t_int WHERE column1 <= '100';
+----
+1
+5
+
+query I rowsort
+SELECT * FROM t_int WHERE column1 > '100';
+----
+1000
+325
+499
+
+# -------------------------------------------------
+# Float column with comparison operators vs string literals
+# -------------------------------------------------
+
+query R rowsort
+SELECT * FROM t_float WHERE column1 < '5';
+----
+1.5
+
+query R rowsort
+SELECT * FROM t_float WHERE column1 > '5';
+----
+1000.1
+325.7
+499.9
+
+query R rowsort
+SELECT * FROM t_float WHERE column1 = '5';
+----
+5
+
+query R rowsort
+SELECT * FROM t_float WHERE column1 = '5.0';
+----
+5
+
+# -------------------------------------------------
+# Error on strings that cannot be cast to the numeric column type
+# -------------------------------------------------
+
+# Non-numeric string against integer column
+statement error Arrow error: Cast error: Cannot cast string 'hello' to value of Int64 type
+SELECT * FROM t_int WHERE column1 < 'hello';
+
+# Non-numeric string against float column
+statement error Arrow error: Cast error: Cannot cast string 'hello' to value of Float64 type
+SELECT * FROM t_float WHERE column1 < 'hello';
+
+# Float string against integer column
+statement error Arrow error: Cast error: Cannot cast string '99.99' to value of Int64 type
+SELECT * FROM t_int WHERE column1 = '99.99';
+
+# Empty string against integer column
+statement error Arrow error: Cast error: Cannot cast string '' to value of Int64 type
+SELECT * FROM t_int WHERE column1 = '';
+
+# Empty string against float column
+statement error Arrow error: Cast error: Cannot cast string '' to value of Float64 type
+SELECT * FROM t_float WHERE column1 = '';
+
+# Overflow
+statement error Arrow error: Cast error: Cannot cast string '99999999999999999999' to value of Int64 type
+SELECT * FROM t_int WHERE column1 = '99999999999999999999';
+
+
+# -------------------------------------------------
+# UNION still uses string coercion (type unification context)
+# -------------------------------------------------
+
+statement ok
+CREATE TABLE t_str AS VALUES ('one'), ('two'), ('three');
+
+query T rowsort
+SELECT column1 FROM t_int UNION ALL SELECT column1 FROM t_str;
+----
+1
+1000
+325
+499
+5
+one
+three
+two
+
+# Verify the UNION coerces to Utf8 (not numeric)
+query TT
+EXPLAIN SELECT column1 FROM t_int UNION ALL SELECT column1 FROM t_str;
+----
+logical_plan
+01)Union
+02)--Projection: CAST(t_int.column1 AS Utf8) AS column1
+03)----TableScan: t_int projection=[column1]
+04)--TableScan: t_str projection=[column1]
+physical_plan
+01)UnionExec
+02)--ProjectionExec: expr=[CAST(column1@0 AS Utf8) as column1]
+03)----DataSourceExec: partitions=1, partition_sizes=[1]
+04)--DataSourceExec: partitions=1, partition_sizes=[1]
+
+# -------------------------------------------------
+# BETWEEN uses comparison coercion (numeric preferred)
+# -------------------------------------------------
+
+query I rowsort
+SELECT * FROM t_int WHERE column1 BETWEEN '5' AND '100';
+----
+5
+
+# -------------------------------------------------
+# IN list uses comparison coercion (numeric preferred)
+# `x IN (a, b)` is semantically equivalent to `x = a OR x = b`
+# -------------------------------------------------
+
+# Basic IN list with string literals against integer column
+query I rowsort
+SELECT * FROM t_int WHERE column1 IN ('5', '325');
+----
+325
+5
+
+# IN list with a value where numeric coercion matters
+query I rowsort
+SELECT * FROM t_int WHERE column1 IN ('1000');
+----
+1000
+
+# IN list with NOT
+query I rowsort
+SELECT * FROM t_int WHERE column1 NOT IN ('1', '5');
+----
+1000
+325
+499
+
+# IN list with float column
+query R rowsort
+SELECT * FROM t_float WHERE column1 IN ('5.0', '325.7');
+----
+325.7
+5
+
+# Verify the plan shows numeric coercion (not CAST to Utf8)
+query TT
+EXPLAIN SELECT * FROM t_int WHERE column1 IN ('5', '325');
+----
+logical_plan
+01)Filter: t_int.column1 = Int64(5) OR t_int.column1 = Int64(325)
+02)--TableScan: t_int projection=[column1]
+physical_plan
+01)FilterExec: column1@0 = 5 OR column1@0 = 325
+02)--DataSourceExec: partitions=1, partition_sizes=[1]
+
+# Error on invalid string in IN list
+statement error Arrow error: Cast error: Cannot cast string 'hello' to value of Int64 type
+SELECT * FROM t_int WHERE column1 IN ('5', 'hello');
+
+# Mixed numeric literal and string literal in IN list against integer column
+query I rowsort
+SELECT * FROM t_int WHERE column1 IN (5, '325');
+----
+325
+5
+
+# Mixed numeric literal and string literal in IN list against float column
+query R rowsort
+SELECT * FROM t_float WHERE column1 IN (5, '325.7');
+----
+325.7
+5
+
+# String and numeric literal order reversed
+query R rowsort
+SELECT * FROM t_float WHERE column1 IN ('5', 325.7);
+----
+325.7
+5
+
+# Float string literal against integer column errors (cannot cast '10.0' to Int64)
+statement error Arrow error: Cast error: Cannot cast string '10.0' to value of Int64 type
+SELECT * FROM t_int WHERE column1 IN (5, '10.0');
+
+# Non-numeric string in mixed IN list still errors
+statement error Arrow error: Cast error: Cannot cast string 'hello' to value of Int64 type
+SELECT * FROM t_int WHERE column1 IN ('hello', 5);
+
+# -------------------------------------------------
+# CASE WHEN uses comparison coercion for conditions (numeric preferred)
+# `CASE expr WHEN val` is semantically equivalent to `expr = val`
+# -------------------------------------------------
+
+# Basic CASE with integer column and string WHEN values
+query T rowsort
+SELECT CASE column1 WHEN '5' THEN 'five' WHEN '1000' THEN 'thousand' ELSE 'other' END FROM t_int;
+----
+five
+other
+other
+other
+thousand
+
+# CASE with float column: '5' is cast to 5.0 numerically, matching the row.
+# (Under string comparison, '5.0' != '5' would fail to match.)
+query T rowsort
+SELECT CASE column1 WHEN '5' THEN 'matched' ELSE 'no match' END FROM t_float;
+----
+matched
+no match
+no match
+no match
+no match
+
+# THEN/ELSE results still use type union coercion (string preferred),
+# so mixing numeric and string coerces to string
+query T rowsort
+SELECT CASE WHEN column1 > 500 THEN column1 ELSE 'small' END FROM t_int;
+----
+1000
+small
+small
+small
+small
+
+# -------------------------------------------------
+# Nested struct/map/list comparisons use comparison coercion
+# (numeric preferred) for their field/element types
+# -------------------------------------------------
+
+statement ok
+CREATE TABLE t_struct_int AS SELECT named_struct('val', column1) as s FROM (VALUES (1), (5), (10));
+
+statement ok
+CREATE TABLE t_struct_str AS SELECT named_struct('val', column1) as s FROM (VALUES ('5'), ('10'));
+
+# Struct comparison: the string field is cast to Int64 (numeric preferred).
+query ? rowsort
+SELECT t1.s FROM t_struct_int t1, t_struct_str t2 WHERE t1.s = t2.s;
+----
+{val: 10}
+{val: 5}
+
+# Struct in UNION uses type union coercion (string preferred).
+# The integer struct field is cast to Utf8.
+query ? rowsort
+SELECT s FROM t_struct_int UNION ALL SELECT s FROM t_struct_str;
+----
+{val: 10}
+{val: 10}
+{val: 1}
+{val: 5}
+{val: 5}
+
+statement ok
+DROP TABLE t_struct_int;
+
+statement ok
+DROP TABLE t_struct_str;
+
+# List comparison: string elements are cast to Int64 (numeric preferred).
+statement ok
+CREATE TABLE t_list_int AS SELECT column1 as l FROM (VALUES ([1, 5, 10]), ([20, 30]));
+
+statement ok
+CREATE TABLE t_list_str AS SELECT column1 as l FROM (VALUES (['5', '10']), (['20', '30']));
+
+# Verify the element types are Int64 and Utf8 respectively
+query T
+SELECT arrow_typeof(l) FROM t_list_int LIMIT 1;
+----
+List(Int64)
+
+query T
+SELECT arrow_typeof(l) FROM t_list_str LIMIT 1;
+----
+List(Utf8)
+
+query ? rowsort
+SELECT t1.l FROM t_list_int t1, t_list_str t2 WHERE t1.l = t2.l;
+----
+[20, 30]
+
+# List in UNION uses type union coercion (string preferred).
+# The integer list elements are cast to Utf8.
+query ? rowsort
+SELECT l FROM t_list_int UNION ALL SELECT l FROM t_list_str;
+----
+[1, 5, 10]
Review Comment:
Here all results are numeric strings and it is hard to tell whether the integers were cast to Utf8.
Let's check the results' type to be sure that it is a List(Utf8) and not List(Int64)
##########
datafusion/expr-common/src/type_coercion/binary.rs:
##########
@@ -1707,7 +1636,8 @@ fn coerce_list_children(lhs_field: &FieldRef, rhs_field: &FieldRef) -> Option<Fi
))
}
-/// Coercion rules for list types.
+/// Coerce two list types by coercing their element types via
+/// [`type_union_resolution`].
fn list_coercion(lhs_type: &DataType, rhs_type: &DataType) -> Option<DataType> {
Review Comment:
Is it intentional that `list_coercion()` does not use `coerce_fn` as `struct_coercion()` and `map_coercion()` ?
Currently it delegates to `type_union_resolution()` which uses `string_numeric_coercion()` (a numeric preferring) even for UNION, CASE THEN/ELSE
##########
datafusion/sqllogictest/test_files/string_numeric_coercion.slt:
##########
@@ -0,0 +1,496 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+##########
+## Tests for string-numeric comparison coercion
+## Verifies that when comparing a numeric column to a string literal,
+## the comparison is performed numerically (not lexicographically).
+## See: https://github.com/apache/datafusion/issues/15161
+##########
+
+# Setup test data
+statement ok
+CREATE TABLE t_int AS VALUES (1), (5), (325), (499), (1000);
+
+statement ok
+CREATE TABLE t_float AS VALUES (1.5), (5.0), (325.7), (499.9), (1000.1);
+
+# -------------------------------------------------
+# Integer column with comparison operators vs string literals.
+# Ensure that the comparison is done with numeric semantics,
+# not lexicographically.
+# -------------------------------------------------
+
+query I rowsort
+SELECT * FROM t_int WHERE column1 < '5';
+----
+1
+
+query I rowsort
+SELECT * FROM t_int WHERE column1 > '5';
+----
+1000
+325
+499
+
+query I rowsort
+SELECT * FROM t_int WHERE column1 <= '5';
+----
+1
+5
+
+query I rowsort
+SELECT * FROM t_int WHERE column1 >= '5';
+----
+1000
+325
+499
+5
+
+query I rowsort
+SELECT * FROM t_int WHERE column1 = '5';
+----
+5
+
+query I rowsort
+SELECT * FROM t_int WHERE column1 != '5';
+----
+1
+1000
+325
+499
+
+query I rowsort
+SELECT * FROM t_int WHERE column1 < '10';
+----
+1
+5
+
+query I rowsort
+SELECT * FROM t_int WHERE column1 <= '100';
+----
+1
+5
+
+query I rowsort
+SELECT * FROM t_int WHERE column1 > '100';
+----
+1000
+325
+499
+
+# -------------------------------------------------
+# Float column with comparison operators vs string literals
+# -------------------------------------------------
+
+query R rowsort
+SELECT * FROM t_float WHERE column1 < '5';
+----
+1.5
+
+query R rowsort
+SELECT * FROM t_float WHERE column1 > '5';
+----
+1000.1
+325.7
+499.9
+
+query R rowsort
+SELECT * FROM t_float WHERE column1 = '5';
+----
+5
+
+query R rowsort
+SELECT * FROM t_float WHERE column1 = '5.0';
+----
+5
+
+# -------------------------------------------------
+# Error on strings that cannot be cast to the numeric column type
+# -------------------------------------------------
+
+# Non-numeric string against integer column
+statement error Arrow error: Cast error: Cannot cast string 'hello' to value of Int64 type
+SELECT * FROM t_int WHERE column1 < 'hello';
+
+# Non-numeric string against float column
+statement error Arrow error: Cast error: Cannot cast string 'hello' to value of Float64 type
+SELECT * FROM t_float WHERE column1 < 'hello';
+
+# Float string against integer column
+statement error Arrow error: Cast error: Cannot cast string '99.99' to value of Int64 type
+SELECT * FROM t_int WHERE column1 = '99.99';
+
+# Empty string against integer column
+statement error Arrow error: Cast error: Cannot cast string '' to value of Int64 type
+SELECT * FROM t_int WHERE column1 = '';
+
+# Empty string against float column
+statement error Arrow error: Cast error: Cannot cast string '' to value of Float64 type
+SELECT * FROM t_float WHERE column1 = '';
+
+# Overflow
+statement error Arrow error: Cast error: Cannot cast string '99999999999999999999' to value of Int64 type
+SELECT * FROM t_int WHERE column1 = '99999999999999999999';
+
+
+# -------------------------------------------------
+# UNION still uses string coercion (type unification context)
+# -------------------------------------------------
+
+statement ok
+CREATE TABLE t_str AS VALUES ('one'), ('two'), ('three');
+
+query T rowsort
+SELECT column1 FROM t_int UNION ALL SELECT column1 FROM t_str;
+----
+1
+1000
+325
+499
+5
+one
+three
+two
+
+# Verify the UNION coerces to Utf8 (not numeric)
+query TT
+EXPLAIN SELECT column1 FROM t_int UNION ALL SELECT column1 FROM t_str;
+----
+logical_plan
+01)Union
+02)--Projection: CAST(t_int.column1 AS Utf8) AS column1
+03)----TableScan: t_int projection=[column1]
+04)--TableScan: t_str projection=[column1]
+physical_plan
+01)UnionExec
+02)--ProjectionExec: expr=[CAST(column1@0 AS Utf8) as column1]
+03)----DataSourceExec: partitions=1, partition_sizes=[1]
+04)--DataSourceExec: partitions=1, partition_sizes=[1]
+
+# -------------------------------------------------
+# BETWEEN uses comparison coercion (numeric preferred)
+# -------------------------------------------------
+
+query I rowsort
+SELECT * FROM t_int WHERE column1 BETWEEN '5' AND '100';
+----
+5
+
+# -------------------------------------------------
+# IN list uses comparison coercion (numeric preferred)
+# `x IN (a, b)` is semantically equivalent to `x = a OR x = b`
+# -------------------------------------------------
+
+# Basic IN list with string literals against integer column
+query I rowsort
+SELECT * FROM t_int WHERE column1 IN ('5', '325');
+----
+325
+5
+
+# IN list with a value where numeric coercion matters
+query I rowsort
+SELECT * FROM t_int WHERE column1 IN ('1000');
+----
+1000
+
+# IN list with NOT
+query I rowsort
+SELECT * FROM t_int WHERE column1 NOT IN ('1', '5');
+----
+1000
+325
+499
+
+# IN list with float column
+query R rowsort
+SELECT * FROM t_float WHERE column1 IN ('5.0', '325.7');
+----
+325.7
+5
+
+# Verify the plan shows numeric coercion (not CAST to Utf8)
+query TT
+EXPLAIN SELECT * FROM t_int WHERE column1 IN ('5', '325');
+----
+logical_plan
+01)Filter: t_int.column1 = Int64(5) OR t_int.column1 = Int64(325)
+02)--TableScan: t_int projection=[column1]
+physical_plan
+01)FilterExec: column1@0 = 5 OR column1@0 = 325
+02)--DataSourceExec: partitions=1, partition_sizes=[1]
+
+# Error on invalid string in IN list
+statement error Arrow error: Cast error: Cannot cast string 'hello' to value of Int64 type
+SELECT * FROM t_int WHERE column1 IN ('5', 'hello');
+
+# Mixed numeric literal and string literal in IN list against integer column
+query I rowsort
+SELECT * FROM t_int WHERE column1 IN (5, '325');
+----
+325
+5
+
+# Mixed numeric literal and string literal in IN list against float column
+query R rowsort
+SELECT * FROM t_float WHERE column1 IN (5, '325.7');
+----
+325.7
+5
+
+# String and numeric literal order reversed
+query R rowsort
+SELECT * FROM t_float WHERE column1 IN ('5', 325.7);
+----
+325.7
+5
+
+# Float string literal against integer column errors (cannot cast '10.0' to Int64)
+statement error Arrow error: Cast error: Cannot cast string '10.0' to value of Int64 type
+SELECT * FROM t_int WHERE column1 IN (5, '10.0');
+
+# Non-numeric string in mixed IN list still errors
+statement error Arrow error: Cast error: Cannot cast string 'hello' to value of Int64 type
+SELECT * FROM t_int WHERE column1 IN ('hello', 5);
+
+# -------------------------------------------------
+# CASE WHEN uses comparison coercion for conditions (numeric preferred)
+# `CASE expr WHEN val` is semantically equivalent to `expr = val`
+# -------------------------------------------------
+
+# Basic CASE with integer column and string WHEN values
+query T rowsort
+SELECT CASE column1 WHEN '5' THEN 'five' WHEN '1000' THEN 'thousand' ELSE 'other' END FROM t_int;
+----
+five
+other
+other
+other
+thousand
+
+# CASE with float column: '5' is cast to 5.0 numerically, matching the row.
+# (Under string comparison, '5.0' != '5' would fail to match.)
+query T rowsort
+SELECT CASE column1 WHEN '5' THEN 'matched' ELSE 'no match' END FROM t_float;
+----
+matched
+no match
+no match
+no match
+no match
+
+# THEN/ELSE results still use type union coercion (string preferred),
+# so mixing numeric and string coerces to string
+query T rowsort
+SELECT CASE WHEN column1 > 500 THEN column1 ELSE 'small' END FROM t_int;
+----
+1000
+small
+small
+small
+small
+
+# -------------------------------------------------
+# Nested struct/map/list comparisons use comparison coercion
+# (numeric preferred) for their field/element types
+# -------------------------------------------------
+
+statement ok
+CREATE TABLE t_struct_int AS SELECT named_struct('val', column1) as s FROM (VALUES (1), (5), (10));
+
+statement ok
+CREATE TABLE t_struct_str AS SELECT named_struct('val', column1) as s FROM (VALUES ('5'), ('10'));
+
+# Struct comparison: the string field is cast to Int64 (numeric preferred).
+query ? rowsort
+SELECT t1.s FROM t_struct_int t1, t_struct_str t2 WHERE t1.s = t2.s;
+----
+{val: 10}
+{val: 5}
+
+# Struct in UNION uses type union coercion (string preferred).
+# The integer struct field is cast to Utf8.
+query ? rowsort
+SELECT s FROM t_struct_int UNION ALL SELECT s FROM t_struct_str;
+----
+{val: 10}
+{val: 10}
+{val: 1}
+{val: 5}
+{val: 5}
+
+statement ok
+DROP TABLE t_struct_int;
+
+statement ok
+DROP TABLE t_struct_str;
+
+# List comparison: string elements are cast to Int64 (numeric preferred).
+statement ok
+CREATE TABLE t_list_int AS SELECT column1 as l FROM (VALUES ([1, 5, 10]), ([20, 30]));
+
+statement ok
+CREATE TABLE t_list_str AS SELECT column1 as l FROM (VALUES (['5', '10']), (['20', '30']));
+
+# Verify the element types are Int64 and Utf8 respectively
+query T
+SELECT arrow_typeof(l) FROM t_list_int LIMIT 1;
+----
+List(Int64)
+
+query T
+SELECT arrow_typeof(l) FROM t_list_str LIMIT 1;
+----
+List(Utf8)
+
+query ? rowsort
+SELECT t1.l FROM t_list_int t1, t_list_str t2 WHERE t1.l = t2.l;
+----
+[20, 30]
+
+# List in UNION uses type union coercion (string preferred).
+# The integer list elements are cast to Utf8.
+query ? rowsort
+SELECT l FROM t_list_int UNION ALL SELECT l FROM t_list_str;
+----
+[1, 5, 10]
+[20, 30]
+[20, 30]
+[5, 10]
+
+statement ok
+DROP TABLE t_list_int;
+
+statement ok
+DROP TABLE t_list_str;
+
+# Map comparison: string values are cast to Int64 (numeric preferred).
+statement ok
+CREATE TABLE t_map_int AS SELECT MAP {'a': 1, 'b': 5} as m;
+
+statement ok
+CREATE TABLE t_map_str AS SELECT MAP {'a': '1', 'b': '5'} as m;
+
+# Verify the value types are Int64 and Utf8 respectively
+query T
+SELECT arrow_typeof(m) FROM t_map_int LIMIT 1;
+----
+Map("entries": non-null Struct("key": non-null Utf8, "value": Int64), unsorted)
+
+query T
+SELECT arrow_typeof(m) FROM t_map_str LIMIT 1;
+----
+Map("entries": non-null Struct("key": non-null Utf8, "value": Utf8), unsorted)
+
+query ? rowsort
+SELECT t1.m FROM t_map_int t1, t_map_str t2 WHERE t1.m = t2.m;
+----
+{a: 1, b: 5}
+
+# Map in UNION uses type union coercion (string preferred).
+# The integer map values are cast to Utf8.
+query ? rowsort
+SELECT m FROM t_map_int UNION ALL SELECT m FROM t_map_str;
+----
+{a: 1, b: 5}
+{a: 1, b: 5}
+
+statement ok
+DROP TABLE t_map_int;
+
+statement ok
+DROP TABLE t_map_str;
+
+# -------------------------------------------------
+# LIKE / regex on dictionary-encoded numeric columns should error,
+# consistent with LIKE on plain numeric columns
+# -------------------------------------------------
+
+# Plain integer column: LIKE is not supported
+statement error There isn't a common type to coerce Int64 and Utf8 in LIKE expression
+SELECT * FROM t_int WHERE column1 LIKE '%5%';
+
+# Dictionary-encoded integer column: should also error
+statement error There isn't a common type to coerce Dictionary\(Int32, Int64\) and Utf8 in LIKE expression
+SELECT arrow_cast(column1, 'Dictionary(Int32, Int64)') LIKE '%5%' FROM t_int;
+
+# Dictionary-encoded string column: LIKE works as normal
+query B rowsort
+SELECT arrow_cast('hello', 'Dictionary(Int32, Utf8)') LIKE '%ell%';
+----
+true
+
+# REE-encoded integer column: LIKE should also error
+statement error There isn't a common type to coerce RunEndEncoded.* and Utf8 in LIKE expression
+SELECT arrow_cast(column1, 'RunEndEncoded("run_ends": non-null Int32, "values": Int64)') LIKE '%5%' FROM t_int;
+
+# REE-encoded string column: LIKE works as normal
+query B rowsort
+SELECT arrow_cast('hello', 'RunEndEncoded("run_ends": non-null Int32, "values": Utf8)') LIKE '%ell%';
+----
+true
+
+# Dictionary-encoded integer column: regex should error
+statement error Cannot infer common argument type for regex operation
+SELECT arrow_cast(column1, 'Dictionary(Int32, Int64)') ~ '5' FROM t_int;
+
+# Dictionary-encoded string column: regex works as normal
+query B rowsort
+SELECT arrow_cast('hello', 'Dictionary(Int32, Utf8)') ~ 'ell';
+----
+true
+
+# REE-encoded integer column: regex should error
+statement error Cannot infer common argument type for regex operation
+SELECT arrow_cast(column1, 'RunEndEncoded("run_ends": non-null Int32, "values": Int64)') ~ '5' FROM t_int;
+
+# REE-encoded string column: regex works as normal
+query B rowsort
+SELECT arrow_cast('hello', 'RunEndEncoded("run_ends": non-null Int32, "values": Utf8)') ~ 'ell';
+----
+true
+
+# -------------------------------------------------
+# Cleanup
+# -------------------------------------------------
+
+statement ok
+DROP TABLE t_int;
+
+statement ok
+DROP TABLE t_float;
+
+statement ok
+DROP TABLE t_str;
+
+# -------------------------------------------------
+# List element coercion should reject mixed
+# numeric/string categories (same as array literals)
+# -------------------------------------------------
+
+# Array literal with mixed numeric/string elements errors
+query error Cannot cast string 'a' to value of Int64 type
+SELECT [1, 'a'];
+
+# MAP with mixed-category list keys should also error
+query error
+SELECT MAP {[1,2,3]:1, ['a', 'b']:2};
+
+# MAP with mixed-category list values should also error
+query error
Review Comment:
and here
##########
datafusion/sqllogictest/test_files/string_numeric_coercion.slt:
##########
@@ -0,0 +1,496 @@
+# Licensed to the Apache Software Foundation (ASF) under one
+# or more contributor license agreements. See the NOTICE file
+# distributed with this work for additional information
+# regarding copyright ownership. The ASF licenses this file
+# to you under the Apache License, Version 2.0 (the
+# "License"); you may not use this file except in compliance
+# with the License. You may obtain a copy of the License at
+#
+# http://www.apache.org/licenses/LICENSE-2.0
+#
+# Unless required by applicable law or agreed to in writing,
+# software distributed under the License is distributed on an
+# "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+# KIND, either express or implied. See the License for the
+# specific language governing permissions and limitations
+# under the License.
+
+##########
+## Tests for string-numeric comparison coercion
+## Verifies that when comparing a numeric column to a string literal,
+## the comparison is performed numerically (not lexicographically).
+## See: https://github.com/apache/datafusion/issues/15161
+##########
+
+# Setup test data
+statement ok
+CREATE TABLE t_int AS VALUES (1), (5), (325), (499), (1000);
+
+statement ok
+CREATE TABLE t_float AS VALUES (1.5), (5.0), (325.7), (499.9), (1000.1);
+
+# -------------------------------------------------
+# Integer column with comparison operators vs string literals.
+# Ensure that the comparison is done with numeric semantics,
+# not lexicographically.
+# -------------------------------------------------
+
+query I rowsort
+SELECT * FROM t_int WHERE column1 < '5';
+----
+1
+
+query I rowsort
+SELECT * FROM t_int WHERE column1 > '5';
+----
+1000
+325
+499
+
+query I rowsort
+SELECT * FROM t_int WHERE column1 <= '5';
+----
+1
+5
+
+query I rowsort
+SELECT * FROM t_int WHERE column1 >= '5';
+----
+1000
+325
+499
+5
+
+query I rowsort
+SELECT * FROM t_int WHERE column1 = '5';
+----
+5
+
+query I rowsort
+SELECT * FROM t_int WHERE column1 != '5';
+----
+1
+1000
+325
+499
+
+query I rowsort
+SELECT * FROM t_int WHERE column1 < '10';
+----
+1
+5
+
+query I rowsort
+SELECT * FROM t_int WHERE column1 <= '100';
+----
+1
+5
+
+query I rowsort
+SELECT * FROM t_int WHERE column1 > '100';
+----
+1000
+325
+499
+
+# -------------------------------------------------
+# Float column with comparison operators vs string literals
+# -------------------------------------------------
+
+query R rowsort
+SELECT * FROM t_float WHERE column1 < '5';
+----
+1.5
+
+query R rowsort
+SELECT * FROM t_float WHERE column1 > '5';
+----
+1000.1
+325.7
+499.9
+
+query R rowsort
+SELECT * FROM t_float WHERE column1 = '5';
+----
+5
+
+query R rowsort
+SELECT * FROM t_float WHERE column1 = '5.0';
+----
+5
+
+# -------------------------------------------------
+# Error on strings that cannot be cast to the numeric column type
+# -------------------------------------------------
+
+# Non-numeric string against integer column
+statement error Arrow error: Cast error: Cannot cast string 'hello' to value of Int64 type
+SELECT * FROM t_int WHERE column1 < 'hello';
+
+# Non-numeric string against float column
+statement error Arrow error: Cast error: Cannot cast string 'hello' to value of Float64 type
+SELECT * FROM t_float WHERE column1 < 'hello';
+
+# Float string against integer column
+statement error Arrow error: Cast error: Cannot cast string '99.99' to value of Int64 type
+SELECT * FROM t_int WHERE column1 = '99.99';
+
+# Empty string against integer column
+statement error Arrow error: Cast error: Cannot cast string '' to value of Int64 type
+SELECT * FROM t_int WHERE column1 = '';
+
+# Empty string against float column
+statement error Arrow error: Cast error: Cannot cast string '' to value of Float64 type
+SELECT * FROM t_float WHERE column1 = '';
+
+# Overflow
+statement error Arrow error: Cast error: Cannot cast string '99999999999999999999' to value of Int64 type
+SELECT * FROM t_int WHERE column1 = '99999999999999999999';
+
+
+# -------------------------------------------------
+# UNION still uses string coercion (type unification context)
+# -------------------------------------------------
+
+statement ok
+CREATE TABLE t_str AS VALUES ('one'), ('two'), ('three');
+
+query T rowsort
+SELECT column1 FROM t_int UNION ALL SELECT column1 FROM t_str;
+----
+1
+1000
+325
+499
+5
+one
+three
+two
+
+# Verify the UNION coerces to Utf8 (not numeric)
+query TT
+EXPLAIN SELECT column1 FROM t_int UNION ALL SELECT column1 FROM t_str;
+----
+logical_plan
+01)Union
+02)--Projection: CAST(t_int.column1 AS Utf8) AS column1
+03)----TableScan: t_int projection=[column1]
+04)--TableScan: t_str projection=[column1]
+physical_plan
+01)UnionExec
+02)--ProjectionExec: expr=[CAST(column1@0 AS Utf8) as column1]
+03)----DataSourceExec: partitions=1, partition_sizes=[1]
+04)--DataSourceExec: partitions=1, partition_sizes=[1]
+
+# -------------------------------------------------
+# BETWEEN uses comparison coercion (numeric preferred)
+# -------------------------------------------------
+
+query I rowsort
+SELECT * FROM t_int WHERE column1 BETWEEN '5' AND '100';
+----
+5
+
+# -------------------------------------------------
+# IN list uses comparison coercion (numeric preferred)
+# `x IN (a, b)` is semantically equivalent to `x = a OR x = b`
+# -------------------------------------------------
+
+# Basic IN list with string literals against integer column
+query I rowsort
+SELECT * FROM t_int WHERE column1 IN ('5', '325');
+----
+325
+5
+
+# IN list with a value where numeric coercion matters
+query I rowsort
+SELECT * FROM t_int WHERE column1 IN ('1000');
+----
+1000
+
+# IN list with NOT
+query I rowsort
+SELECT * FROM t_int WHERE column1 NOT IN ('1', '5');
+----
+1000
+325
+499
+
+# IN list with float column
+query R rowsort
+SELECT * FROM t_float WHERE column1 IN ('5.0', '325.7');
+----
+325.7
+5
+
+# Verify the plan shows numeric coercion (not CAST to Utf8)
+query TT
+EXPLAIN SELECT * FROM t_int WHERE column1 IN ('5', '325');
+----
+logical_plan
+01)Filter: t_int.column1 = Int64(5) OR t_int.column1 = Int64(325)
+02)--TableScan: t_int projection=[column1]
+physical_plan
+01)FilterExec: column1@0 = 5 OR column1@0 = 325
+02)--DataSourceExec: partitions=1, partition_sizes=[1]
+
+# Error on invalid string in IN list
+statement error Arrow error: Cast error: Cannot cast string 'hello' to value of Int64 type
+SELECT * FROM t_int WHERE column1 IN ('5', 'hello');
+
+# Mixed numeric literal and string literal in IN list against integer column
+query I rowsort
+SELECT * FROM t_int WHERE column1 IN (5, '325');
+----
+325
+5
+
+# Mixed numeric literal and string literal in IN list against float column
+query R rowsort
+SELECT * FROM t_float WHERE column1 IN (5, '325.7');
+----
+325.7
+5
+
+# String and numeric literal order reversed
+query R rowsort
+SELECT * FROM t_float WHERE column1 IN ('5', 325.7);
+----
+325.7
+5
+
+# Float string literal against integer column errors (cannot cast '10.0' to Int64)
+statement error Arrow error: Cast error: Cannot cast string '10.0' to value of Int64 type
+SELECT * FROM t_int WHERE column1 IN (5, '10.0');
+
+# Non-numeric string in mixed IN list still errors
+statement error Arrow error: Cast error: Cannot cast string 'hello' to value of Int64 type
+SELECT * FROM t_int WHERE column1 IN ('hello', 5);
+
+# -------------------------------------------------
+# CASE WHEN uses comparison coercion for conditions (numeric preferred)
+# `CASE expr WHEN val` is semantically equivalent to `expr = val`
+# -------------------------------------------------
+
+# Basic CASE with integer column and string WHEN values
+query T rowsort
+SELECT CASE column1 WHEN '5' THEN 'five' WHEN '1000' THEN 'thousand' ELSE 'other' END FROM t_int;
+----
+five
+other
+other
+other
+thousand
+
+# CASE with float column: '5' is cast to 5.0 numerically, matching the row.
+# (Under string comparison, '5.0' != '5' would fail to match.)
+query T rowsort
+SELECT CASE column1 WHEN '5' THEN 'matched' ELSE 'no match' END FROM t_float;
+----
+matched
+no match
+no match
+no match
+no match
+
+# THEN/ELSE results still use type union coercion (string preferred),
+# so mixing numeric and string coerces to string
+query T rowsort
+SELECT CASE WHEN column1 > 500 THEN column1 ELSE 'small' END FROM t_int;
+----
+1000
+small
+small
+small
+small
+
+# -------------------------------------------------
+# Nested struct/map/list comparisons use comparison coercion
+# (numeric preferred) for their field/element types
+# -------------------------------------------------
+
+statement ok
+CREATE TABLE t_struct_int AS SELECT named_struct('val', column1) as s FROM (VALUES (1), (5), (10));
+
+statement ok
+CREATE TABLE t_struct_str AS SELECT named_struct('val', column1) as s FROM (VALUES ('5'), ('10'));
+
+# Struct comparison: the string field is cast to Int64 (numeric preferred).
+query ? rowsort
+SELECT t1.s FROM t_struct_int t1, t_struct_str t2 WHERE t1.s = t2.s;
+----
+{val: 10}
+{val: 5}
+
+# Struct in UNION uses type union coercion (string preferred).
+# The integer struct field is cast to Utf8.
+query ? rowsort
+SELECT s FROM t_struct_int UNION ALL SELECT s FROM t_struct_str;
+----
+{val: 10}
+{val: 10}
+{val: 1}
+{val: 5}
+{val: 5}
+
+statement ok
+DROP TABLE t_struct_int;
+
+statement ok
+DROP TABLE t_struct_str;
+
+# List comparison: string elements are cast to Int64 (numeric preferred).
+statement ok
+CREATE TABLE t_list_int AS SELECT column1 as l FROM (VALUES ([1, 5, 10]), ([20, 30]));
+
+statement ok
+CREATE TABLE t_list_str AS SELECT column1 as l FROM (VALUES (['5', '10']), (['20', '30']));
+
+# Verify the element types are Int64 and Utf8 respectively
+query T
+SELECT arrow_typeof(l) FROM t_list_int LIMIT 1;
+----
+List(Int64)
+
+query T
+SELECT arrow_typeof(l) FROM t_list_str LIMIT 1;
+----
+List(Utf8)
+
+query ? rowsort
+SELECT t1.l FROM t_list_int t1, t_list_str t2 WHERE t1.l = t2.l;
+----
+[20, 30]
+
+# List in UNION uses type union coercion (string preferred).
+# The integer list elements are cast to Utf8.
+query ? rowsort
+SELECT l FROM t_list_int UNION ALL SELECT l FROM t_list_str;
+----
+[1, 5, 10]
+[20, 30]
+[20, 30]
+[5, 10]
+
+statement ok
+DROP TABLE t_list_int;
+
+statement ok
+DROP TABLE t_list_str;
+
+# Map comparison: string values are cast to Int64 (numeric preferred).
+statement ok
+CREATE TABLE t_map_int AS SELECT MAP {'a': 1, 'b': 5} as m;
+
+statement ok
+CREATE TABLE t_map_str AS SELECT MAP {'a': '1', 'b': '5'} as m;
+
+# Verify the value types are Int64 and Utf8 respectively
+query T
+SELECT arrow_typeof(m) FROM t_map_int LIMIT 1;
+----
+Map("entries": non-null Struct("key": non-null Utf8, "value": Int64), unsorted)
+
+query T
+SELECT arrow_typeof(m) FROM t_map_str LIMIT 1;
+----
+Map("entries": non-null Struct("key": non-null Utf8, "value": Utf8), unsorted)
+
+query ? rowsort
+SELECT t1.m FROM t_map_int t1, t_map_str t2 WHERE t1.m = t2.m;
+----
+{a: 1, b: 5}
+
+# Map in UNION uses type union coercion (string preferred).
+# The integer map values are cast to Utf8.
+query ? rowsort
+SELECT m FROM t_map_int UNION ALL SELECT m FROM t_map_str;
+----
+{a: 1, b: 5}
+{a: 1, b: 5}
+
+statement ok
+DROP TABLE t_map_int;
+
+statement ok
+DROP TABLE t_map_str;
+
+# -------------------------------------------------
+# LIKE / regex on dictionary-encoded numeric columns should error,
+# consistent with LIKE on plain numeric columns
+# -------------------------------------------------
+
+# Plain integer column: LIKE is not supported
+statement error There isn't a common type to coerce Int64 and Utf8 in LIKE expression
+SELECT * FROM t_int WHERE column1 LIKE '%5%';
+
+# Dictionary-encoded integer column: should also error
+statement error There isn't a common type to coerce Dictionary\(Int32, Int64\) and Utf8 in LIKE expression
+SELECT arrow_cast(column1, 'Dictionary(Int32, Int64)') LIKE '%5%' FROM t_int;
+
+# Dictionary-encoded string column: LIKE works as normal
+query B rowsort
+SELECT arrow_cast('hello', 'Dictionary(Int32, Utf8)') LIKE '%ell%';
+----
+true
+
+# REE-encoded integer column: LIKE should also error
+statement error There isn't a common type to coerce RunEndEncoded.* and Utf8 in LIKE expression
+SELECT arrow_cast(column1, 'RunEndEncoded("run_ends": non-null Int32, "values": Int64)') LIKE '%5%' FROM t_int;
+
+# REE-encoded string column: LIKE works as normal
+query B rowsort
+SELECT arrow_cast('hello', 'RunEndEncoded("run_ends": non-null Int32, "values": Utf8)') LIKE '%ell%';
+----
+true
+
+# Dictionary-encoded integer column: regex should error
+statement error Cannot infer common argument type for regex operation
+SELECT arrow_cast(column1, 'Dictionary(Int32, Int64)') ~ '5' FROM t_int;
+
+# Dictionary-encoded string column: regex works as normal
+query B rowsort
+SELECT arrow_cast('hello', 'Dictionary(Int32, Utf8)') ~ 'ell';
+----
+true
+
+# REE-encoded integer column: regex should error
+statement error Cannot infer common argument type for regex operation
+SELECT arrow_cast(column1, 'RunEndEncoded("run_ends": non-null Int32, "values": Int64)') ~ '5' FROM t_int;
+
+# REE-encoded string column: regex works as normal
+query B rowsort
+SELECT arrow_cast('hello', 'RunEndEncoded("run_ends": non-null Int32, "values": Utf8)') ~ 'ell';
+----
+true
+
+# -------------------------------------------------
+# Cleanup
+# -------------------------------------------------
+
+statement ok
+DROP TABLE t_int;
+
+statement ok
+DROP TABLE t_float;
+
+statement ok
+DROP TABLE t_str;
+
+# -------------------------------------------------
+# List element coercion should reject mixed
+# numeric/string categories (same as array literals)
+# -------------------------------------------------
+
+# Array literal with mixed numeric/string elements errors
+query error Cannot cast string 'a' to value of Int64 type
+SELECT [1, 'a'];
+
+# MAP with mixed-category list keys should also error
+query error
Review Comment:
Why there is no expected error message here ?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: Prefer numeric in type coercion for comparisons [datafusion]
Posted by "adriangb (via GitHub)" <gi...@apache.org>.
adriangb commented on PR #20426:
URL: https://github.com/apache/datafusion/pull/20426#issuecomment-4127017141
I have not heard any push back or alternative proposals in [Discord](https://discord.com/channels/885562378132000778/1166447479609376850/1474493692118302934) or the [mailing list](https://lists.apache.org/thread/5od7k7hmlzfv36p7jco65x29755xlkt8).
From my perspective this is good to go. Let's give Andrew and Martin a day or so to chime in, if there's no feedback to address by then we can go ahead and merge this. Thanks for your work @neilconway.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: Prefer numeric in type coercion for comparisons [datafusion]
Posted by "AlonSpivack (via GitHub)" <gi...@apache.org>.
AlonSpivack commented on PR #20426:
URL: https://github.com/apache/datafusion/pull/20426#issuecomment-3928006537
Hey @neilconway ,
thanks for moving so quickly on this. Splitting the coercion rules between comparison_coercion and type_union_coercion is definitely the right architectural foundation, and it aligns with what @alamb suggested previously about context-aware coercion.
Regarding the bad query plans you mentioned (like CAST(text_col AS Int64) < 5) and the ClickBench issues:
I think we should tackle this in two separate PRs to respect the project's architecture:
PR 1 (This PR - Type Level): Your current approach (coercing string→numeric for comparisons) is correct at the type-abstraction level. Generating a CAST here is the right logical step to ensure mathematical correctness. As @alamb noted in the past (e.g., on PR #15482), type coercion rules should be based strictly on (DataType, DataType) pairs, not on checking for Expr::Literal. So mixing literal evaluation into this specific phase would be architecturally incorrect.
PR 2 (Follow-up - Optimizer Level): Once this PR lands, I'd like to submit a follow-up PR targeting the simplifier (specifically extending cast_literal_to_type_with_op in unwrap_cast.rs). Since the simplifier is expected to be expression-aware, we can safely intercept the CAST there:
Early Literal Folding: Fold Literal(Utf8("10")) → Literal(Int64(10)) at planning time when compared against a numeric column. This gives zero runtime cost and restores performance/predicate pushdown.
Fail-Fast for Invalid Literals: Throw an instant plan_err! for unparseable literals like int_col < 'hello' instead of deferring to a runtime CAST failure.
Expanded Coverage: Extend this logic beyond =/!= to handle <, >, <=, >=, and include Float types.
This two-phase approach gets the critical bug (#15161) fixed quickly, keeps the type coercion pure, and handles the performance/AST-awareness in the correct optimizer pass.
Happy to help with the follow-up PR. Thanks again for driving this fix!
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: Prefer numeric in type coercion for comparisons [datafusion]
Posted by "neilconway (via GitHub)" <gi...@apache.org>.
neilconway commented on PR #20426:
URL: https://github.com/apache/datafusion/pull/20426#issuecomment-3936851610
@AlonSpivack I wrote up an alternative approach to fixing it, and some comments back on the [issue](https://github.com/apache/datafusion/issues/15161#issuecomment-3936788737); it might be better to continue further discussion there since there's two PRs now.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: Prefer numeric in type coercion for comparisons [datafusion]
Posted by "AdamGS (via GitHub)" <gi...@apache.org>.
AdamGS commented on PR #20426:
URL: https://github.com/apache/datafusion/pull/20426#issuecomment-4025211765
@adriangb this seems reasonable, but so far we've tried to not do any type coercion internally, we've already had some issues with decimals, I suspect this might trigger more of these cases, and we'll need to do some rewriting to match the types.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: Prefer numeric in type coercion for comparisons [datafusion]
Posted by "adriangb (via GitHub)" <gi...@apache.org>.
adriangb commented on code in PR #20426:
URL: https://github.com/apache/datafusion/pull/20426#discussion_r2906316372
##########
datafusion/core/src/physical_planner.rs:
##########
@@ -3436,16 +3436,16 @@ mod tests {
#[tokio::test]
async fn in_list_types() -> Result<()> {
- // expression: "a in ('a', 1)"
- let list = vec![lit("a"), lit(1i64)];
Review Comment:
I guess the point of this change is that `a in ('a', 1)` now is an error? It would be great to move that to another test that explicitly checks the error, if it is plan time or execution time, etc.
##########
datafusion/expr-common/src/type_coercion/binary.rs:
##########
@@ -843,102 +842,100 @@ pub fn try_type_union_resolution_with_struct(
Ok(final_struct_types)
}
-/// Coerce `lhs_type` and `rhs_type` to a common type for the purposes of a
-/// comparison operation
+/// Coerce `lhs_type` and `rhs_type` to a common type for type unification
+/// contexts — where two values must be brought to a common type but are not
+/// being compared. Examples: UNION, CASE THEN/ELSE branches, NVL2.
///
-/// Example comparison operations are `lhs = rhs` and `lhs > rhs`
+/// Prefers strings over numeric types (e.g., `SELECT 1 UNION SELECT '2'`
+/// coerces both sides to `Utf8`).
///
-/// Binary comparison kernels require the two arguments to be the (exact) same
-/// data type. However, users can write queries where the two arguments are
-/// different data types. In such cases, the data types are automatically cast
-/// (coerced) to a single data type to pass to the kernels.
-///
-/// # Numeric comparisons
-///
-/// When comparing numeric values, the lower precision type is coerced to the
-/// higher precision type to avoid losing data. For example when comparing
-/// `Int32` to `Int64` the coerced type is `Int64` so the `Int32` argument will
-/// be cast.
-///
-/// # Numeric / String comparisons
-///
-/// When comparing numeric values and strings, both values will be coerced to
-/// strings. For example when comparing `'2' > 1`, the arguments will be
-/// coerced to `Utf8` for comparison
-pub fn comparison_coercion(lhs_type: &DataType, rhs_type: &DataType) -> Option<DataType> {
+/// For comparisons (`=`, `<`, `>`), use [`comparison_coercion`] instead.
+pub fn type_union_coercion(lhs_type: &DataType, rhs_type: &DataType) -> Option<DataType> {
if lhs_type.equals_datatype(rhs_type) {
- // same type => equality is possible
return Some(lhs_type.clone());
}
binary_numeric_coercion(lhs_type, rhs_type)
- .or_else(|| dictionary_comparison_coercion(lhs_type, rhs_type, true))
- .or_else(|| ree_comparison_coercion(lhs_type, rhs_type, true))
+ .or_else(|| dictionary_coercion(lhs_type, rhs_type, true, type_union_coercion))
+ .or_else(|| ree_coercion(lhs_type, rhs_type, true, type_union_coercion))
.or_else(|| temporal_coercion_nonstrict_timezone(lhs_type, rhs_type))
.or_else(|| string_coercion(lhs_type, rhs_type))
.or_else(|| list_coercion(lhs_type, rhs_type))
.or_else(|| null_coercion(lhs_type, rhs_type))
- .or_else(|| string_numeric_coercion(lhs_type, rhs_type))
+ .or_else(|| string_numeric_union_coercion(lhs_type, rhs_type))
.or_else(|| string_temporal_coercion(lhs_type, rhs_type))
.or_else(|| binary_coercion(lhs_type, rhs_type))
- .or_else(|| struct_coercion(lhs_type, rhs_type))
- .or_else(|| map_coercion(lhs_type, rhs_type))
+ .or_else(|| struct_coercion(lhs_type, rhs_type, type_union_coercion))
+ .or_else(|| map_coercion(lhs_type, rhs_type, type_union_coercion))
}
-/// Similar to [`comparison_coercion`] but prefers numeric if compares with
-/// numeric and string
+/// Coerce `lhs_type` and `rhs_type` to a common type for comparison
+/// contexts — any context where two values are compared rather than
+/// unified. This includes binary comparison operators, IN lists,
+/// CASE/WHEN conditions, and BETWEEN.
+///
+/// When the two types differ, this function determines the common type
+/// to cast to.
///
/// # Numeric comparisons
///
-/// When comparing numeric values and strings, the values will be coerced to the
-/// numeric type. For example, `'2' > 1` if `1` is an `Int32`, the arguments
-/// will be coerced to `Int32`.
-pub fn comparison_coercion_numeric(
- lhs_type: &DataType,
- rhs_type: &DataType,
-) -> Option<DataType> {
- if lhs_type == rhs_type {
+/// The lower precision type is widened to the higher precision type
+/// (e.g., `Int32` vs `Int64` → `Int64`).
+///
+/// # Numeric / String comparisons
+///
+/// Prefers the numeric type (e.g., `'2' > 1` where `1` is `Int32` coerces
+/// `'2'` to `Int32`).
+///
+/// For type unification contexts (UNION, CASE THEN/ELSE), use
+/// [`type_union_coercion`] instead.
Review Comment:
Worth including a bit more here, e.g. rationale, why we have different behavior (matches Postgres, etc?)
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: Prefer numeric in type coercion for comparisons [datafusion]
Posted by "neilconway (via GitHub)" <gi...@apache.org>.
neilconway commented on code in PR #20426:
URL: https://github.com/apache/datafusion/pull/20426#discussion_r2908222551
##########
datafusion/expr-common/src/type_coercion/binary.rs:
##########
@@ -843,102 +842,100 @@ pub fn try_type_union_resolution_with_struct(
Ok(final_struct_types)
}
-/// Coerce `lhs_type` and `rhs_type` to a common type for the purposes of a
-/// comparison operation
+/// Coerce `lhs_type` and `rhs_type` to a common type for type unification
+/// contexts — where two values must be brought to a common type but are not
+/// being compared. Examples: UNION, CASE THEN/ELSE branches, NVL2.
///
-/// Example comparison operations are `lhs = rhs` and `lhs > rhs`
+/// Prefers strings over numeric types (e.g., `SELECT 1 UNION SELECT '2'`
+/// coerces both sides to `Utf8`).
///
-/// Binary comparison kernels require the two arguments to be the (exact) same
-/// data type. However, users can write queries where the two arguments are
-/// different data types. In such cases, the data types are automatically cast
-/// (coerced) to a single data type to pass to the kernels.
-///
-/// # Numeric comparisons
-///
-/// When comparing numeric values, the lower precision type is coerced to the
-/// higher precision type to avoid losing data. For example when comparing
-/// `Int32` to `Int64` the coerced type is `Int64` so the `Int32` argument will
-/// be cast.
-///
-/// # Numeric / String comparisons
-///
-/// When comparing numeric values and strings, both values will be coerced to
-/// strings. For example when comparing `'2' > 1`, the arguments will be
-/// coerced to `Utf8` for comparison
-pub fn comparison_coercion(lhs_type: &DataType, rhs_type: &DataType) -> Option<DataType> {
+/// For comparisons (`=`, `<`, `>`), use [`comparison_coercion`] instead.
+pub fn type_union_coercion(lhs_type: &DataType, rhs_type: &DataType) -> Option<DataType> {
if lhs_type.equals_datatype(rhs_type) {
- // same type => equality is possible
return Some(lhs_type.clone());
}
binary_numeric_coercion(lhs_type, rhs_type)
- .or_else(|| dictionary_comparison_coercion(lhs_type, rhs_type, true))
- .or_else(|| ree_comparison_coercion(lhs_type, rhs_type, true))
+ .or_else(|| dictionary_coercion(lhs_type, rhs_type, true, type_union_coercion))
+ .or_else(|| ree_coercion(lhs_type, rhs_type, true, type_union_coercion))
.or_else(|| temporal_coercion_nonstrict_timezone(lhs_type, rhs_type))
.or_else(|| string_coercion(lhs_type, rhs_type))
.or_else(|| list_coercion(lhs_type, rhs_type))
.or_else(|| null_coercion(lhs_type, rhs_type))
- .or_else(|| string_numeric_coercion(lhs_type, rhs_type))
+ .or_else(|| string_numeric_union_coercion(lhs_type, rhs_type))
.or_else(|| string_temporal_coercion(lhs_type, rhs_type))
.or_else(|| binary_coercion(lhs_type, rhs_type))
- .or_else(|| struct_coercion(lhs_type, rhs_type))
- .or_else(|| map_coercion(lhs_type, rhs_type))
+ .or_else(|| struct_coercion(lhs_type, rhs_type, type_union_coercion))
+ .or_else(|| map_coercion(lhs_type, rhs_type, type_union_coercion))
}
-/// Similar to [`comparison_coercion`] but prefers numeric if compares with
-/// numeric and string
+/// Coerce `lhs_type` and `rhs_type` to a common type for comparison
+/// contexts — any context where two values are compared rather than
+/// unified. This includes binary comparison operators, IN lists,
+/// CASE/WHEN conditions, and BETWEEN.
+///
+/// When the two types differ, this function determines the common type
+/// to cast to.
///
/// # Numeric comparisons
///
-/// When comparing numeric values and strings, the values will be coerced to the
-/// numeric type. For example, `'2' > 1` if `1` is an `Int32`, the arguments
-/// will be coerced to `Int32`.
-pub fn comparison_coercion_numeric(
- lhs_type: &DataType,
- rhs_type: &DataType,
-) -> Option<DataType> {
- if lhs_type == rhs_type {
+/// The lower precision type is widened to the higher precision type
+/// (e.g., `Int32` vs `Int64` → `Int64`).
+///
+/// # Numeric / String comparisons
+///
+/// Prefers the numeric type (e.g., `'2' > 1` where `1` is `Int32` coerces
+/// `'2'` to `Int32`).
+///
+/// For type unification contexts (UNION, CASE THEN/ELSE), use
+/// [`type_union_coercion`] instead.
Review Comment:
Done, although I added the comment to `type_union_coercion` instead.
FWIW our behavior here does not match Postgres; for example, Postgres rejects queries like "SELECT 1 UNION SELECT 'a'", which we allow. I started going down the path of rejecting those queries in #20456, but that will break more user code, so I figured it would be best to defer for now. Another construct we allow that Postgres doesn't is `text_col <> int_col` -- again, I'd argue that is probably better to reject, but I didn't take that on as part of this PR.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: Prefer numeric in type coercion for comparisons [datafusion]
Posted by "adriangb (via GitHub)" <gi...@apache.org>.
adriangb commented on PR #20426:
URL: https://github.com/apache/datafusion/pull/20426#issuecomment-4026458043
> I'm not sure if i can give meaningful contribution in this case, ballista is relaying on datafusion for this behaviour.
>
> would we need to capture change of behaviour somewhere, like upgrade guide ?
Yes I think we should make an upgrading guide entry @neilconway.
If Ballista just uses DataFusion semantics and you are okay with this change then nothing to do on your end 😃
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: Prefer numeric in type coercion for comparisons [datafusion]
Posted by "neilconway (via GitHub)" <gi...@apache.org>.
neilconway commented on PR #20426:
URL: https://github.com/apache/datafusion/pull/20426#issuecomment-4126881239
@alamb @adriangb @martin-g I wanted to check in on this -- is there anything else that we need to do here before landing this?
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org
Re: [PR] fix: Prefer numeric in type coercion for comparisons [datafusion]
Posted by "neilconway (via GitHub)" <gi...@apache.org>.
neilconway commented on PR #20426:
URL: https://github.com/apache/datafusion/pull/20426#issuecomment-3926891792
@AlonSpivack The challenge with rejecting `text_col < 5` under this approach is that in `comparison_coercion`, we've already lost the fact that we're comparing a column with a literal. All we see is a comparison between a string-typed value and a numeric-typed value, which this approach would allow.
If we want to reject `text_col < 5` and similar expressions, I think we need to remove the `comparison_coercion` logic for coercing string -> numeric and the like. We'd then have two options:
1. Introduce an "unknown" type. All string literals would have unknown type, and then would be coerced into a concrete type depending on the context in which they are used. This is roughly what Postgres does.
2. Don't introduce a distinct type, but just add some (perhaps ad-hoc) rules for coercing binary expressions where exactly one operand is a string literal and the other operand is a numeric-typed column reference.
(1) is probably more principled, but I think it's too big of a change to undertake on short notice.
(2) sounds plausible to me at first blush, but it needs more consideration.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: github-unsubscribe@datafusion.apache.org
For additional commands, e-mail: github-help@datafusion.apache.org