sunchao commented on code in PR #5872:
URL: https://github.com/apache/datafusion-comet/pull/5872#discussion_r4104569400
##########
native/core/src/parquet/objectstore/s3.rs:
##########
@@ -239,24 +240,33 @@ fn extract_s3_config_options(
s3_configs.insert(AmazonS3ConfigKey::Region, region.to_string());
}
- // Extract and handle path style access (virtual hosted style)
- let mut virtual_hosted_style_request = false;
- if let Some(path_style) = get_config_trimmed(configs, bucket,
"path.style.access") {
- virtual_hosted_style_request = path_style.to_lowercase() == "true";
- s3_configs.insert(
- AmazonS3ConfigKey::VirtualHostedStyleRequest,
- virtual_hosted_style_request.to_string(),
- );
- }
-
- // Extract endpoint configuration and modify if virtual hosted style is
enabled
- if let Some(endpoint) = get_config_trimmed(configs, bucket, "endpoint") {
- let normalized_endpoint =
- normalize_endpoint(endpoint, bucket, virtual_hosted_style_request);
- if let Some(endpoint) = normalized_endpoint {
- s3_configs.insert(AmazonS3ConfigKey::Endpoint, endpoint);
+ // Hadoop defaults fs.s3a.path.style.access to false, which means
virtual-hosted addressing,
+ // and treats non-boolean text as that default. object_store expects the
inverse flag.
+ let path_style_access = get_config_trimmed(configs, bucket,
"path.style.access")
+ .is_some_and(|value| value.eq_ignore_ascii_case("true"));
+ let mut virtual_hosted_style_request = !path_style_access;
Review Comment:
[P2] Preserve path-style addressing for legacy mixed-case bucket names. For
an existing US East bucket named `LegacyBucket`, scanning
`s3a://LegacyBucket/object` with `fs.s3a.endpoint.region=us-east-1`, no custom
endpoint and `path.style.access` unset now enables virtual hosting. The later
guard only rejects dotted names. Base and Hadoop’s AWS SDK retain
`https://s3.us-east-1.amazonaws.com/LegacyBucket/object`, but head produces
`https://legacybucket.s3.us-east-1.amazonaws.com/object`. Hostname
canonicalization changes the bucket being addressed, breaking previously valid
reads. AWS supports these pre-March-2018 bucket names. Could we apply the SDK’s
DNS bucket-name eligibility rules independently of the HTTPS dotted-name rule
and add a final-URL regression test?
Evidence: An isolated Rust probe extracted the configuration functions
verbatim from the requested base and head, then passed their output through
locked `object_store 0.13.2` and `url 2.5.8`. Offline signing with synthetic
credentials produced
base=`https://s3.us-east-1.amazonaws.com/LegacyBucket/object` and
head=`https://legacybucket.s3.us-east-1.amazonaws.com/object`. No storage
requests were sent. Probe and output: `/tmp/comet-5872-probe/src/main.rs`,
`/tmp/comet-5872-probe.log`. An offline AWS Java SDK 1.12.780 check retained
the uppercase path. Source inspection confirmed the same eligibility rejection
in Hadoop 3.3.4’s SDK 1.12.262 `S3RequestEndpointResolver`/`BucketNameUtils`
and SDK 2.29.52’s `IsVirtualHostableS3Bucket`. Hadoop’s URI handling preserves
the bucket’s 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]
