sunchao commented on code in PR #5825:
URL: https://github.com/apache/datafusion-comet/pull/5825#discussion_r3975237725
##########
native/core/src/parquet/parquet_support.rs:
##########
@@ -665,8 +667,13 @@ pub(crate) fn prepare_object_store_with_configs(
url: String,
object_store_configs: &HashMap<String, String>,
) -> Result<(ObjectStoreUrl, Path), ExecutionError> {
- let url = normalize_object_store_url(url.as_str(), object_store_configs)?;
- let is_hdfs_scheme = is_hdfs_scheme(&url, object_store_configs);
+ // `is_hdfs` comes back from normalization because it must be decided on
the URL as written.
+ // Re-deriving it from the normalized URL would let an `s3a`/alias rewrite
land on an `s3`
+ // entry in `fs.comet.libhdfs.schemes` and route an S3 read through
libhdfs.
+ let NormalizedObjectStoreUrl {
+ url,
+ is_hdfs: is_hdfs_scheme,
Review Comment:
**[P2] Keep different backend selections isolated**
Preserving the original routing decision looks right, but could we
coordinate this with the backend/cache/registry isolation in #5503 before
merging? With the same `fs.comet.libhdfs.schemes=s3` configuration, this
sequence still selects the wrong backend:
1. Prepare `s3a://bucket/file`. It now correctly selects native S3 and
caches that store under `s3://bucket`.
2. Prepare `s3://bucket/file`. It correctly computes `is_hdfs=true`, but the
identical `(url_key, config_hash)` hits the existing native S3 store and
bypasses backend construction.
In the reverse order, the `s3a` request inherits the cached HDFS store. The
newly preserved flag can distinguish these requests, but the cache key cannot.
Backend-aware cache keys alone also leave the DataFusion registry collision
when both stores are registered in the same runtime, so #5503's registration
isolation matters too.
Could we add a regression that prepares both schemes for the same bucket
with an unchanged configuration, in both orders? The new dispatch test changes
the configuration to `libhdfs.schemes=s3a` before its final assertion, which
changes the cache key and misses this case.
--
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]
