dwsmith1983 opened a new pull request, #6065: URL: https://github.com/apache/datafusion-comet/pull/6065
## Which issue does this PR close? Closes #5541. ## Rationale for this change The JVM validator for native Iceberg scans checked file schemes against a hand-written set that disagreed with the native storage factory, so a table under `gcs`, `abfs`, `abfss`, `wasb` or `wasbs` was claimed and every task died with "Unsupported storage scheme", while `oss` worked natively and was rejected. #5314 aligned the read list, but three lists still exist by hand: the read gate, the write gate and the native factory, with a comment telling contributors to update the JVM when the native arms change. ## What changes are included in this PR? - Native `builtin_storage_schemes(access_mode)` becomes the single point of change: `storage_factory_for` rejects any scheme that is neither listed for the access mode nor an opted-in S3-compliant alias before matching an arm, so the `oss` arm no longer carries its own write rejection and `memory` no longer needs one. The list is exposed over JNI as `NativeBase.icebergStorageSchemes(forWrite)`, the same pattern as the existing `isObjectStoreSchemeSupported` probe. Opt-in aliases stay JVM-side and additive, as before. - A new `IcebergStorageSchemes` object loads the read and write sets lazily, once per JVM, from that probe. It consults `NativeBase.isLoaded` first and returns the previous constants only when the library is not loaded; every caller sits behind `isCometLoaded`, so that fallback is only consulted in a JVM where nothing runs natively. A native fault while answering the probe propagates instead of being swallowed. `CometScanRule.icebergReadableSchemes` and `CometIcebergNativeWrite.SupportedStorageSchemes` are lazy views of it, so constructing the extension no longer touches the native library. - `memory` is not admitted for reads, on either side. The native `memory` arm builds a fresh, empty in-process OpenDAL store per `FileIO`; it exists for the write path, which assembles manifest bytes in it, and a `memory:` read can never find a table. Main declined it at plan time; this PR keeps that, and the write gate keeps admitting it. - The JVM gates now match the built-in set verbatim instead of lowercasing the scheme. OpenDAL strips the scheme prefix from every path case-sensitively at open time (`S3://bucket/key` fails its `s3://bucket/` prefix check before any request is made), so a `S3://` location the JVM admitted was failing natively. Both gates decline it now; the opt-in alias list is still matched case-insensitively, as native does. ## How are these changes tested? Rust tests in `iceberg_common.rs` check that every listed scheme builds a factory for its mode, that the pre-check is load-bearing (`oss` reads but is rejected for writes purely because it is absent from the write list, and `memory` the other way round), that `hdfs`, `abfs`, `abfss`, `wasb`, `wasbs`, `gcs`, `http`, `https` and `azure` are rejected and unlisted, and that mixed-case schemes are rejected for both modes. They do not prove that the JVM declines everything native rejects; the pre-check makes the list the only thing native consults, which is what closes that direction. `CometScanSchemeFallbackSuite` unit-tests the JNI list parser, calls the JNI probe directly and asserts the result equals the fallback constants, pins the lazily loaded sets to the same constants, and asserts the scan gate admits `file`, `s3`, `s3a`, `gs` and `oss` while declining `memory`, `S3://`, `File://`, `http(s)`, `abfs(s)` and `wasb(s)`. The `wasb` test is a regression guard rather than proof of the fix (main already declined `wasb`): it backs the scheme with a local filesystem, runs the INSERT under plan capture and asserts no `CometIcebergWriteExec` with the reason `unsupported storage scheme: wasb`, which gives the write gate its only end-to-end coverage, then asserts the scan is declined with a reason naming the scheme, that nothing but the scheme caused the fallback, and that the answer matches Spark. `CometIcebergWriteDetectionSuite` gains a case for a `S3://` data location, which must be declined. Run locally with Spark 3.5 / Java 17: - `cargo test -p datafusion-comet --lib iceberg_common`: 9 passed. - `./mvnw -Pspark-3.5 test -DwildcardSuites=org.apache.comet.rules.CometScanSchemeFallbackSuite,org.apache.comet.CometIcebergNativeSuite,org.apache.comet.CometIcebergWriteDetectionSuite`: `CometScanSchemeFallbackSuite` 12 succeeded, `CometIcebergWriteDetectionSuite` 53 succeeded, `CometIcebergNativeSuite` 99 succeeded and 8 canceled (the pre-existing Iceberg 1.11+ and Spark 4.0+ gates); 0 failed. - `cargo clippy --all-targets -- -D warnings`, `cargo fmt`, `spotless:check`, `scalafix` CHECK, `dev/ci/check-suites.py`: 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] --------------------------------------------------------------------- To unsubscribe, e-mail: [email protected] For additional commands, e-mail: [email protected]
