You are viewing a plain text version of this content. The canonical link for it is here.
Posted to reviews@spark.apache.org by "Joorgem (via GitHub)" <gi...@apache.org> on 2026/07/30 23:53:59 UTC
[PR] [SPARK-58457][SQL][DOCS] Correct the PERMISSIVE mode claim that corrupt records are dropped [spark]
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-58457][SQL][DOCS] Correct the PERMISSIVE mode claim that corrupt records are dropped [spark]
Posted by "Joorgem (via GitHub)" <gi...@apache.org>.
Joorgem commented on PR #57658:
URL: https://github.com/apache/spark/pull/57658#issuecomment-5180571469
Scope note, so this is not a silent expansion: 4a1ad5e adds a second correction to the same bullet, on @szehon-ho's suggestion from #57608.
Alongside the "drops corrupt records" claim this PR already fixes, the bullet also says:
> A record with less/more tokens than schema is not a corrupted record to CSV.
That is false. `UnivocityParser.convert`:
```scala
var badRecordException: Option[Throwable] = if (tokens.length != parsedSchema.length) {
// If the number of tokens doesn't match the schema, we should treat it as a malformed record.
// However, we still have chance to parse some of the tokens. It continues to parses the
// tokens normally and sets null when `ArrayIndexOutOfBoundsException` occurs for missing
// tokens.
```
The record is flagged malformed *and* parsing continues with `null` for the tokens it does not have. So the sentence's second half was accurate and its first half was not, which is likely why it lasted: what a user observes matches the accurate half.
It landed here rather than on #57608 because this PR already edits this bullet, and #57608 would otherwise have shipped a table cell contradicting itself two lines apart. Both changes are now one concern — what `PERMISSIVE` actually does with a corrupt record — in one bullet.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-58457][SQL][DOCS] Correct the PERMISSIVE mode claim that corrupt records are dropped [spark]
Posted by "szehon-ho (via GitHub)" <gi...@apache.org>.
szehon-ho commented on PR #57658:
URL: https://github.com/apache/spark/pull/57658#issuecomment-5259942291
Thanks for the catch — name collision on assignee resolution.
Reassigned [SPARK-58457](https://issues.apache.org/jira/browse/SPARK-58457) from Jorge Machado (`jomach`) to Jorge Molina (`joorgem`), and added `joorgem` to the SPARK Contributors JIRA role so the assignment sticks.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-58457][SQL][DOCS] Correct the PERMISSIVE mode claim that corrupt records are dropped [spark]
Posted by "Joorgem (via GitHub)" <gi...@apache.org>.
Joorgem commented on PR #57658:
URL: https://github.com/apache/spark/pull/57658#issuecomment-5253479539
Thanks for merging this, @szehon-ho.
One thing on the JIRA side: SPARK-58457 was assigned to Jorge Machado (`jomach`) on resolution. I believe that is a name collision — I am Jorge Molina (`joorgem`), the reporter of the issue and the author of the commit. Would you mind reassigning it?
The commit credit is correct, so this is only about the JIRA assignee and the 4.3.0 release credits.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-58457][SQL][DOCS] Correct the PERMISSIVE mode claim that corrupt records are dropped [spark]
Posted by "szehon-ho (via GitHub)" <gi...@apache.org>.
szehon-ho commented on PR #57658:
URL: https://github.com/apache/spark/pull/57658#issuecomment-5249573625
**Merge Summary:**
- merged into master https://github.com/apache/spark/commit/96931993a5bb85edd4d0e26e8f8c43bca8e517e0
- merged into branch-4.x https://github.com/apache/spark/commit/8490cd49e646789060a0dcac64f3b994ca7bd3fb
- merged into branch-4.3 https://github.com/apache/spark/commit/13534826fe2fa9a8a1b89288d93e1fc6617becae
*Posted by `merge_spark_pr.py`*
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-58457][SQL][DOCS] Correct the PERMISSIVE mode claim that corrupt records are dropped [spark]
Posted by "uros-b (via GitHub)" <gi...@apache.org>.
uros-b commented on code in PR #57658:
URL: https://github.com/apache/spark/pull/57658#discussion_r3692731067
##########
docs/sql-data-sources-json.md:
##########
@@ -170,7 +170,7 @@ Data source options of JSON can be set via:
<td><code>PERMISSIVE</code></td>
<td>Allows a mode for dealing with corrupt records during parsing.<br>
<ul>
- <li><code>PERMISSIVE</code>: when it meets a corrupted record, puts the malformed string into a field configured by <code>columnNameOfCorruptRecord</code>, and sets malformed fields to <code>null</code>. To keep corrupt records, an user can set a string type field named <code>columnNameOfCorruptRecord</code> in an user-defined schema. If a schema does not have the field, it drops corrupt records during parsing. When inferring a schema, it implicitly adds a <code>columnNameOfCorruptRecord</code> field in an output schema.</li>
+ <li><code>PERMISSIVE</code>: when it meets a corrupted record, puts the malformed string into a field configured by <code>columnNameOfCorruptRecord</code>, and sets malformed fields to <code>null</code>. To capture the malformed string, an user can set a string type field named <code>columnNameOfCorruptRecord</code> 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 <code>null</code>, but the malformed string is not available. When inferring a schema, it implicitly adds a <code>columnNameOfCorruptRecord</code> field in an output schema.</li>
Review Comment:
```suggestion
<li><code>PERMISSIVE</code>: when it meets a corrupted record, puts the malformed string into a field configured by <code>columnNameOfCorruptRecord</code>, and sets malformed fields to <code>null</code>. To capture the malformed string, a user can set a string type field named <code>columnNameOfCorruptRecord</code> in a user-defined schema. If a schema does not have the field, the corrupt record is still retained with its malformed fields set to <code>null</code>, but the malformed string is not available. When inferring a schema, it implicitly adds a <code>columnNameOfCorruptRecord</code> field in an output schema.</li>
```
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-58457][SQL][DOCS] Correct the PERMISSIVE mode claim that corrupt records are dropped [spark]
Posted by "uros-b (via GitHub)" <gi...@apache.org>.
uros-b commented on code in PR #57658:
URL: https://github.com/apache/spark/pull/57658#discussion_r3692729851
##########
docs/sql-data-sources-csv.md:
##########
@@ -209,7 +209,7 @@ Data source options of CSV can be set via:
<td>PERMISSIVE</td>
<td>Allows a mode for dealing with corrupt records during parsing. It supports the following case-insensitive modes. Note that Spark tries to parse only required columns in CSV under column pruning. Therefore, corrupt records can be different based on required set of fields. This behavior can be controlled by <code>spark.sql.csv.parser.columnPruning.enabled</code> (enabled by default).<br>
<ul>
- <li><code>PERMISSIVE</code>: when it meets a corrupted record, puts the malformed string into a field configured by <code>columnNameOfCorruptRecord</code>, and sets malformed fields to <code>null</code>. To keep corrupt records, an user can set a string type field named <code>columnNameOfCorruptRecord</code> in an user-defined schema. If a schema does not have the field, it drops corrupt records during parsing. A record with less/more tokens than schema is not a corrupted record to CSV. When it meets a record having fewer tokens than the length of the schema, sets <code>null</code> to extra fields. When the record has more tokens than the length of the schema, it drops extra tokens.</li>
+ <li><code>PERMISSIVE</code>: when it meets a corrupted record, puts the malformed string into a field configured by <code>columnNameOfCorruptRecord</code>, and sets malformed fields to <code>null</code>. To capture the malformed string, an user can set a string type field named <code>columnNameOfCorruptRecord</code> 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 <code>null</code>, but the malformed string is not available. A record with less/more tokens than schema is not a corrupted record to CSV. When it meets a record having fewer tokens than the length of the schema, sets <code>null</code> to extra fields. When the record has more tokens than the length of the schema, it drops extra tokens.</li>
Review Comment:
```suggestion
<li><code>PERMISSIVE</code>: when it meets a corrupted record, puts the malformed string into a field configured by <code>columnNameOfCorruptRecord</code>, and sets malformed fields to <code>null</code>. To capture the malformed string, a user can set a string type field named <code>columnNameOfCorruptRecord</code> in a user-defined schema. If a schema does not have the field, the corrupt record is still retained with its malformed fields set to <code>null</code>, but the malformed string is not available. A record with less/more tokens than schema is not a corrupted record to CSV. When it meets a record having fewer tokens than the length of the schema, sets <code>null</code> to extra fields. When the record has more tokens than the length of the schema, it drops extra tokens.</li>
```
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-58457][SQL][DOCS] Correct the PERMISSIVE mode claim that corrupt records are dropped [spark]
Posted by "szehon-ho (via GitHub)" <gi...@apache.org>.
szehon-ho closed pull request #57658: [SPARK-58457][SQL][DOCS] Correct the PERMISSIVE mode claim that corrupt records are dropped
URL: https://github.com/apache/spark/pull/57658
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-58457][SQL][DOCS] Correct the PERMISSIVE mode claim that corrupt records are dropped [spark]
Posted by "Joorgem (via GitHub)" <gi...@apache.org>.
Joorgem commented on PR #57658:
URL: https://github.com/apache/spark/pull/57658#issuecomment-5197117764
Dropped in 3203978 — thanks @szehon-ho. You're right that there was no precedent: that was the only `<em>` in the file, and there is none in either `sql-data-sources-csv.md` or `sql-data-sources-json.md` on master. Plain "is" carries it.
Cross-referencing your other thread: this PR now holds the whole `PERMISSIVE` bullet correction, so if it merges before #57608 that page never contradicts itself. I've said the same over there rather than folding the edit into both, which would conflict whichever landed second.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-58457][SQL][DOCS] Correct the PERMISSIVE mode claim that corrupt records are dropped [spark]
Posted by "Joorgem (via GitHub)" <gi...@apache.org>.
Joorgem commented on PR #57658:
URL: https://github.com/apache/spark/pull/57658#issuecomment-5151977399
Thanks @uros-b — both suggestions applied verbatim in 2baa525.
The `an user` / `an user-defined` wording was inherited from the original line and I had kept it to hold the diff to the one sentence that was wrong. Fixing it is better; it now reads `a user` in both files.
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org
Re: [PR] [SPARK-58457][SQL][DOCS] Correct the PERMISSIVE mode claim that corrupt records are dropped [spark]
Posted by "szehon-ho (via GitHub)" <gi...@apache.org>.
szehon-ho commented on code in PR #57658:
URL: https://github.com/apache/spark/pull/57658#discussion_r3715174164
##########
docs/sql-data-sources-csv.md:
##########
@@ -209,7 +209,7 @@ Data source options of CSV can be set via:
<td>PERMISSIVE</td>
<td>Allows a mode for dealing with corrupt records during parsing. It supports the following case-insensitive modes. Note that Spark tries to parse only required columns in CSV under column pruning. Therefore, corrupt records can be different based on required set of fields. This behavior can be controlled by <code>spark.sql.csv.parser.columnPruning.enabled</code> (enabled by default).<br>
<ul>
- <li><code>PERMISSIVE</code>: when it meets a corrupted record, puts the malformed string into a field configured by <code>columnNameOfCorruptRecord</code>, and sets malformed fields to <code>null</code>. To keep corrupt records, an user can set a string type field named <code>columnNameOfCorruptRecord</code> in an user-defined schema. If a schema does not have the field, it drops corrupt records during parsing. A record with less/more tokens than schema is not a corrupted record to CSV. When it meets a record having fewer tokens than the length of the schema, sets <code>null</code> to extra fields. When the record has more tokens than the length of the schema, it drops extra tokens.</li>
+ <li><code>PERMISSIVE</code>: when it meets a corrupted record, puts the malformed string into a field configured by <code>columnNameOfCorruptRecord</code>, and sets malformed fields to <code>null</code>. To capture the malformed string, a user can set a string type field named <code>columnNameOfCorruptRecord</code> in a user-defined schema. If a schema does not have the field, the corrupt record is still retained with its malformed fields set to <code>null</code>, but the malformed string is not available. A record with a different number of tokens than the schema <em>is</em> a corrupted record, and is handled by this <code>mode</code> like any other; it is still parsed as far as it can be, with <code>null</code> set for tokens the record does not have and extra tokens dropped.</li>
Review Comment:
do we need the <em> in the doc? seems a bit unnecessary here and no precedent in this doc
##########
docs/sql-data-sources-csv.md:
##########
@@ -209,7 +209,7 @@ Data source options of CSV can be set via:
<td>PERMISSIVE</td>
<td>Allows a mode for dealing with corrupt records during parsing. It supports the following case-insensitive modes. Note that Spark tries to parse only required columns in CSV under column pruning. Therefore, corrupt records can be different based on required set of fields. This behavior can be controlled by <code>spark.sql.csv.parser.columnPruning.enabled</code> (enabled by default).<br>
<ul>
- <li><code>PERMISSIVE</code>: when it meets a corrupted record, puts the malformed string into a field configured by <code>columnNameOfCorruptRecord</code>, and sets malformed fields to <code>null</code>. To keep corrupt records, an user can set a string type field named <code>columnNameOfCorruptRecord</code> in an user-defined schema. If a schema does not have the field, it drops corrupt records during parsing. A record with less/more tokens than schema is not a corrupted record to CSV. When it meets a record having fewer tokens than the length of the schema, sets <code>null</code> to extra fields. When the record has more tokens than the length of the schema, it drops extra tokens.</li>
+ <li><code>PERMISSIVE</code>: when it meets a corrupted record, puts the malformed string into a field configured by <code>columnNameOfCorruptRecord</code>, and sets malformed fields to <code>null</code>. To capture the malformed string, a user can set a string type field named <code>columnNameOfCorruptRecord</code> in a user-defined schema. If a schema does not have the field, the corrupt record is still retained with its malformed fields set to <code>null</code>, but the malformed string is not available. A record with a different number of tokens than the schema <em>is</em> a corrupted record, and is handled by this <code>mode</code> like any other; it is still parsed as far as it can be, with <code>null</code> set for tokens the record does not have and extra tokens dropped.</li>
Review Comment:
do we need the `<em>` in the doc? seems a bit unnecessary here and no precedent in this doc
--
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: reviews-unsubscribe@spark.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
---------------------------------------------------------------------
To unsubscribe, e-mail: reviews-unsubscribe@spark.apache.org
For additional commands, e-mail: reviews-help@spark.apache.org