JingsongLi commented on code in PR #975:
URL: https://github.com/apache/paimon-rust/pull/975#discussion_r4172569053
##########
crates/paimon/src/arrow/format/text.rs:
##########
@@ -842,20 +842,23 @@ fn decode_text_chunk(
message: format!("Invalid UTF-8 in CSV file: {e}"),
source: Some(Box::new(e)),
})?;
- let row = if line.trim().is_empty() {
+ let mut row = if line.trim().is_empty() {
vec![None; schema.fields().len()]
} else {
parse_csv_line(line, options)?
};
- if row.len() != schema.fields().len() {
- return Err(Error::DataInvalid {
- message: format!(
- "CSV row has {} fields, expected {}",
- row.len(),
- schema.fields().len()
- ),
- source: None,
- });
+ // Read permissively, matching Java `CsvParser` under the
default
+ // `csv.mode=permissive`: a row with fewer fields than the
schema
+ // pads the missing trailing columns with null, and a row
with more
+ // fields drops the extras. Failing the whole scan on a
field-count
+ // mismatch rejected files Java reads fine — e.g. reading
a table
+ // after ADD COLUMN (old files carry one fewer column), or
any
+ // externally produced CSV that omits an optional trailing
column.
+ let field_count = schema.fields().len();
+ if row.len() < field_count {
+ row.resize(field_count, None);
Review Comment:
[P2] Guard nontrailing CSV additions before padding old rows
The new DROP-only guard leaves positional ADD COLUMN accepted. With a real
CSV format table `(id BIGINT, label VARCHAR)` containing `1,old`,
`Catalog::alter_table` accepts
`SchemaChange::add_column_with_description_and_column_move("extra", VARCHAR,
"", ColumnMove::move_after("extra", "id"))`. Reading the existing file with
SQLContext then succeeds with `(id, extra, label) = (1, "old", NULL)`; `WHERE
label = 'old'` returns no rows. The new padding changes the old width-mismatch
error into a successful but incorrect read. Format-table scans use the current
physical field order, without historical position mapping. Please also reject
additions that shift existing CSV physical columns (including ADD followed by a
position change), or map the old positions before permissive decoding. The
same-width reorder issue already existed and is not a separate finding here.
--
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]
