Skip to content

fix: cap the field list in "No field named" errors - #24931

Open
akashjainn wants to merge 2 commits into
apache:mainfrom
akashjainn:truncate-valid-fields-error
Open

fix: cap the field list in "No field named" errors#24931
akashjainn wants to merge 2 commits into
apache:mainfrom
akashjainn:truncate-valid-fields-error

Conversation

@akashjainn

@akashjainn akashjainn commented Sep 3, 2026

Copy link
Copy Markdown

Closes #5332.

Which issue does this PR close?

Closes #5332.

Rationale for this change

Selecting a column that doesn't exist lists every field in the schema. On a 200 column table that's about 2.8k characters, and the useful part of the message - the name that wasn't found and the "Did you mean" suggestion - ends up scrolled off the top.

> SELECT not_a_column FROM wide;
Error: Schema error: No field named not_a_column.
Valid fields are wide.col_0, wide.col_1, ... wide.col_199.

Filed in 2023, still reproduces.

What changes are included in this PR?

Cap the list at 20 fields and summarise the rest:

Valid fields are wide.col_0, ... wide.col_19 and 180 others.

2.8k characters down to 356. Schemas at or under 20 fields print the same as before, so the existing message assertions in dfschema.rs, column.rs, expr_rewriter and the dataframe tests are untouched - they're all well under that.

The issue suggested either dropping the list like Postgres does or truncating it. I kept it and bounded it, since the list is useful on narrow schemas and closest_valid_field already handles the common typo. Happy to switch to the Postgres behaviour instead if you'd rather.

Are these changes tested?

Four unit tests in datafusion/common/src/error.rs: a narrow schema, exactly 20 fields, 21 fields (checks it says "1 other" not "1 others"), and 200 fields.

cargo test -p datafusion-common: 608 passed. datafusion-expr --lib: 258 passed. cargo fmt clean.

Two things in my checkout that aren't from this change and reproduce on unmodified main: the arrow_test_data doctest fails because the testing/ submodule isn't initialised, and clippy reports an unused import of crate::config::TableParquetOptions.

Are there any user-facing changes?

The wording of SchemaError::FieldNotFound when a schema has more than 20 fields. No API change.

Selecting an unknown column from a wide table produces an error message that
lists every field in the schema. On a 200 column table that is ~2.8k characters
of output, and schemas with hundreds or thousands of columns are ordinary in
analytics workloads, so in practice the useful part of the message -- the name
that was not found, and the "Did you mean" suggestion -- is pushed off screen by
the list that follows it.

Cap the listed fields at 20 and summarise the rest, so the same query now
reports:

    Error: Schema error: No field named not_a_column.
    Valid fields are wide.col_0, ..., wide.col_19 and 180 others.

2.8k characters down to 356. Schemas at or below the cap are unchanged, which
keeps the list useful where it is short enough to read and leaves the existing
message assertions in dfschema, column, expr_rewriter and the dataframe tests
untouched.

Postgres omits the valid-field list entirely, but DataFusion's list is helpful
on narrow schemas and the closest-match suggestion already handles the common
typo case, so this keeps the list and bounds it rather than removing it.

Tests cover a narrow schema, exactly the cap, one field past the cap (singular
"1 other"), and a 200 field schema.
@github-actions github-actions Bot added the common Related to common crate label Sep 3, 2026
.to_string()
}

#[test]

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The tests mostly use contains, negative checks, and a length ceiling. They would still pass if fields 1–18 disappeared, punctuation broke, or unrelated text was appended.

How about using an insta snapshot for the complete 3-, 20-, 21-, and 200-field messages?

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for pointing that out! fixed here: 92c606c

@xudong963 xudong963 left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for the first contribution!

@codecov-commenter

codecov-commenter commented Sep 6, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 97.77778% with 1 line in your changes missing coverage. Please review.
✅ Project coverage is 81.64%. Comparing base (20d1c56) to head (31c5184).
⚠️ Report is 32 commits behind head on main.

Files with missing lines Patch % Lines
datafusion/common/src/error.rs 97.77% 0 Missing and 1 partial ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main   #24931      +/-   ##
==========================================
- Coverage   81.64%   81.64%   -0.01%     
==========================================
  Files        1123     1123              
  Lines      410248   410286      +38     
  Branches   410248   410286      +38     
==========================================
+ Hits       334940   334971      +31     
- Misses      55617    55622       +5     
- Partials    19691    19693       +2     

☔ View full report in Codecov by Harness.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
  • 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

The truncation tests asserted with `contains`, negative checks and a length
ceiling, so they would still pass if the middle of the field list vanished,
punctuation broke, or unrelated text were appended.

Replace them with insta inline snapshots of the complete message for the 3,
20, 21 and 200 field cases, which pins the wording, the separators and the
exact set of listed fields. Also hoist the test module's imports to the top
and drop the explicit `use crate::error::{...}`, which duplicated names
already brought in by `use super::*`.
@akashjainn
akashjainn force-pushed the truncate-valid-fields-error branch from f927543 to 92c606c Compare September 6, 2026 22:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

common Related to common crate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

huge error message when many columns in table

3 participants