Skip to content

fix: decide libhdfs routing from the scheme as written - #5825

Merged
comphead merged 1 commit into
apache:mainfrom
comphead:fix/s3a-libhdfs-scheme-as-written
Sep 10, 2026
Merged

fix: decide libhdfs routing from the scheme as written#5825
comphead merged 1 commit into
apache:mainfrom
comphead:fix/s3a-libhdfs-scheme-as-written

Conversation

@comphead

Copy link
Copy Markdown
Contributor

Which issue does this PR close?

Closes #5816

Rationale for this change

fs.comet.libhdfs.schemes is matched against the scheme after the s3a/alias rewrite, so listing s3 also routes s3a:// reads through libhdfs.

prepare_object_store_with_configs reads:

let url = normalize_object_store_url(url.as_str(), object_store_configs)?;
let is_hdfs_scheme = is_hdfs_scheme(&url, object_store_configs);

normalize_object_store_url guards its own early return with is_hdfs_scheme, so an s3a:// URL whose scheme is not in the list falls through to rewrite_alias_to_s3 and comes back as s3://. The next line then sees s3, matches the list, and selects the libhdfs backend with a name node of s3://bucket. Schemes opted in via fs.comet.s3Compliant.schemes normalize onto s3 too, so they hit the same trap.

Before #5314 the flag was computed on the original URL and the rewrite was guarded by it, so the rewrite could not flip the decision. #5314 replaced that inline rewrite with a call to normalize_object_store_url and moved the check after it.

The JVM gate matches fs.comet.libhdfs.schemes against the scheme the user wrote (CometScanRule.classifyRootPaths) and so admits these scans as object_store-native, which means the recompute also desyncs the planner from the executor.

Only an asymmetric list triggers it. Listing both s3 and s3a, or leaving the config unset (default hdfs), behaves identically before and after.

What changes are included in this PR?

  • normalize_object_store_url returns NormalizedObjectStoreUrl { url, is_hdfs }, carrying the libhdfs decision it already takes on the URL as written. Callers read the flag rather than re-deriving it from the rewritten URL, which makes the recompute structurally impossible instead of merely discouraged.
  • prepare_object_store_with_configs destructures that flag.
  • The object-key call site in planner.rs takes .url.

No behavior change for any other configuration.

How are these changes tested?

Two new Rust unit tests:

  • test_libhdfs_routing_uses_the_scheme_as_written in s3_blob_fs_support.rs covers s3a and an opted-in blob alias under fs.comet.libhdfs.schemes=s3 (must not route to libhdfs), the legitimate fs.comet.libhdfs.schemes=s3a opt-in (must still route there, with the URL left unrewritten), and the unset default. It also pins the trap explicitly: is_hdfs_scheme on the normalized URL still returns true, which is why the flag is carried.
  • test_prepare_object_store_keeps_s3a_off_libhdfs_when_only_s3_is_listed in parquet_support.rs covers the real dispatch, asserting an S3 store is built for both spellings and that an explicitly listed s3a still reaches the libhdfs backend.

Verified as regression guards: with the defect reintroduced, both fail on the exact assertion, while the five pre-existing tests in the same two modules pass, confirming the gap was previously uncovered.

The full datafusion-comet lib suite passes under both feature configurations, 337 tests with --no-default-features and 334 with default features (hdfs-opendal). cargo fmt --check and cargo clippy --all-targets -- -D warnings are clean under both.

Per #5816, the isolates_backends_even_when_s3_alias_and_configs_match test in #5503 covers this same pair and was failing on merge with main. This should unblock it.

`prepare_object_store_with_configs` computed `is_hdfs_scheme` on the URL
returned by `normalize_object_store_url`, that is, after the `s3a`/alias
rewrite to `s3://`. With `fs.comet.libhdfs.schemes=s3` -- which asks for
`s3://` to go through libhdfs and says nothing about `s3a` -- an `s3a://` scan
normalized to `s3://` and was then captured by the libhdfs list on the second
look, handing an S3 read to `create_hdfs_object_store` with a name node of
`s3://bucket`. Schemes opted in via `fs.comet.s3Compliant.schemes` normalize
onto `s3` too, so they hit the same trap.

Before apache#5314 the flag was computed on the original URL and the rewrite was
guarded by it, so the rewrite could not flip the decision. apache#5314 replaced that
inline rewrite with a call to `normalize_object_store_url` and moved the check
after it.

The JVM gate matches `fs.comet.libhdfs.schemes` against the scheme the user
wrote (`CometScanRule.classifyRootPaths`), so it classifies such roots as
object_store-native and admits the scan. The recompute therefore also desynced
the planner from the executor.

`normalize_object_store_url` now returns `NormalizedObjectStoreUrl`, carrying
the libhdfs decision it already takes on the URL as written, so callers cannot
re-derive it from the rewritten URL.

Closes apache#5816
@github-actions github-actions Bot added bug Something isn't working area:scan Parquet scan / data reading labels Sep 10, 2026
Comment thread native/core/src/parquet/parquet_support.rs

@sunchao sunchao left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. The routing decision now stays tied to the original scheme.

@comphead
comphead merged commit 2d3eca2 into apache:main Sep 10, 2026
79 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:scan Parquet scan / data reading bug Something isn't working

Projects

None yet

Development

Successfully merging this pull request may close these issues.

fs.comet.libhdfs.schemes is matched after the s3a alias rewrite, so listing s3 also routes s3a through libhdfs

2 participants