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:
   **Withdrawn:** I’m retracting the P2 classification and the request to 
coordinate these fixes before merging.
   
   The cache/registry isolation work is already covered by #5503. It can land 
after #5825 as long as both fixes are included before the next Comet release. 
This does not require combining the PRs or holding #5825 for #5503.
   
   My original comment applied a release-readiness requirement to an 
intermediate state on `main`.



-- 
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]

Reply via email to