sundapeng opened a new pull request, #8903:
URL: https://github.com/apache/paimon/pull/8903
Follow-up to #8861. That PR wrote down a contract — *only entries below the
listed root are judged by the `'_'` / `'.'` rule, never the root itself* — and
this closes the two places where the code does not yet keep it, plus the
coverage hole that let the listing's descent go untested.
### The scan root is still judged during partition discovery
`PartitionPathUtils.listStatusRecursively` applies the rule at every level,
and level 0 is the scan root itself, built by `getFileStatusRecurse` from the
table location:
```java
fileStatus = fileIO.getFileStatus(path); // the table
location
listStatusRecursively(fileIO, fileStatus, 0, expectLevel, ...);
-> if (isHiddenFile(fileStatus, ...)) { return; } // judges the
location's own name
```
A format table at a location ending in `_raw` therefore finds no partitions
at all — **zero splits, no exception, no warning** — while the same directory
reads fine when the table is declared unpartitioned, because that path goes
through `createSplits`. Locations like that are ordinary: migrations, scratch
areas, generated catalog paths. It is the expensive kind of failure, since a
downstream job does not error, it computes on nothing.
The root is where the caller told us to look; only what is found below it
can be a staging tree. The rule now applies from level 1 down, which is what
`FormatTableScan.listDataFiles` already does, so the two halves of the read
path answer the same way.
### The descent had no test
Every test of the new listing covered only its negative half — a hidden
directory is skipped. Mutating `collectDataFiles` so it never descends:
```java
if (child.isDir()) {
continue; // was: directories.add(child.getPath());
}
```
left the whole format-table suite green. The half that returns the files of
an ordinary sub-directory — and the sort merging what different depths found —
was unpinned, although it is the behaviour the previous `fileIO.listFiles(path,
true)` had. A guard widened to "never descend" makes whole subtrees of
committed data disappear from a read, silently.
`testCreateSplitsSkipsStagingFilesInsidePartitions` now stages a committed
file two non-hidden levels below the partition, at the same depth as the staged
one, so the staging directory in the middle of one path is the only thing
separating them.
The failure contract was unpinned for the same reason: with no non-hidden
sub-directory in any fixture, the descent's `catch` never ran, so widening it
from `FileNotFoundException` to `IOException` also left the suite green. Two
tests now separate the halves — a directory that vanishes between its parent's
listing and its own is skipped; one that cannot be read fails loudly.
### Two claims corrected
- **`@throws FileNotFoundException` for a missing `listedRoot`** is the
`FileIO`'s choice, not this method's: `LocalFileIO.listStatus` answers with no
entries for a missing path, so the result is simply empty. The javadoc says
that now instead of promising the throw.
- **`testCreateSplitsKeepsFilesUnderAStagingLikeTableLocation`** claimed a
table directory may sit under a leading-underscore path, but named the listed
root `t` while production only inspects the root's own name — no implementation
could fail it. The root is now `_t`, and its partitioned counterpart is the new
test above.
### Two smaller ones
- `testStagingTreesAreNotEnumerated` measured `FileIO.listFiles(path, true)`
as a baseline, a path planning no longer calls, so no change to the code under
test could turn those three assertions red — and they contradicted the comment
two lines above them. Dropped; the exact counts already carry the claim.
- `warnIfFilesystemPartitionsExist` still excluded only `.`-prefixed
directories, so a leftover staging tree told the reader to run `MSCK REPAIR
TABLE` for data that does not exist. It now uses
`PartitionPathUtils.isHiddenName`.
### Verification
| mutation | before | after |
| --- | --- | --- |
| `collectDataFiles`: never descend | suite green |
`testCreateSplitsSkipsStagingFilesInsidePartitions` fails, both layouts |
| descent `catch (FileNotFoundException)` → `catch (IOException)` | suite
green | `testCreateSplitsFailsWhenASubDirectoryCannotBeListed` fails, both
layouts |
| `level > 0 &&` guard reverted | suite green |
`testReadsAPartitionedTableWhoseOwnDirectoryNameIsHidden` fails, both layouts |
`listStatusRecursively` is private and reached only through
`searchPartSpecAndPaths`, whose production callers are
`FileSystemSplitEnumerator` and the Spark `FormatTablePartitionRepair`
procedure — both format-table paths; managed tables do not use it.
143 tests pass across `FormatTableScanTest`,
`CatalogManagedPartitionScanTest`, `FormatTableCommitTest`,
`PartitionPathUtilsTest`, `FormatTableWriteTest`, `FormatReadBuilderTest`,
`FormatTableCompatibilityTest` and `FormatDataSplitTest`; spotless and
checkstyle clean.
--
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]
