Improve formatting of datatypes by emilk · Pull Request #20605 · apache/datafusion

emilk · 2026-02-27T18:15:11Z

Which issue does this PR close?

Part of Improve error messages when coercion fails #19004

Rationale for this change

Error messages should be friendly, short, and readable.

What changes are included in this PR?

Implement proper Display for dyn LogicalType, TypeSignature, TypeSignatureClass, and Coercion instead of falling back to Debug
Switch error messages across the codebase from {:?} (Debug) to {} (Display)
Add non-null prefix for non-nullable fields in NativeTypeDisplay, following Arrow's convention

The north star is to follow the convention set by arrow-rs

Are these changes tested?

New snapshot tests have been introduced, and existing ones updated.

Are there any user-facing changes?

Yes!

Before

LogicalType(Native(List(LogicalField { name: "item", logical_type: LogicalType(Native(Int32), Int32), nullable: true })), List(LogicalField { name: "item", logical_type: LogicalType(Native(Int32), Int32), nullable: true }))

After

List(Int32)

Before

No function matches the given name and argument types 'round(Utf8, Utf8)'.
Candidate functions:
round(Coercion(TypeSignatureClass::Native(LogicalType(Native(Float64), Float64)),
implicit_coercion=ImplicitCoercion([Numeric], default_type=Float64)),
Coercion(TypeSignatureClass::Native(LogicalType(Native(Int64), Int64)),
implicit_coercion=ImplicitCoercion([Integer], default_type=Int64)))
Function 'abs' expects NativeType::Numeric but received NativeType::String
Failed to coerce arguments: coercion from Int32, Utf8 to the signature
Coercible([Coercion { desired_type: Native(LogicalType(Native(Float64), Float64)), ... }]) failed

After

No function matches the given name and argument types 'round(Utf8, Utf8)'.
Candidate functions:
round(Float64, Int64)
Function 'abs' expects Numeric but received String
Failed to coerce arguments: coercion from Int32, Utf8 to the signature Coercible(Float64, Int64) failed

emilk · 2026-02-28T12:59:42Z

oooops - sorry, didn't realize you were working on this :) Feel free to force-push again, and I'll go clean my apartment instead, like I should be doing…

…ercion Use Arrow DataType Display style as the north-star: terse, readable type names instead of dumping internal Rust enum structure. - `dyn LogicalType`: delegates to NativeType Display instead of Debug, e.g. `Int32` instead of `LogicalType(Native(Int32), Int32)` - `TypeSignatureClass`: shows just the class name, e.g. `Numeric` instead of `TypeSignatureClass::Numeric` - `Coercion`: shows just the desired type, e.g. `Float64` instead of `Coercion(TypeSignatureClass::...)` - `ImplicitCoercion`: uses Display for its fields instead of Debug This dramatically improves error messages like: Before: `round(Coercion(TypeSignatureClass::Native(LogicalType(Native(Float64), Float64)), implicit_coercion=ImplicitCoercion([Numeric], default_type=Float64), ...)` After: `round(Float64, Int64)` Co-Authored-By: Claude Opus 4.6 <noreply@anthropic.com>

…n the snapshots, so just check the first part of the error message

timsaucer

Very nice cleanup! I'll leave it open for one more working day in case anyone else wants to chime in.

timsaucer · 2026-03-02T13:43:27Z

@Jefffrey any opposition to merging this in? I see you also have #20070 which touches on some of the same things

Jefffrey · 2026-03-02T13:52:26Z

I'll try take a look at this tomorrow

We've updated to arrow 57, which _finally_ implements [proper `Display` formatting for `DataType`](apache/arrow-rs#8290). arrow-rs formats things slightly differently from how we have, but I think we should just converge on one standard here, in arrow-rs, and [in datafusion](apache/datafusion#20605). ## How to review Only look at changes in `.rs` files by [clicking here](https://github.com/rerun-io/reality/pull/967/changes?file-filters%5B%5D=.rs). Then take a look at a few snapshots Source-Ref: 8587961f0f1089a9a0f22c7e6a1cf907af81d0e6

Jefffrey

I think this is a very good step forward; we should leave the original issue open though as there's still improvements to be made to the error messages, but this PR really helps in cutting down the ugliness

## Which issue does this PR close? - Part of apache#19004 ## Rationale for this change Error messages should be friendly, short, and readable. ## What changes are included in this PR? * Implement proper `Display` for `dyn LogicalType`, `TypeSignature`, `TypeSignatureClass`, and `Coercion` instead of falling back to `Debug` * Switch error messages across the codebase from `{:?}` (`Debug`) to `{}` (`Display`) * Add `non-null` prefix for non-nullable fields in `NativeType` `Display`, following Arrow's convention The north star is to follow the convention set by arrow-rs ## Are these changes tested? New snapshot tests have been introduced, and existing ones updated. ## Are there any user-facing changes? Yes! #### Before > LogicalType(Native(List(LogicalField { name: "item", logical_type: LogicalType(Native(Int32), Int32), nullable: true })), List(LogicalField { name: "item", logical_type: LogicalType(Native(Int32), Int32), nullable: true })) #### After > List(Int32) #### Before ``` No function matches the given name and argument types 'round(Utf8, Utf8)'. Candidate functions: round(Coercion(TypeSignatureClass::Native(LogicalType(Native(Float64), Float64)), implicit_coercion=ImplicitCoercion([Numeric], default_type=Float64)), Coercion(TypeSignatureClass::Native(LogicalType(Native(Int64), Int64)), implicit_coercion=ImplicitCoercion([Integer], default_type=Int64))) Function 'abs' expects NativeType::Numeric but received NativeType::String Failed to coerce arguments: coercion from Int32, Utf8 to the signature Coercible([Coercion { desired_type: Native(LogicalType(Native(Float64), Float64)), ... }]) failed ``` #### After ``` No function matches the given name and argument types 'round(Utf8, Utf8)'. Candidate functions: round(Float64, Int64) Function 'abs' expects Numeric but received String Failed to coerce arguments: coercion from Int32, Utf8 to the signature Coercible(Float64, Int64) failed ``` --------- Co-authored-by: Claude Opus 4.6 <noreply@anthropic.com> Co-authored-by: Tim Saucer <timsaucer@gmail.com>

Three categories of fallout from the OneOf-signature-diagnostics cherry-pick that the upstream PR resolved via apache#20605 (formatting cleanup) but apache#20605 isn't on 52.5.0: 1. Two unused imports in datafusion-expr's test module (`IntervalUnit`, `logical_int64`) that compile-fail under `-D warnings` and trip clippy + lib-test builds. Dropped from the test imports in type_coercion/functions.rs. 2. Two sql-integration tests assert on the old `user-defined coercion failed with "..."` wording; updated to the new `failed with: ...` shape. 3. Eight sqllogictest expected-error patterns across arrow_typeof.slt, encoding.slt, expr.slt, scalar.slt, and datetime/timestamps.slt asserted on the old `Internal error: Expect TypeSignatureClass::X but received NativeType::Y` shape; updated to the new `Function 'NAME' requires TypeSignatureClass::X, but received Y (DataType: Y)` shape.

Cherry-pick of apache#21032. Adapted for 52.5.0: - Two unused imports in datafusion-expr's test module (`IntervalUnit`, `logical_int64`) compile-fail under -D warnings; dropped. - Two sql-integration tests and several sqllogictest expected-error patterns assumed the friendlier Display impl from apache#20605 was already on 52.5.0 (it isn't); updated those patterns to the pre-apache#20605 verbose `TypeSignatureClass::Native(LogicalType(...))` shape and to the post-apache#21032 `failed with: ...` (no quoted inner message) shape. - Also includes the author's own follow-up CI fix (was a separate commit upstream, folded here). Signed-off-by: yaommen <myanstu@163.com>

Cherry-pick of apache#21032. Adapted for 52.5.0: - Two unused imports in datafusion-expr's test module (`IntervalUnit`, `logical_int64`) compile-fail under -D warnings; dropped. - Two sql-integration tests and several sqllogictest expected-error patterns assumed the friendlier Display impl from apache#20605 was already on 52.5.0 (it isn't); updated those patterns to the pre-apache#20605 verbose `TypeSignatureClass::Native(LogicalType(...))` shape and to the post-apache#21032 `failed with: ...` (no quoted inner message) shape. - Also includes the author's own follow-up CI fix (was a separate commit upstream, folded here). Signed-off-by: yaommen <myanstu@163.com> (cherry picked from commit f478cdc)

github-actionsBot added logical-expr Logical plan and expressions optimizer Optimizer rules sqllogictest SQL Logic Tests (.slt) common Related to common crate functions Changes to functions implementation physical-plan Changes to the physical-plan crate sql SQL Planner labels Feb 27, 2026

timsaucer self-requested a review February 27, 2026 19:36

timsaucer force-pushed the emilk/improve-errors branch from 9b2c119 to 30f7fc8Compare February 28, 2026 12:54

emilk force-pushed the emilk/improve-errors branch from 30f7fc8 to 21e0efbCompare February 28, 2026 12:57

emilkand others added 8 commits February 28, 2026 08:42

Add snapshot tests of existing formatting
10ef397

Add non-null prefix
ef9408d

Use Display formatting for errors
bbb7ed3

cargo fmt
4b2f97d

A few more improvements
2cd0b73

Update another test
1991966

Update sql tests for new error message format
aea6186

timsaucer force-pushed the emilk/improve-errors branch from 21e0efb to aea6186Compare February 28, 2026 15:28

timsaucer added 2 commits February 28, 2026 14:25

Difference between ci and test profiles generates different results o…
013abab
…n the snapshots, so just check the first part of the error message

minor test update
dc6910e

timsaucer approved these changes Mar 1, 2026
View reviewed changes

Jefffrey mentioned this pull request Mar 2, 2026
Type signature matching errors are overly verbose and show internal details #20501
Open

Jefffrey approved these changes Mar 3, 2026
View reviewed changes

Comment threaddatafusion/sqllogictest/test_files/window.slt

timsaucer added this pull request to the merge queue Mar 4, 2026

Merged via the queue into apache:main with commit 3b028feMar 4, 2026
36 checks passed

Jefffrey mentioned this pull request Mar 22, 2026
feat: change approx percentile/median UDFs to return floats #21074
Merged

emilk mentioned this pull request Apr 10, 2026
New lint: disallowed_trait_usagerust-lang/rust-clippy#16810
Open

erratic-pattern mentioned this pull request May 13, 2026
Patched DF 52.5.0 (revision b) influxdata/arrow-datafusion#98
Closed

Navigation Menu

Uh oh!

Improve formatting of datatypes - #20605

Improve formatting of datatypes#20605
timsaucer merged 10 commits into
apache:mainfrom
rerun-io:emilk/improve-errors

emilk commented Feb 27, 2026•
edited
Loading

Uh oh!

emilk commented Feb 28, 2026

Uh oh!

timsaucer left a comment

Uh oh!

timsaucer commented Mar 2, 2026

Uh oh!

Jefffrey commented Mar 2, 2026

Uh oh!

Jefffrey left a comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Uh oh!

Conversation

emilk commented Feb 27, 2026• edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Which issue does this PR close?

Rationale for this change

What changes are included in this PR?

Are these changes tested?

Are there any user-facing changes?

Before

After

Before

After

Uh oh!

emilk commented Feb 28, 2026

Uh oh!

timsaucer left a comment

Choose a reason for hiding this comment

Uh oh!

timsaucer commented Mar 2, 2026

Uh oh!

Jefffrey commented Mar 2, 2026

Uh oh!

Jefffrey left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

emilk commented Feb 27, 2026•
edited
Loading