Joorgem opened a new pull request, #57769:
URL: https://github.com/apache/spark/pull/57769
### What changes were proposed in this pull request?
`DataSource.checkAndGlobPathIfNecessary` runs its lambda once per
glob-looking path, but the `enableGlobbing == false` branch returned
`qualifiedPaths` — the whole input list:
```scala
ThreadUtils.parmap(globPaths, "globPath", numThreads) { globPath =>
val fs = globPath.getFileSystem(hadoopConf)
val globResult = if (enableGlobbing) {
SparkHadoopUtil.get.globPath(fs, globPath)
} else {
qualifiedPaths // <-- the whole list, once per glob-looking path
}
```
The branch now returns `Seq(globPath)`, the path it was handed.
### Why are the changes needed?
**The query returns duplicate rows.** With `G` paths that look like globs
and `n` that do not, the result held `G*(G+n)+n` entries instead of `G+n`.
Those duplicates become the file index's `rootPaths`, and for an unpartitioned
relation `PartitioningAwareFileIndex.allFiles()` does `rootPaths.flatMap`, so
the same `FileStatus` is returned several times and `FileScanRDD` reads the
same file repeatedly.
Three single-row files, one of them named `weird_[x].csv`:
```
spark.read.options(**{"__globPaths__": "false"}).csv(paths).count()
# 5 <-- three files, five rows
# x2 ('0', 'row_from_plain_a.csv') DUPLICATED
# x2 ('1', 'row_from_plain_b.csv') DUPLICATED
# x1 ('2', 'row_from_weird_[x].csv')
```
"Looks like a glob" is a bare character scan with no notion of escaping
(`SparkHadoopUtil.isGlobPath` tests for any of `{}[]*?\`), so the branch is
reached by ordinary data. Filenames containing brackets are legal on HDFS and
S3 — and SPARK-32810 and SPARK-32815 exist precisely because the project
decided such names must work.
**The highest-impact caller is the Structured Streaming file source**, which
sets `GLOB_PATHS_KEY -> "false"` unconditionally and hands the whole
microbatch's file list to a `DataSource`. So a microbatch containing one file
whose *name* holds a metacharacter emits every other file in that batch more
than once. `MLUtils.parseLibSVMFile` and the `TextInput*.infer`
schema-inference paths pass multiple paths with globbing disabled too.
The diff is one expression, so it is worth being explicit that the *finding*
is silent data duplication rather than a style change.
### Does this PR introduce _any_ user-facing change?
Yes, and only in the direction of correctness: affected reads return the
right number of rows instead of a larger one, and do less I/O.
No behaviour changes for the case the option was introduced to serve. With a
single path, `qualifiedPaths` and `Seq(globPath)` are the same value, so every
scenario SPARK-32810 fixed and tested behaves identically. `globResult` stays
non-empty, so the `checkEmptyGlobPath` check cannot begin raising
`PATH_NOT_FOUND` where it did not before.
### How was this patch tested?
Two tests added to `DataSourceSuite`, and **verified to fail on unmodified
`master` before the fix was applied** — the fork CI run of the test alone
reported `Tests: succeeded 21345, failed 2`, the two failures being exactly
these, with the duplication visible in the messages:
```
- SPARK-58518: checkAndGlobPathIfNecessary must not duplicate paths when
globbing is disabled *** FAILED ***
List(mockFs://mockFs/somepath1, mockFs://mockFs/somepath2,
mockFs://mockFs/globpath1*,
mockFs://mockFs/somepath1, mockFs://mockFs/somepath2)
- SPARK-58518: a filename with a glob metacharacter must not duplicate rows
when globbing is disabled *** FAILED ***
Array("row_a", "row_a", "row_b", "row_b", "row_c") did not equal
Array("row_a", "row_b", "row_c")
```
With the fix, the full matrix is green.
**Both tests assert on ordered sequences rather than sets, and that is
deliberate.** Every existing assertion in this suite compares
`resultPaths.toSet` against a `Set` — and a `Set` is exactly what erases
duplication. Together with the fact that all six existing
`checkAndGlobPathIfNecessary` tests pass `enableGlobbing = true`, that is why
this went unnoticed: the branch was never exercised, and the suite's assertion
style could not have caught it if it had been.
The end-to-end test uses `text` rather than CSV, since the defect is in
`DataSource` and is format-agnostic. Verified by hand in `text`, `json`, `csv`
and `parquet`, all returning five rows for three files.
The predicted count holds exactly, which is what distinguishes a diagnosis
from an observation:
| G | n | files | predicted `G*(G+n)+n` | observed |
|---|---|---|---|---|
| 1 | 0 | 1 | 1 | 1 |
| 1 | 2 | 3 | 5 | 5 |
| 2 | 0 | 2 | 4 | 4 |
| 2 | 2 | 4 | 10 | 10 |
| 3 | 1 | 4 | 13 | 13 |
The `G=1, n=0` row is also the answer to why this survived since 2020: every
test SPARK-32810 added reads a single path, and that is the one case that comes
back correct.
**Scope, stated honestly:** the correctness impact needs an unpartitioned
relation. When partition columns are discovered, `allFiles()` takes a branch
returning a `Map`'s values, which absorbs the duplicates — measured on the same
data, 15 rows without `basePath` against 9 with it. On partitioned relations
this remains wasted listing work rather than wrong results.
### Was this patch authored or co-authored using generative AI tooling?
Generated-by: Claude Code (Opus 5)
### Related
- SPARK-32810 (#29659) introduced this branch; SPARK-32815 covers LibSVM
with such filenames. Neither reports this defect.
- SPARK-28266 also produced duplicate rows, but from a Hive serde `path`
property repeating the table LOCATION, and was fixed in #33328 — nowhere near
this method. It does establish that duplicated input paths are treated as a
correctness bug.
- No JIRA matches this method plus duplication, and no PR mentions
`enableGlobbing` apart from #29659.
--
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]
