Improve error message when string functions receive Binary types by lemorage · Pull Request #19819 · apache/datafusion

lemorage · 2026-01-15T04:17:23Z

Which issue does this PR close?

Related to Improve error messages when coercion fails #19004 and type coercion error in clickbench50 query #19809.

What changes are included in this PR?

Change internal error to user-facing error when function type coercion fails. Add helpful hint when Binary types are used with string functions.

Before:

Internal error: Expect TypeSignatureClass::Native(...) but received NativeType::Binary, DataType: Binary

After:

Error: Function 'split_part' requires String, but received Binary (DataType: Binary).
Hint: Binary types are not automatically coerced to String. Use CAST(column AS VARCHAR) to convert Binary data to String.

lemorage · 2026-01-15T04:24:29Z

The fix is prepared, I am not sure if this would be a reasonable change tbh tho. Maybe just setting config is fine? set datafusion.execution.parquet.binary_as_string = true. Need to be discussed before merging.

Jefffrey · 2026-01-15T04:43:27Z

Personally I'm not sure about this approach as it might suggest all other string functions with similar signature should now be fixed to also ensure they coerce from binary 🤔

e.g.

datafusion/datafusion/functions/src/string/ascii.rs

Lines 63 to 74 in 0808f3a

    
           impl AsciiFunc { 
        
               pub fn new() -> Self { 
        
                   Self { 
        
                       signature: Signature::coercible( 
        
                           vec![Coercion::new_exact(TypeSignatureClass::Native( 
        
                               logical_string(), 
        
                           ))], 
        
                           Volatility::Immutable, 
        
                       ), 
        
                   } 
        
               } 
        
           }

datafusion/datafusion/functions/src/string/btrim.rs

Lines 80 to 98 in 0808f3a

    
           impl BTrimFunc { 
        
               pub fn new() -> Self { 
        
                   Self { 
        
                       signature: Signature::one_of( 
        
                           vec![ 
        
                               TypeSignature::Coercible(vec![ 
        
                                   Coercion::new_exact(TypeSignatureClass::Native(logical_string())), 
        
                                   Coercion::new_exact(TypeSignatureClass::Native(logical_string())), 
        
                               ]), 
        
                               TypeSignature::Coercible(vec![Coercion::new_exact( 
        
                                   TypeSignatureClass::Native(logical_string()), 
        
                               )]), 
        
                           ], 
        
                           Volatility::Immutable, 
        
                       ), 
        
                       aliases: vec![String::from("trim")], 
        
                   } 
        
               } 
        
           }

datafusion/datafusion/functions/src/string/contains.rs

Lines 59 to 71 in 0808f3a

    
           impl ContainsFunc { 
        
               pub fn new() -> Self { 
        
                   Self { 
        
                       signature: Signature::coercible( 
        
                           vec![ 
        
                               Coercion::new_exact(TypeSignatureClass::Native(logical_string())), 
        
                               Coercion::new_exact(TypeSignatureClass::Native(logical_string())), 
        
                           ], 
        
                           Volatility::Immutable, 
        
                       ), 
        
                   } 
        
               } 
        
           }

adriangb · 2026-01-15T04:44:19Z

I guess ClickHouse does coerce them? So is this just a case of our semantics != ClickHouse?

Jefffrey · 2026-01-15T04:56:03Z

I guess ClickHouse does coerce them? So is this just a case of our semantics != ClickHouse?

It's a similar problem to how Spark will coerce strings to ints for their math functions but we don't.

Related: Support numeric string argument in functions #13285 (well it mentions PG here but same holds for Spark)

martin-g · 2026-01-15T11:29:55Z

replace.rs uses binary_to_string_coercion() in its invoke_with_args():
https://github.com/lemorage/datafusion/blob/85eebcd25dfbe8e2d2d75d85b8683de8be4851e8/datafusion/functions/src/string/replace.rs#L117-L118

Same for:

and maybe more.

alamb · 2026-01-15T18:35:05Z

I guess ClickHouse does coerce them? So is this just a case of our semantics != ClickHouse?

The problem is only with the ClickHouse hits_partitioned dataset (which has 100 parquet files) which was written by an ancient version of pyarrow which did not correctly annotate the string columns as STRING. This is the only usecase for this config:

https://datafusion.apache.org/user-guide/configs.html

datafusion.execution.parquet.binary_as_string	false	(reading) If true, parquet reader will read columns of Binary/LargeBinary with Utf8, and BinaryView with Utf8View. Parquet files generated by some legacy writers do not correctly set the UTF8 flag for strings, causing string columns to be loaded as BLOB instead.

So TLDR I don't think we need to make split_part work with binary data, as proposed in this PR.

Instead I propose:

Change the internal error to a proper error when running split_part on binary data
Update the benchmarks to properly set the binary as string. I made a PR here: Use correct setting for click bench queries in sql_planner benchmark #19835

alamb · 2026-01-15T18:36:01Z

Thank you for working on this @lemorage

Did you find out what was causing the internal error? I would expect the query would error with a normal error (about not supporting binary columns)

Jefffrey · 2026-01-15T23:44:31Z

Did you find out what was causing the internal error? I would expect the query would error with a normal error (about not supporting binary columns)

This is related to

Improve error messages when coercion fails #19004

lemorage · 2026-01-16T02:50:17Z

Thanks folks! Have updated the PR to improve the error msg better instead.

datafusion/expr/src/type_coercion/functions.rs

… Binary type

alamb

Thank you @lemorage -- looks great to me

alamb · 2026-01-18T17:12:28Z

datafusion/sqllogictest/test_files/encoding.slt

 ;

 # errors
-query error DataFusion error: Error during planning: Internal error: Expect TypeSignatureClass::Binary but received NativeType::Int64, DataType: Int64


👍 for a planning error rather than internal error

alamb · 2026-01-18T17:13:09Z

datafusion/sqllogictest/test_files/datetime/timestamps.slt

 select make_time(22, 1, '');

-query error Expect TypeSignatureClass::Native\(LogicalType\(Native\(Int32\), Int32\)\) but received NativeType::Float64, DataType: Float64
+query error DataFusion error: Error during planning: Function 'make_time' requires TypeSignatureClass::Native\(LogicalType\(Native\(Int32\), Int32\)\), but received Float64 \(DataType: Float64\)


I think these messages are much nicer -- thank you @lemorage

alamb · 2026-01-22T22:05:52Z

Thanks again @lemorage

lemorage · 2026-01-23T04:27:16Z

@alamb Thanks! And also all the discussions through! ❤️

…che#19819) ## Which issue does this PR close?  - Related to apache#19004 and apache#19809. ## What changes are included in this PR?  Change internal error to user-facing error when function type coercion fails. Add helpful hint when Binary types are used with string functions. Before: ``` Internal error: Expect TypeSignatureClass::Native(...) but received NativeType::Binary, DataType: Binary ``` After: ``` Error: Function 'split_part' requires String, but received Binary (DataType: Binary). Hint: Binary types are not automatically coerced to String. Use CAST(column AS VARCHAR) to convert Binary data to String. ```

github-actions bot added sqllogictest SQL Logic Tests (.slt) functions Changes to functions implementation labels Jan 15, 2026

fix: improve error message for type coercion failures

62d712e

lemorage force-pushed the hotfix/coerce branch from 9d88bfb to 62d712e Compare January 16, 2026 02:42

github-actions bot added logical-expr Logical plan and expressions and removed sqllogictest SQL Logic Tests (.slt) functions Changes to functions implementation labels Jan 16, 2026

lemorage changed the title ~~fix: allow Binary type coercion in split_part function~~ Improve error message when string functions receive Binary types Jan 16, 2026

alamb reviewed Jan 16, 2026

View reviewed changes

datafusion/expr/src/type_coercion/functions.rs Show resolved Hide resolved

lemorage added 2 commits January 17, 2026 16:08

test: add tests for verifying error msg when string functions receive…

8fc2842

… Binary type

Merge branch 'main' into hotfix/coerce

a517432

github-actions bot added the sqllogictest SQL Logic Tests (.slt) label Jan 17, 2026

test: update test expectations for new type coercion error format

4439926

alamb approved these changes Jan 18, 2026

View reviewed changes

alamb added this pull request to the merge queue Jan 22, 2026

Merged via the queue into apache:main with commit cf76352 Jan 22, 2026
32 checks passed

Conversation

lemorage commented Jan 15, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Which issue does this PR close?

What changes are included in this PR?

Uh oh!

lemorage commented Jan 15, 2026

Uh oh!

Jefffrey commented Jan 15, 2026

Uh oh!

adriangb commented Jan 15, 2026

Uh oh!

Jefffrey commented Jan 15, 2026

Uh oh!

martin-g commented Jan 15, 2026

Uh oh!

alamb commented Jan 15, 2026

Uh oh!

alamb commented Jan 15, 2026

Uh oh!

Jefffrey commented Jan 15, 2026

Uh oh!

lemorage commented Jan 16, 2026

Uh oh!

Uh oh!

alamb left a comment

Choose a reason for hiding this comment

Uh oh!

alamb Jan 18, 2026

Choose a reason for hiding this comment

Uh oh!

alamb Jan 18, 2026

Choose a reason for hiding this comment

Uh oh!

alamb commented Jan 22, 2026

Uh oh!

Uh oh!

lemorage commented Jan 23, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

5 participants

lemorage commented Jan 15, 2026 •

edited

Loading