aakashofficial-k01 opened a new pull request, #4380:
URL: https://github.com/apache/amoro/pull/4380
## Why are the changes needed?
Table locations and file paths in Iceberg are URIs (RFC 3986) where path
components are delimited by `/`. In `amoro-format-iceberg`, three locations
constructed paths using the host OS `File.separator`:
1. `TableFileUtil.getNewFilePath()`
2. `IcebergTableMaintainer.clearInternalTableContentsFiles()`
3. `IcebergTableMaintainer.clearInternalTableMetadata()`
On Windows environments, `File.separator` resolves to `\`. This produces
invalid URIs (such as `hdfs://nn:8020/tbl\00000-0.parquet` or
`s3://bucket/tbl\metadata`), which causes:
- `TableFileUtil.getUriPath()` / `URI.create()` to throw
`IllegalArgumentException: Illegal character in path` during table optimize
commit (`UnKeyedTableCommit.moveTargetFiles()`).
- Cloud object store prefix queries (`pio.listPrefix()`) to treat `\` as a
literal ASCII character, returning 0 files and silently skipping orphan
metadata cleanup.
- Redundant double slashes (`//`) on Linux when a base table location
contains a trailing slash.
Fix #4379.
## Brief change log
- In `TableFileUtil.getNewFilePath()`, use Iceberg's standard
`LocationUtil.stripTrailingSlash(newDirectory)` and format child file paths
with forward slash `/`.
- In `IcebergTableMaintainer`, construct `dataLocation` and
`metadataLocation` using `LocationUtil.stripTrailingSlash(table.location())`
and format with `/` adhering to Iceberg's `DefaultLocationProvider` convention.
- Remove unused `import java.io.File;` in both classes.
- Add parameterized unit tests in `TestFileUtil` verifying `getNewFilePath`
with and without trailing slashes across S3 and HDFS URIs.
## How was this patch tested?
- [x] Added unit tests for `getNewFilePath` in `TestFileUtil.java`.
`./mvnw test -pl amoro-common,amoro-format-iceberg -Dtest=TestFileUtil`:
BUILD SUCCESS (14/14 passed)
- [x] Validated Spotless and Checkstyle:
`./mvnw spotless:check checkstyle:check -pl amoro-format-iceberg`: BUILD
SUCCESS (0 violations)
- [ ] Screenshots are not applicable.
## Documentation
- Does this pull request introduce a new feature? no
- If yes, how is the feature documented? not applicable
--
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]
