Repository navigation
Incorrect Schema Adaption for CSV #4918
Description
Describe the bug
The schema adaption logic added in #1709 misbehaves for CSV data. In particular it incorrectly assumes that it can create a schema for the entire dataset that is a superset of those of the individual files, and that the CSV reader will pad any missing columns with nulls, and reorder those that appear in a different order.
In reality the CSV reader does not handle missing or reordered columns, it only accidentally works when the file schema happens to be an exact prefix of the aggregate schema. This was relying on an accidental quirk of the arrow reader prior to 30.0.0, after this the arrow reader returns an error as the schema does not match.
To Reproduce
Both of the following tests fail on current master
#[tokio::test]
async fn csv_schema_reordered() -> Result<()> {
use object_store::path::Path;
let session_ctx = SessionContext::new();
let store = InMemory::new();
let data = bytes::Bytes::from("a,b\n1,2\n3,4");
store.put(&Path::from("a.csv"), data).await.unwrap();
let data = bytes::Bytes::from("b,a\n1,2\n3,4");
store.put(&Path::from("b.csv"), data).await.unwrap();
session_ctx
.runtime_env()
.register_object_store("memory", "", Arc::new(store));
let df = session_ctx
.read_csv("memory:///", CsvReadOptions::new())
.await
.unwrap();
let result = df.collect().await.unwrap();
let expected = vec![
"+---+---+",
"| a | b |",
"+---+---+",
"| 1 | 2 |",
"| 2 | 1 |",
"| 3 | 4 |",
"| 4 | 3 |",
"+---+---+",
];
crate::assert_batches_eq!(expected, &result);
Ok(())
}
#[tokio::test]
async fn csv_schema_extra_column() -> Result<()> {
use object_store::path::Path;
let session_ctx = SessionContext::new();
let store = InMemory::new();
let data = bytes::Bytes::from("a,b\n1,2\n3,4");
store.put(&Path::from("a.csv"), data).await.unwrap();
let data = bytes::Bytes::from("a,c\n5,6\n7,8");
store.put(&Path::from("b.csv"), data).await.unwrap();
session_ctx
.runtime_env()
.register_object_store("memory", "", Arc::new(store));
let df = session_ctx
.read_csv("memory:///", CsvReadOptions::new())
.await
.unwrap();
let result = df.collect().await.unwrap();
let expected = vec![
"+---+---+---+",
"| a | b | c |",
"+---+---+---+",
"| 1 | 2 | |",
"| 3 | 4 | |",
"| 5 | | 6 |",
"| 7 | | 8 |",
"+---+---+---+",
];
crate::assert_batches_eq!(expected, &result);
Ok(())
}
Expected behavior
I think both of the following would be valid:
- Don't perform schema adaption for CSV, as they aren't a self-describing format like JSON or parquet, instead returning an error if the schema don't match
- Correctly infer the schema on a per-file basis, and use this when reading
Additional context
#4818 (comment)
Activity
alamb commentedon Jan 15, 2023
That memory store is quite clever
comphead commentedon Jan 24, 2023
Sorry @tustvold still not very clear, for tests above do you expect them to pass?
tustvold commentedon Jan 24, 2023
It probably should error, I'm not sure that schema adaption for CSV makes all that much sense
alamb commentedon Jan 25, 2023
I think it would be valuable to support reading several CSV files from a directory as long as their schema's are compatible, even if they all did not have exactly the same schema
tustvold commentedon Jan 25, 2023
Provided they have headers, this should be possible, all bets are off if any files lack headers though...
That being said, this would require fairly extensive reworking on the schema adaption mechanism, as it currently does not infer per-file schemas.
My vote would be to fix the silent data corruption, which is what currently happens, by returning an error if the schema aren't identical, and then potentially look into supporting more sophisticated schema management down the line if there is interest.
alamb commentedon Jan 25, 2023
I agree this sounds like the best plan to me
Tim-53 commentedon Oct 27, 2026
I tried reproducing this on current main (v55.1.0), and the issue still exists.
With an explicit schema, both reordered headers (
a,bvsb,a) and different column names (a,bvsa,c) silently produce incorrect results.I also tested whether schema inference behaves correctly here. Reordered headers still produce incorrect results, while different column names result in a merged schema (
a,c,b) but fail during execution withexpected 3 got 2.arrow-rs recently added
with_header_validation()in apache/arrow-rs#10144.Enabling it in CsvSource::builder() catches both cases that silently produce incorrect results, including the reordered-header case with schema inference, and produces explicit header mismatch errors instead:
The inferred-schema case with different column names still fails with the field-count error, as before.
This seems like a fairly simple way to prevent silent incorrect results. The question is whether validation should be enabled by default (possibly with an opt-out for positional mapping), or exposed as an opt-in option.
Supporting reordered or missing columns by name would be another possibility, but that seems like a larger change.
Any thoughts on which direction makes more sense?