akashjainn opened a new pull request, #24931:
URL: https://github.com/apache/datafusion/pull/24931

   Closes #5332.
   
   ## Which issue does this PR close?
   
   Closes #5332.
   
   ## Rationale for this change
   
   Selecting an unknown column from a wide table lists every field in the 
schema. The issue was filed in 2023 and still reproduces on `main` — on a 200 
column table the message is ~2.8k characters:
   
   ```console
   > CREATE TABLE wide(col_0 INT, ... col_199 INT) AS VALUES (...);
   > 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.      <- 200 names
   ```
   
   Schemas with hundreds or thousands of columns are ordinary in analytics 
workloads, so the useful part of the message — the name that was not found, and 
the `Did you mean` suggestion — gets pushed off screen by the list that follows 
it.
   
   ## What changes are included in this PR?
   
   Cap the listed fields at 20 and summarise the remainder. Same query after:
   
   ```console
   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, so the list stays useful where it 
is short enough to read:
   
   ```console
   > SELECT alpah FROM t;                        -- t(alpha, beta, gamma)
   Error: Schema error: No field named alpah. Did you mean 't.alpha'?
   Valid fields are t.alpha, t.beta, t.gamma.
   ```
   
   That also means the existing message assertions in `dfschema.rs`, 
`column.rs`, `expr_rewriter/mod.rs` and `core/tests/dataframe/mod.rs` are 
untouched — all of those schemas are well under 20 fields.
   
   The issue suggested either dropping the list (as Postgres does) or 
truncating it. I kept it and bounded it: the list is genuinely helpful on 
narrow schemas, and `closest_valid_field` already covers the common typo case, 
so removing it outright would lose more than it gains. Happy to switch to the 
Postgres behaviour if maintainers prefer.
   
   ## Are these changes tested?
   
   Yes — four unit tests in `datafusion/common/src/error.rs`:
   
   | Test | Covers |
   |---|---|
   | `field_not_found_lists_every_field_for_a_narrow_schema` | 3 fields — full 
list, no "other" suffix |
   | `field_not_found_lists_every_field_at_the_cap` | exactly 20 — boundary, 
still a full list |
   | `field_not_found_uses_singular_for_one_extra_field` | 21 — reads "and 1 
other." not "1 others" |
   | `field_not_found_truncates_a_wide_schema` | 200 — "and 180 others.", 
asserts the message stays under 500 chars |
   
   `cargo test -p datafusion-common` — **608 passed, 0 failed**. `cargo test -p 
datafusion-expr --lib` — 258 passed, 0 failed. `cargo fmt` clean.
   
   Verified end to end with `datafusion-cli` built from this branch, output 
shown above.
   
   Two notes on the local run: the `arrow_test_data` doctest fails in my 
checkout because the `testing/` submodule isn't initialised, and `cargo clippy 
-p datafusion-common --all-targets` reports a pre-existing `unused import: 
crate::config::TableParquetOptions`. Both reproduce on unmodified `main`, so 
neither is from this change.
   
   ## Are there any user-facing changes?
   
   Yes — the wording of `SchemaError::FieldNotFound` when a schema has more 
than 20 fields. It gains a trailing `and N others.` instead of the full list. 
No API 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: [email protected]

For queries about this service, please contact Infrastructure at:
[email protected]


---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]

Reply via email to