Joorgem opened a new pull request, #57658:
URL: https://github.com/apache/spark/pull/57658
### What changes were proposed in this pull request?
Both the CSV and JSON data source option tables describe `PERMISSIVE` mode
with a
character-identical pair of sentences:
> **To keep corrupt records**, an user can set a string type field named
> `columnNameOfCorruptRecord` in an user-defined schema. **If a schema does
not have
> the field, it drops corrupt records during parsing.**
Neither is correct for either data source. `columnNameOfCorruptRecord` does
not
control whether a corrupt record is kept — it is always kept, with its
malformed
fields set to `null`. The field controls whether the *malformed string
itself* is
available.
This PR replaces both sentences in both files:
> To capture the malformed string, an user can set a string type field named
> `columnNameOfCorruptRecord` in an user-defined schema. If a schema does
not have the
> field, the corrupt record is still retained with its malformed fields set
to `null`,
> but the malformed string is not available.
Both sentences move together on purpose. Correcting only the second one
would leave
the paragraph contradicting itself, since the first assigns retention to the
field
while the second would deny retention without it.
### Why are the changes needed?
The sentence is the one a reader consults when deciding whether they need
`columnNameOfCorruptRecord`, and it points them the wrong way. Read
literally, a
schema *without* the field is the safe choice, because bad records disappear
rather
than entering the result. The truth is the opposite: without the field,
malformed
records are still there, indistinguishable from records that were genuinely
null in
the source, and there is no signal that anything was wrong.
That matters most in exactly the situation the docs are meant to protect: a
pipeline whose schema does not declare the field will silently admit partial
records, and no error, count or mode setting reveals it.
### Does this PR introduce _any_ user-facing change?
No. Documentation only; no behaviour change. The behaviour being documented
is
already asserted by the test suite on `master` (see below), so this corrects
the
description rather than describing anything new.
### How was this patch tested?
Documentation-only change, so no tests were added. No new verification is
needed
either: **`master`'s own test suite already asserts the corrected behaviour,
in both
data sources.**
**CSV** — `CSVSuite`'s `SPARK-18699 put malformed records in a
'columnNameOfCorruptRecord' field` reads `test-data/value-malformed.csv`,
whose two
records are `0,2013-111_11 12:13:14` (an unparseable date) and
`1,1983-08-04`. With
schema `a INT, b DATE` and **no** `columnNameOfCorruptRecord` field it
asserts:
```scala
checkAnswer(df1, Row(0, null) :: Row(1, java.sql.Date.valueOf("1983-08-04"))
:: Nil)
```
Two rows. The corrupt record is kept with the unparseable field nulled. Were
the
documented sentence accurate, this assertion would expect one row.
**JSON** — `JsonSuite` is even more direct. The test is named
`Corrupt records: PERMISSIVE mode, without designated column for malformed
records`,
reads the `corruptRecords` fixture with a three-column user-defined schema
and no
corrupt-record column, and asserts five rows — four of them entirely null —
under
Spark's own comment:
```scala
// Corrupted records are replaced with null
Row(null, null, null),
```
Nothing is dropped in either data source, on `master`, by the project's own
assertions.
**Corroborated by direct measurement** across four kinds of bad record on
`pyspark` 3.5.9 — three source records per case, `PERMISSIVE`, user-defined
schema
`a INT, b STRING`:
| format | bad record | rows without the field | rows with the field |
flagged corrupt |
|---|---|---|---|---|
| JSON | not parseable as JSON at all | 3 | 3 | 1 |
| JSON | valid JSON, wrong type for schema | 3 | 3 | 1 |
| CSV | wrong type for schema | 3 | 3 | 1 |
| CSV | unterminated quote | 3 | 3 | 0 |
The counts are identical either way in every case — nothing is dropped.
Declaring
the field adds the ability to see the record, not the decision to keep it.
The JSON rows are worth calling out, since JSON is the data source where the
sentence would be most defensible: a whole JSON record genuinely can be
unparseable, and it is still retained.
Reproduction script: `docs/evidence/c4_repro_json_vs_csv_drop.py` in the
authoring
repo (not part of this patch). Note that this measurement was taken on
3.5.9, not on
`master` — it is corroboration, and the two `master` test assertions above
are the
load-bearing evidence.
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Opus 5)
--
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]
