You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@datafusion.apache.org by "scsmithr (via GitHub)" <gi...@apache.org> on 2025/03/11 14:55:48 UTC

[I] Incorrect cast of integer columns to utf8 when comparing with utf8 constant [datafusion]

scsmithr opened a new issue, #15161:
URL: https://github.com/apache/datafusion/issues/15161

   ### Describe the bug
   
   A comparison like `column1 < '10'` (where `column1` is an int64) will cast `column1` to utf8 instead of casting the utf8 constant to an integer.
   
   Typically string constants in a sql query are treated as unknown, and preference should be on casting the "unknown" value to a target type.
   
   
   ### To Reproduce
   
   ```
   DataFusion CLI v46.0.0
   > create table t1 as (values (1), (2), (3));
   0 row(s) fetched. 
   Elapsed 0.026 seconds.
   
   > select * from t1 where column1 < '10';
   +---------+
   | column1 |
   +---------+
   | 1       |
   +---------+
   1 row(s) fetched. 
   Elapsed 0.015 seconds.
   
   > select * from t1 where column1 < 'hello';
   +---------+
   | column1 |
   +---------+
   | 1       |
   | 2       |
   | 3       |
   +---------+
   3 row(s) fetched. 
   Elapsed 0.007 seconds.
   
   > select arrow_typeof(column1) from t1 limit 1;
   +--------------------------+
   | arrow_typeof(t1.column1) |
   +--------------------------+
   | Int64                    |
   +--------------------------+
   1 row(s) fetched. 
   Elapsed 0.010 seconds.
   
   > explain select * from t1 where column1 < '10';
   +---------------+-------------------------------------------------------+
   | plan_type     | plan                                                  |
   +---------------+-------------------------------------------------------+
   | logical_plan  | Filter: CAST(t1.column1 AS Utf8) < Utf8("10")         |
   |               |   TableScan: t1 projection=[column1]                  |
   | physical_plan | CoalesceBatchesExec: target_batch_size=8192           |
   |               |   FilterExec: CAST(column1@0 AS Utf8) < 10            |
   |               |     DataSourceExec: partitions=1, partition_sizes=[1] |
   |               |                                                       |
   +---------------+-------------------------------------------------------+
   2 row(s) fetched. 
   Elapsed 0.009 seconds.
   ```
   
   ### Expected behavior
   
   `column1` not to be cast to a string.
   
   ### Additional context
   
   _No response_


-- 
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.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: [I] Incorrect cast of integer columns to utf8 when comparing with utf8 constant [datafusion]

Posted by "cetra3 (via GitHub)" <gi...@apache.org>.
cetra3 commented on issue #15161:
URL: https://github.com/apache/datafusion/issues/15161#issuecomment-3958341953

   Here's an example of where `Unknown` might be preferable: https://github.com/apache/datafusion/pull/20543


-- 
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: [I] Incorrect cast of integer columns to utf8 when comparing with utf8 constant [datafusion]

Posted by "mbutrovich (via GitHub)" <gi...@apache.org>.
mbutrovich commented on issue #15161:
URL: https://github.com/apache/datafusion/issues/15161#issuecomment-3998951698

   > I agree; the question is which existing system do we want to follow here. Postgres (and the SQL spec) are more strict here, whereas other systems like DuckDB and SparkSQL tend to be more permissive.
   
   Historically DataFusion errs more toward Postgres behavior. When in doubt, that's what I'll recommend.


-- 
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: [I] Incorrect cast of integer columns to utf8 when comparing with utf8 constant [datafusion]

Posted by "neilconway (via GitHub)" <gi...@apache.org>.
neilconway commented on issue #15161:
URL: https://github.com/apache/datafusion/issues/15161#issuecomment-3923244192

   > when comparing a numeric column (int or float) with a string literal, cast the string literal to the column's numeric type, not the other way around.
   
   I don't think making type coercion behave differently for literals vs. columns (of the same type) is the right approach.
   
   I think @alamb's comment above is well-put:
   
   > I agree we should split the coercion rules for union and binary comparison
   
   For UNION (and CASE, coercion within an IN list, maybe other situations), we want to find the most general type that might be able to hold both values. So we want numeric -> string.
   
   Otherwise, we want string -> numeric. In an expression like `numeric_col < '500'`, the user obviously intends a numeric comparison. And an expression like `numeric_col < 'foo'` is invalid and we can safely reject it.
   
   So we need two different sets of type coercion rules, one for UNION, CASE, and maybe a few other situations, and a different set of rules for the rest of the 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: [I] Incorrect cast of integer columns to utf8 when comparing with utf8 constant [datafusion]

Posted by "jonahgao (via GitHub)" <gi...@apache.org>.
jonahgao commented on issue #15161:
URL: https://github.com/apache/datafusion/issues/15161#issuecomment-2717913303

   I checked and found that Spark has the same behavior as PostgreSQL. Maybe we should use a different coercion rule for this since the current [`comparison_coercion`](https://github.com/apache/datafusion/blob/bb1611578f8af42aacf611cb580940964665702d/datafusion/expr-common/src/type_coercion/binary.rs#L710) is also used by `union`.


-- 
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: [I] Incorrect cast of integer columns to utf8 when comparing with utf8 constant [datafusion]

Posted by "adriangb (via GitHub)" <gi...@apache.org>.
adriangb closed issue #15161: Incorrect cast of integer columns to utf8 when comparing with utf8 constant
URL: https://github.com/apache/datafusion/issues/15161


-- 
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: [I] Incorrect cast of integer columns to utf8 when comparing with utf8 constant [datafusion]

Posted by "neilconway (via GitHub)" <gi...@apache.org>.
neilconway commented on issue #15161:
URL: https://github.com/apache/datafusion/issues/15161#issuecomment-3922429717

   take


-- 
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: [I] Incorrect cast of integer columns to utf8 when comparing with utf8 constant [datafusion]

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on issue #15161:
URL: https://github.com/apache/datafusion/issues/15161#issuecomment-2722603481

   > I checked and found that Spark has the same behavior as PostgreSQL. Maybe we should use a different coercion rule for this since the current [`comparison_coercion`](https://github.com/apache/datafusion/blob/bb1611578f8af42aacf611cb580940964665702d/datafusion/expr-common/src/type_coercion/binary.rs#L710) is also used by `union`.
   
   I agree we should split the coercion rules for union and binary comparison
   
   


-- 
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: [I] Incorrect cast of integer columns to utf8 when comparing with utf8 constant [datafusion]

Posted by "neilconway (via GitHub)" <gi...@apache.org>.
neilconway commented on issue #15161:
URL: https://github.com/apache/datafusion/issues/15161#issuecomment-3922429375

   @AlonSpivack Thanks for flagging this! I'm sorry that this resulted in incorrect query results in production for you.
   
   At a high-level, I agree with your analysis, and also that this is a serious issue that has not been sufficiently addressed by the prior work in this area. I'll take a look at coming up with a plan on how to address the problem.


-- 
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: [I] Incorrect cast of integer columns to utf8 when comparing with utf8 constant [datafusion]

Posted by "AlonSpivack (via GitHub)" <gi...@apache.org>.
AlonSpivack commented on issue #15161:
URL: https://github.com/apache/datafusion/issues/15161#issuecomment-3941065101

   Thanks @neilconway for putting together such a clear comparison of the two approaches.
   
   After reviewing both PRs, I'm in favor of **Approach 2 (PR #20456)**. Here's my reasoning:
   
   The key advantage of Approach 2 is that it catches errors at **planning time** rather than deferring them to runtime. Under Approach 1, `text_col < 5` silently inserts a `CAST(text_col AS Int64)` that will crash at runtime if the column contains non-numeric data. The user gets no indication of the problem until a specific row triggers the cast failure, which could happen intermittently depending on the data. Approach 2 rejects this at planning time with a clear type mismatch error, which is the correct behavior.
   
   The `IN` list handling is also a significant differentiator. Under Approach 1, `int_col IN ('5', '10')` still routes through `type_union_coercion`, resulting in lexicographic comparison - the exact class of bug we're trying to fix. Approach 2 handles this correctly by converting the literals at planning time.
   
   Regarding @adriangb's concern about stricter coercion impacting user experience: I think silently returning wrong results is a far worse UX than a clear planning error with an actionable message. Users who genuinely need string comparison can always use an explicit `CAST`.
   
   ---
   
   **Looking further ahead**, I think it's worth acknowledging what @neilconway already noted: Approach 2 is fundamentally an approximation of Postgres's `unknown` type semantics. It works well for the common cases (literal comparisons, `IN` lists, `BETWEEN`), but it won't cover non-literal string expressions like `int_col = '2025' || '-' || '01'`, and it requires the analyzer to handle each expression type individually.
   
   The "right" long-term solution is probably introducing an `unknown` (or `unresolved`) type that string literals receive initially, which then gets resolved to a concrete type based on context - similar to how Postgres handles this. That would solve the problem at its root and cover all expression forms uniformly, not just the ones we explicitly handle in the analyzer.
   
   I fully understand this is a large undertaking - it touches the type system, the parser/planner, the analyzer, coercion rules, built-in function signatures, and likely hundreds of tests. It's not something to block the current fix on.
   
   But if the community agrees this is the right long-term direction, I'd be happy to take part in driving it forward - from the initial design/RFC through implementation. I'm already deep in this area of the codebase due to hitting this bug in production, and I'd like to turn that experience into a meaningful contribution.
   
   So my suggestion would be:
   1. Merge PR #20456 as the immediate fix (it solves the critical correctness issue now)
   2. Open a separate issue/RFC to discuss the `unknown` type as a long-term project
   
   What do you think, @neilconway @alamb @adriangb?


-- 
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: [I] Incorrect cast of integer columns to utf8 when comparing with utf8 constant [datafusion]

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on issue #15161:
URL: https://github.com/apache/datafusion/issues/15161#issuecomment-2722604447

   I also put this issue on my wishlist for
   - https://github.com/apache/datafusion/issues/15072


-- 
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: [I] Incorrect cast of integer columns to utf8 when comparing with utf8 constant [datafusion]

Posted by "neilconway (via GitHub)" <gi...@apache.org>.
neilconway commented on issue #15161:
URL: https://github.com/apache/datafusion/issues/15161#issuecomment-3936788737

   Alright, I spent some more time on this; I have PRs open for two possible approaches:
   
   1. #20426 
   2. #20456 
   
   Approach (1) is what I was initially planning. We prefer string -> numeric coercions in comparison contexts, and numeric -> string coercions in contexts like UNION and CASE. That fixes the issue raised in this ticket, but it continues to allow queries that I think we should reject, like `WHERE text_col < 5`, `WHERE text_col < int_col`, and `WHERE text_col IN (5, 10)`.
   
   Approach (2) removes string <-> numeric coercions in comparison contexts entirely. Instead, we arrange for the type coercion analysis pass to check for situations where a numeric column is compared with a string literal, and then insert a coercion for that literal as a special case. This is stricter than (1) but I think it rejects more classes of dubious queries. I initially thought that treating literals differently for type coercion seemed wrong, but on reflection I think it's okay: we are basically approximating the idea that string literals might not really be "string literals", they are more properly treated as "unknown literals" and then typed based on context. This is explicitly what Postgres does; we could consider a similar approach but IMO that's too large of a project for right now.
   
   I had Claude Code compare the two approaches:
   
   ```
   
     ┌───────────────────────────────┬───────────────────────────────────────────────────────────────────────┬──────────────────────────────────────────────────────────────────┐
     │         SQL Construct         │                        PR #20426 (type-based)                         │                    PR #20456 (literal-aware)                     │
     ├───────────────────────────────┼───────────────────────────────────────────────────────────────────────┼──────────────────────────────────────────────────────────────────┤
     │ int_col < '5'                 │ Numeric comparison (runtime CAST)                                     │ Numeric comparison (planning-time literal conversion)            │
     ├───────────────────────────────┼───────────────────────────────────────────────────────────────────────┼──────────────────────────────────────────────────────────────────┤
     │ text_col < 5                  │ Inserts CAST(text_col AS Int64) — runtime error if data isn't numeric │ Planning-time error (type mismatch)                              │
     ├───────────────────────────────┼───────────────────────────────────────────────────────────────────────┼──────────────────────────────────────────────────────────────────┤
     │ text_col = int_col            │ Inserts CAST(text_col AS Int64) — runtime error if data isn't numeric │ Planning-time error (type mismatch)                              │
     ├───────────────────────────────┼───────────────────────────────────────────────────────────────────────┼──────────────────────────────────────────────────────────────────┤
     │ int_col IN ('5', '10')        │ Coerces to Utf8 (uses type_union_coercion) — lexicographic comparison │ Converts literals to Int64 at planning time — numeric comparison │
     ├───────────────────────────────┼───────────────────────────────────────────────────────────────────────┼──────────────────────────────────────────────────────────────────┤
     │ text_col IN (5, 10)           │ Coerces to Utf8 — string comparison, succeeds                         │ Planning-time error                                              │
     ├───────────────────────────────┼───────────────────────────────────────────────────────────────────────┼──────────────────────────────────────────────────────────────────┤
     │ int_col BETWEEN '5' AND '100' │ Numeric (runtime CAST)                                                │ Numeric (planning-time literal conversion)                       │
     ├───────────────────────────────┼───────────────────────────────────────────────────────────────────────┼──────────────────────────────────────────────────────────────────┤
     │ text_col BETWEEN 1 AND 10     │ Runtime error (CASTs column to Int64)                                 │ Planning-time error                                              │
     ├───────────────────────────────┼───────────────────────────────────────────────────────────────────────┼──────────────────────────────────────────────────────────────────┤
     │ '5' BETWEEN 1 AND 10          │ true (runtime CAST)                                                   │ true (planning-time literal conversion)                          │
     ├───────────────────────────────┼───────────────────────────────────────────────────────────────────────┼──────────────────────────────────────────────────────────────────┤
     │ int_col < 'hello'             │ Runtime error (CAST fails)                                            │ Planning-time error (same message)                               │
     ├───────────────────────────────┼───────────────────────────────────────────────────────────────────────┼──────────────────────────────────────────────────────────────────┤
     │ int_col = '99.99'             │ Runtime error                                                         │ Planning-time error                                              │
     ├───────────────────────────────┼───────────────────────────────────────────────────────────────────────┼──────────────────────────────────────────────────────────────────┤
     │ UNION int + string            │ Coerces to Utf8                                                       │ Coerces to Utf8 (same)                                           │
     ├───────────────────────────────┼───────────────────────────────────────────────────────────────────────┼──────────────────────────────────────────────────────────────────┤
     │ CASE mixed types              │ Coerces to Utf8                                                       │ Coerces to Utf8 (same)                                           │
     └───────────────────────────────┴───────────────────────────────────────────────────────────────────────┴──────────────────────────────────────────────────────────────────┘
   ```
   
   If folks have an opinion on which approach they think is better (or would like to suggest an alternative), I'd love to hear 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: [I] Incorrect cast of integer columns to utf8 when comparing with utf8 constant [datafusion]

Posted by "AlonSpivack (via GitHub)" <gi...@apache.org>.
AlonSpivack commented on issue #15161:
URL: https://github.com/apache/datafusion/issues/15161#issuecomment-3925376196

   Thanks @neilconway  for looking into this so quickly and validating the severity of the issue! 
   
   I completely agree with you and @alamb that splitting the coercion rules (having one set for UNION/CASE and another strict set for comparisons) is the right architectural fix. 
   
   I see you've already opened PR #20426 to implement this approach. That's awesome.
   I'll move over to the PR and leave some thoughts there regarding the edge cases you mentioned!


-- 
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: [I] Incorrect cast of integer columns to utf8 when comparing with utf8 constant [datafusion]

Posted by "piyushgarg5021 (via GitHub)" <gi...@apache.org>.
piyushgarg5021 commented on issue #15161:
URL: https://github.com/apache/datafusion/issues/15161#issuecomment-2715474312

   @scsmithr 
   
   The issue lies in the type coercion logic during query planning. The fix involves modifying the coercion rules to prioritize casting the string literal to the column's type instead of casting the column to Utf8.
   
   Changes Required:
   1. Modify Type Coercion Logic:
   - Update the coercion rules in the query planner to cast the string literal to the column's type when comparing a column with a literal.
   
   For example, in the query column1 < '10', the string literal '10' should be cast to Int64.
   
   2. Error Handling:
   - Ensure that invalid casts (e.g., 'hello' to Int64) result in an error during query planning or execution.
   
   **Implementation Plan:**
   
   1. Locate the type coercion logic in the DataFusion codebase (likely in the query planner or expression evaluation module).
   2. Modify the logic to prioritize casting the literal to the column's type.
   3. Add tests to verify the fix:
   - Test numeric string literals (e.g., '10').
   - Test non-numeric string literals (e.g., 'hello').
   - Test edge cases (e.g., empty strings, NULL values).
   
   // Updated coercion logic
   if left_type != right_type {
       if left_type.is_numeric() && right_type == DataType::Utf8 {
           // Cast right to left_type (e.g., cast '10' to Int64)
       } else if right_type.is_numeric() && left_type == DataType::Utf8 {
           // Cast left to right_type
       } else {
           // Handle other cases
       }
   }
   
   #[test]
   fn test_int_column_less_than_string_literal() {
       let sql = "select * from t1 where column1 < '10';";
       let expected = vec![1, 2, 3]; // Expected results after correct casting
       assert_eq!(execute_query(sql), expected);
   }
   
   #[test]
   fn test_int_column_less_than_invalid_string() {
       let sql = "select * from t1 where column1 < 'hello';";
       assert!(execute_query(sql).is_err()); // Should return an error
   }
   
   
   
   By follows that i think it is solve


-- 
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: [I] Incorrect cast of integer columns to utf8 when comparing with utf8 constant [datafusion]

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on issue #15161:
URL: https://github.com/apache/datafusion/issues/15161#issuecomment-2714719767

   👋  @scsmithr 
   
   FYI @jonahgao 
   
   I vaguely remember something related to this automatic coercion in the past 🤔 
   
   BTW @alan910127  and I are trying to fix the equality case in 
   - https://github.com/apache/datafusion/issues/15035
   - https://github.com/apache/datafusion/pull/15110


-- 
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: [I] Incorrect cast of integer columns to utf8 when comparing with utf8 constant [datafusion]

Posted by "AlonSpivack (via GitHub)" <gi...@apache.org>.
AlonSpivack commented on issue #15161:
URL: https://github.com/apache/datafusion/issues/15161#issuecomment-3904440723

   <html><head></head><body><p>I'm hitting this bug in production on <strong>DataFusion v52</strong>, and it's causing <strong>silently incorrect query results</strong> across multiple scenarios.</p>
   <p>I want to add important context beyond what's already discussed here - this bug is wider than originally reported.</p>
   <hr>
   <h3>1. PR #15110 (merged) only fixed a very narrow subset of the problem</h3>
   <p>The <code>cast_literal_to_type_with_op</code> function that was merged only handles:</p>
   <ul>
   <li><strong>Operators</strong>: <code>=</code> and <code>!=</code> only</li>
   <li><strong>Types</strong>: integer types only (<code>Int8</code>–<code>Int64</code>, <code>UInt8</code>–<code>UInt64</code>)</li>
   </ul>
   <p>This means <strong><code>&lt;</code>, <code>&gt;</code>, <code>&lt;=</code>, <code>&gt;=</code> are still broken for integers</strong>, and <strong>all operators are broken for float types</strong> (<code>Float32</code>, <code>Float64</code>). PR #15482 attempted to fix the broader problem at the type coercion level but was closed as stale in June 2025.</p>
   <hr>
   <h3>2. Float columns are also affected - not just integers</h3>
   <p>The original issue only mentions integer columns, but float columns have the same problem - and in some cases it's even worse:</p>
   <pre><code class="language-sql">CREATE TABLE t AS SELECT * FROM (VALUES (1.0), (5.0), (5.5), (325.0), (499.0)) AS t(num_float);
   
   -- WRONG RESULTS: returns 499.0, 325.0 because '499.0' &lt; '5' lexicographically
   SELECT * FROM t WHERE num_float &lt; '5';
   
   -- WRONG RESULTS: returns 0 rows! CAST(5.0 AS Utf8) produces '5.0' which != '5'
   SELECT * FROM t WHERE num_float = '5';
   
   -- Might accidentally "work" because CAST(5.0 AS Utf8) = '5.0', but it's still doing string comparison
   SELECT * FROM t WHERE num_float = '5.0';
   
   -- WRONG: same lexicographic issue
   SELECT * FROM t WHERE num_float &gt; '5';
   </code></pre>
   <p>The <code>num_float = '5'</code> case is especially dangerous - it <strong>silently returns zero rows</strong> instead of matching the value <code>5.0</code>, because the float gets cast to the string <code>"5.0"</code> which doesn't equal <code>"5"</code>. No error, no warning, just wrong data.</p>
   <hr>
   <h3>3. The integer <code>&lt;</code>/<code>&gt;</code> cases are equally broken</h3>
   <pre><code class="language-sql">CREATE TABLE t2 AS SELECT * FROM (VALUES (1), (5), (325), (499), (1000)) AS t2(num_int);
   
   -- WRONG RESULTS: returns 499, 325 etc. because '499' &lt; '5' in string ordering  
   SELECT * FROM t2 WHERE num_int &lt; '5';
   
   -- This one works (fixed by PR #15110), but only for = and !=
   SELECT * FROM t2 WHERE num_int = '5';
   </code></pre>
   <hr>
   <h3>4. What Postgres does (the correct behavior)</h3>
   <p>Postgres treats the string literal as "unknown" type and casts it to the column type - not the other way around:</p>
   <pre><code>postgres=# SELECT * FROM t WHERE num_float &lt; '5';
   -- Returns all values less than 5.0 (correct numeric comparison)
   
   postgres=# SELECT * FROM t WHERE num_float = '5';
   -- Matches the value 5.0 (correctly casts '5' to float)
   
   postgres=# SELECT * FROM t WHERE num_float &lt; 'hello';
   ERROR:  invalid input syntax for type double precision: "hello"
   </code></pre>
   <hr>
   <h3>5. Summary of broken scenarios in v52</h3>
   
   Scenario | Fixed? | What happens
   -- | -- | --
   int_col = '5' | ✅ (PR #15110) | Correct
   int_col != '5' | ✅ (PR #15110) | Correct
   int_col < '5' | ❌ | Wrong results
   int_col > '5' | ❌ | Wrong results
   int_col <= '5' | ❌ | Wrong results
   int_col >= '5' | ❌ | Wrong results
   float_col = '5' | ❌ | Wrong results
   float_col = '5.0' | ✅ | Accidentally works via string match
   float_col != '5' | ❌ | Wrong results
   float_col < '5' | ❌ | Wrong results
   float_col > '5' | ❌ | Wrong results
   
   
   <hr>
   <h3>6. My thoughts on the fix</h3>
   <p>I think the ideal solution is what @scsmithr suggested - fix this at the <strong>type coercion level</strong>: when comparing a numeric column (int or float) with a string literal, <strong>cast the string literal to the column's numeric type</strong>, not the other way around. If the cast fails (e.g., <code>num_col &lt; 'hello'</code>), throw a clear planning error like <code>Cannot coerce 'hello' to type 'Float64'</code>.</p>
   <p>At the very least, even if the full coercion fix takes time, I think DataFusion should <strong>throw an error/exception immediately</strong> when it encounters a string value on one side of any comparison operator (<code>=</code>, <code>!=</code>, <code>&lt;</code>, <code>&gt;</code>, <code>&lt;=</code>, <code>&gt;=</code>) with a numeric column on the other side, with a message explaining that implicit string-to-column casting is not supported and that the types are incompatible. This would prevent the current situation where users silently get wrong results without any indication.</p>
   <p>If a user actually wants string comparison behavior, they can explicitly cast in the SQL:</p>
   <pre><code class="language-sql">-- Explicit cast: user is intentionally asking for string comparison
   SELECT * FROM t WHERE CAST(num_float AS TEXT) &lt; '5';
   </code></pre>
   <p>This way, the default behavior is safe (either correct numeric comparison or a clear error), and users who truly want string comparison can opt into it explicitly.</p>
   <hr>
   
   ### 7. Status question
   
   Is anyone currently working on a revised approach after PR #15482 was closed? 
   
   In that PR, @alamb [[noted](https://github.com/apache/datafusion/pull/15482#discussion_r2026999963)](https://github.com/apache/datafusion/pull/15482#discussion_r2026999963) that the fix shouldn't check for `Expr::Literal` inside the type coercion rewriter, but instead should change the **base type coercion rules** themselves (which work on `DataType`, not on expression kind). He also pointed out that checking for literals won't handle string expressions like `date_col = '2025'||'-'||'02'`. Separately in PR #15110, @alamb [[agreed with @scsmithr](https://github.com/apache/datafusion/pull/15110#issuecomment-2715747690)](https://github.com/apache/datafusion/pull/15110#issuecomment-2715747690) that changing the coercion rules is a better plan than trying to fix it up afterwards in the optimizer.
   
   Has there been any progress on that direction? This is a **correctness bug** that silently returns wrong data. In my production system I was getting incorrect results for a long time before I tracked it down to this coercion issue. I'd love to help move this forward if there's a way I can contribute.
   


-- 
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: [I] Incorrect cast of integer columns to utf8 when comparing with utf8 constant [datafusion]

Posted by "alamb (via GitHub)" <gi...@apache.org>.
alamb commented on issue #15161:
URL: https://github.com/apache/datafusion/issues/15161#issuecomment-3954526650

   > Matching Postgres’ behavior makes sense to me in general, but I do think there’s a negative user experience impact by making coercion stricter, especially since there is no way for implementers to add special cases or customize coercion atm.
   
   In general, I think making DataFusion match the behavior of another system is much more preferable than inventing its own behavior (the world has enough SQL dialects already in my opinion)
   
   > I fully understand this is a large undertaking - it touches the type system, the parser/planner, the analyzer, coercion rules, built-in function signatures, and likely hundreds of tests. It's not something to block the current fix on.
   
   I agree it sounds pretty major. I think it would be worth sketching out what benefits such work would produce (other than potentially a "cleaner" codebase) -- it is a tough sell on API churn for downstream users if they don't get anything out of having to update their APIs


-- 
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: [I] Incorrect cast of integer columns to utf8 when comparing with utf8 constant [datafusion]

Posted by "adriangb (via GitHub)" <gi...@apache.org>.
adriangb commented on issue #15161:
URL: https://github.com/apache/datafusion/issues/15161#issuecomment-3939265787

   Matching Postgres’ behavior makes sense to me in general, but I do think there’s a negative user experience impact by making coercion stricter, especially since there is no way for implementers to add special cases or customize coercion atm.


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