fix: honor the S3 profile name and file and Hadoop's addressing mode for custom endpoints

[dwsmith1983]

Which issue does this PR close?

Closes #4245, closes #2802.

Rationale for this change

Two gaps in how native scans use fs.s3a.* settings.

Comet rejected Hadoop's org.apache.hadoop.fs.s3a.auth.ProfileAWSCredentialsProvider with Unsupported credential provider, so a Spark 4.1 job that selects a profile with fs.s3a.auth.profile.name or fs.s3a.auth.profile.file (#4245) failed in the native scan.

fs.s3a.path.style.access was applied inverted: true set object_store's virtual_hosted_style_request to true and then appended /bucket to the endpoint, which object_store, treating a virtual-hosted endpoint as already containing the bucket, sent as a path-style URL anyway. The net effect was that every custom endpoint was addressed path-style whatever the flag said, and virtual-hosted addressing (bucket.host) was never produced (#2802).

What changes are included in this PR?

• When a bucket's fs.s3a.aws.credentials.provider list names ProfileAWSCredentialsProvider and no Comet credential provider class is set, the native store gets its credentials from the built-in HadoopS3ACredentialProviderAdapter. The adapter builds Hadoop's own provider list on the executor, so the profile name and file, the default credentials path, role chains, credential_process, credential_source, STS regions and endpoints, and the list's fall-through all come from Hadoop and the Java SDK. A configured Comet class still wins, and a blank one still opts out. The AWS SDK ProfileCredentialsProvider names are unchanged; their differences from Hadoop are tracked in Native scans resolve the AWS SDK ProfileCredentialsProvider names differently from Hadoop #6575.
• Why the adapter rather than a native implementation: matching the Java SDK's profile resolution in Rust took about 1,500 lines of role-chain code and tests in s3.rs, and checking it against the real Hadoop 3.4.2 provider over 63 profile fixtures still found 22 differences (credential precedence in source profiles, STS region and endpoint selection, cycle handling, SSO with roles, the list's error handling). Routing to Hadoop's provider removes that whole class of drift. The native side stays independent of the Hadoop version: it only matches the class name, and the class itself first shipped in Hadoop 3.4.2, which Spark needs to load anyway to list the files before the scan runs.
• path.style.access is parsed the way Hadoop's Configuration.getBoolean parses it (default false, non-boolean text falls back to the default), virtual_hosted_style_request is its negation and is always passed, and normalize_endpoint returns the endpoint together with the effective mode so the two cannot disagree. Virtual-hosted rebuilds scheme://bucket.host[:port][/path]; path-style leaves the endpoint alone for object_store to append the bucket. An IP-literal host, or a bucket name the AWS SDK cannot virtual-host (including dotted names over HTTPS), stays path-style, so http://127.0.0.1:9000 keeps working without the flag. localhost is not special-cased, matching Hadoop. The s3.amazonaws.com skip is unchanged.
• The default hadoop-aws.version is now 3.4.2, matching what Spark 4.1 ships, so ProfileAWSCredentialsProvider is on the test classpath; spark-4.0 and spark-4.2 pin 3.4.1. This is its own commit and can be reverted on its own.
• Docs: the data sources page and the S3 credential provider guide describe the routing, its requirements and the opt-out; the migration guide has an addressing entry under Comet 1.2.0.

Requirements and limits of the adapter route, also in the docs:

• hadoop-aws and the AWS SDK must be visible to Comet's class loader, as for the other built-in adapters. With Comet on extraClassPath and hadoop-aws only through --packages, the scan fails with NoClassDefFoundError.
• Static keys from a profile carry no expiry, so each S3 request asks the provider through JNI; role and SSO profiles are cached until shortly before they expire.
• S3A delegation-token clusters are refused by the adapter, and AssumedRoleCredentialProvider with the profile provider as its base is not routed automatically (set the adapter class explicitly).

Behavior change: Comet's native Parquet and CSV scans now address S3 the way Hadoop S3A does; 1.1.0 and earlier addressed these endpoints path-style. A custom fs.s3a.endpoint with fs.s3a.path.style.access unset is now addressed virtual-hosted. Deployments on MinIO, Ceph RGW or similar services behind a hostname that relied on the previous always-path-style behavior need fs.s3a.path.style.access=true, which Hadoop already requires for those services; IP-address endpoints keep working either way. Reads with no custom endpoint change too: with only fs.s3a.endpoint.region=us-east-1 set, s3a://my-bucket/key moves from https://s3.us-east-1.amazonaws.com/my-bucket/key to https://my-bucket.s3.us-east-1.amazonaws.com/key, and with fs.s3a.path.style.access=true and no endpoint it moves the other way. A proxy or egress rule that matches on host names will see the new hosts. Native Iceberg reads go through iceberg-rust's FileIO and are unchanged. Vendor alias schemes are unaffected because the JVM side already synthesizes the flag for them.

How are these changes tested?

• Routing: a table test of the provider class lookup covering the global and per-bucket lists, padded and empty entries, exact matching, explicit classes, blank opt-outs and the SDK names, plus a test that an opted-out bucket still rejects the profile provider.
• Adapter: offline Java tests resolve a named profile from a configured credentials file (a per-bucket file beats a decoy global one) and check that a missing profile falls through Hadoop's list to the next provider. They run on the default and spark-4.1 builds and skip where hadoop-aws is older than 3.4.2.
• End to end: HadoopProfileCredentialsProviderSuite reads Parquet from MinIO natively with only the profile provider configured and checks the adapter was used, and that an explicit Comet class still wins. Like the other MinIO suites it is manual; it passed locally on Spark 4.1 together with HadoopS3ACredentialProviderAdapterBridgeSuite and CometS3CredentialBridgeSuite, and fails with Unsupported credential provider when the routing is disabled. There is no role-profile case, since role chains now run in the Java SDK.
• Addressing: the flag unset, true, false, mixed case with whitespace and an invalid value; per-bucket overrides of the flag and the endpoint; the s3.amazonaws.com skip in both modes; scheme-less, http://, port, trailing slash, path suffix, regional AWS hosts, dotted and other non-virtual-hostable bucket names; IPv4 and IPv6 hosts. Request URLs are pinned through presigned URLs, so no live store is needed.

[sunchao]

Reviewed 1917adca against base db790673. One verified P2 finding: the new default addressing mode breaks HTTPS buckets whose names contain dots.

Correctness

Previously, the native S3 configuration ignored the two profile override keys and inverted fs.s3a.path.style.access. This change carries the selected profile name/file through provider metadata, preserves bucket-specific precedence, and returns an endpoint together with its effective addressing mode. The maintained Spark 3.5/4.0 sources pass spark.hadoop.* values into Hadoop configuration. Comet's existing prefix-based extraction carries these S3 options through to native code. The new boolean parsing matches Hadoop's trimmed, case-insensitive true/false handling and default-false behavior.

The addressing change needs one correction before merge. Setting virtual_hosted_style_request = !path_style_access forces virtual hosting for dotted bucket names over HTTPS, including ordinary AWS endpoints with no override. For review.dotted.bucket, the resulting host is review.dotted.bucket.s3.us-east-1.amazonaws.com. The AWS SDK chooses the path-style URL instead, because the dotted hostname does not match S3's wildcard certificate. The inline P2 requests the same eligibility check for default and custom HTTPS endpoints.

I reproduced the endpoint difference without credentials or storage requests. A Rust harness using the HEAD configuration expressions and the local object_store 0.13.2 endpoint expression produced the dotted hostname. The actual offline AWS Java SDK 2.29.52 endpoint resolver, the version declared by Hadoop 3.4.2, selected path style. Normal bucket names, explicit path style and synthetic profile/bucket precedence checks behaved as expected. These are isolated configuration checks, not native/JNI or live S3 tests.

At 2026-09-12 20:03:22 UTC, only the label check succeeded. CI, CodeQL, the Delta gate and title validation were awaiting workflow approval. The cached merge has the assigned base/head parents and HEAD's tree, but no product CI execution can be credited. The author's reported 52 S3 tests/full-core pass was not rerun locally. Exact locked aws-config 1.12.0 and aws-runtime 1.9.2 source was unavailable locally, so SDK profile-file loading and refresh remain unverified. Maintained Spark 3.4/4.1 source gaps also remain. The final publication check confirmed that the head, base and discussion were unchanged after the temporary API rate limit cleared.

Performance

The added profile string handling and URL parsing occur during store/provider construction. The existing store cache includes the full configuration hash, so changing a profile name or file selects a different cached store. This PR does not add work to the object-read loop or alter credential expiry caching.

The endpoint result keeps normalization to one pass, and the profile description allocates only during construction-time logging. I found no separate verified performance issue. No benchmark or speedup claim is established by this review, and an expression microbenchmark is not applicable to this configuration change.

Design

Returning the normalized endpoint and addressing mode together is a useful safeguard against the original disagreement between those two values. The missing piece is deciding whether a bucket is eligible for virtual hosting before producing either result. That decision must also run when the endpoint is omitted, where object_store constructs the normal AWS URL.

Profile name and file are independently optional, with bucket values taking precedence over global values. The PR preserves the existing provider chain and expiry wrapper. Its metadata tests demonstrate option selection but do not establish actual SDK file precedence or credential renewal. The review therefore keeps those validation boundaries explicit.

Abstraction & complexity

The small NormalizedEndpoint type and blank-filtering helper are proportionate to the change. Configuration lookup remains centralized, and the new direct aws-runtime dependency supplies file-kind types already present in the dependency graph.

No broader provider or endpoint framework is needed. The actionable change is to extend the addressing decision with HTTPS bucket eligibility and test the final URL, rather than relying on a configuration-map assertion or a builder that has not issued a request.

[sunchao]

Correctness

[P2] Preserve path-style addressing for dotted HTTPS buckets

Could we apply the AWS SDK's virtual-host eligibility rules before enabling this flag? With fs.s3a.endpoint.region=us-east-1 and path.style.access unset or false, a bucket such as review.dotted.bucket now becomes https://review.dotted.bucket.s3.us-east-1.amazonaws.com. BASE used path-style addressing, and the AWS SDK endpoint resolver still selects https://s3.us-east-1.amazonaws.com/review.dotted.bucket for this case. The dotted host does not match S3's wildcard TLS certificate, so this breaks native reads of otherwise valid buckets. The same eligibility issue exists for custom HTTPS endpoints. Please retain path-style addressing for dotted HTTPS buckets, including when no custom endpoint is configured, and add assertions on the resulting request URL. The current dotted-bucket test asserts the virtual-hosted string, which misses this regression.

[dwsmith1983]

Please retain path-style addressing for dotted HTTPS buckets, including when no custom endpoint is configured, and add assertions on the resulting request URL.

In 0c76e6c. A bucket name containing a dot is addressed path-style whenever the endpoint is HTTPS, the default AWS endpoint included, and stays virtual-hosted over HTTP as the SDK does. The tests assert the endpoint and flag pair for the default endpoint, an explicit HTTPS endpoint, an HTTP endpoint and a plain bucket.

[andygrove]

The two keys being read here are PROFILE_NAME and PROFILE_FILE on Hadoop's org.apache.hadoop.fs.s3a.auth.ProfileAWSCredentialsProvider, and Hadoop only applies them when fs.s3a.aws.credentials.provider names that class. When the provider is spelled software.amazon.awssdk.auth.credentials.ProfileCredentialsProvider, which is the spelling this arm matches, Hadoop instantiates it through S3AUtils.getInstanceFromReflection, which finds no (URI, Configuration) constructor and falls through to the SDK's static create(). The Hadoop config never reaches it. So the JVM side of the job resolves the SDK default profile while the native side now resolves the configured one, and the two halves of one job authenticate as different identities.

At the same time build_aws_credential_provider_metadata has no arm for org.apache.hadoop.fs.s3a.auth.ProfileAWSCredentialsProvider, so the one provider spelling that does honor these keys on the Hadoop side hits the _ => arm and fails the native scan with Unsupported credential provider. Would it make sense to accept that FQCN as a third alias here and add it to the datasources.md row, so the configuration Hadoop actually documents works on both sides?

On the addressing change, the new paragraph in datasources.md explains the rule well but does not mention that the default is changing. Today a custom fs.s3a.endpoint is addressed path-style whatever fs.s3a.path.style.access says, so a MinIO or Ceph RGW deployment that never set the flag works right now and will start sending requests to http://<bucket>.<host> once this lands. All the user sees is a DNS failure with nothing tying it back to this config. Could we add a sentence saying deployments that relied on the previous always-path-style behavior need to set fs.s3a.path.style.access=true?

For what it is worth, the object store cache key is fine. hash_object_store_configs hashes the whole forwarded map and extractObjectStoreOptions forwards every fs.s3a.* key by prefix, so two stores differing only in profile name or file get distinct hashes. Nothing logs credentials either.

[dwsmith1983]

Would it make sense to accept that FQCN as a third alias here and add it to the datasources.md row, so the configuration Hadoop actually documents works on both sides?

Added org.apache.hadoop.fs.s3a.auth.ProfileAWSCredentialsProvider as the provider that reads the profile keys, with its own row. The SDK spellings now resolve the SDK default profile and ignore the keys, since Hadoop constructs those without its configuration, so both halves of a job resolve the same identity. A test pins that for both SDK spellings and the chain test uses the Hadoop provider.

Could we add a sentence saying deployments that relied on the previous always-path-style behavior need to set fs.s3a.path.style.access=true?

Added, right after the addressing paragraph.

[sunchao]

Re-reviewed 3f66db90 against 1d0ce5fe. The Hadoop profile-provider spelling now reads the profile keys, the SDK spellings ignore them, and the custom-endpoint migration note is present.

The existing P2 dotted HTTPS bucket finding remains unresolved. The addressing code is unchanged, and rerunning the offline SDK endpoint resolver and configuration probe confirms the mismatch.

One new P2 is inline: with the Hadoop provider and no profile-file override, native code still merges the SDK config and credentials files, whereas Hadoop reads only the credentials file. A same-name role profile in the config file can therefore change native credential resolution.

The exact aws-config 1.12.0 and aws-runtime 1.9.2 source gaps from the first review are now closed using lockfile-checksummed archives. Validation included source tracing and an isolated probe of the SDK file-selection code. No account files, credential resolution, storage requests, full native/JNI tests, or benchmarks were used. The merge tree matches HEAD. At September 15, 05:56 UTC, CI, CodeQL, and the Delta gate awaited approval with zero jobs; only labeling passed.

[sunchao]

Correctness

[P2] Keep Hadoop's default profile source credentials-only

Could we preserve Hadoop's file selection when fs.s3a.auth.profile.file is unset too? The newly supported HADOOP_PROFILE arm reaches this branch with file: None, so it leaves the Rust SDK defaults in place. In the locked SDK those defaults merge ~/.aws/config with ~/.aws/credentials. Hadoop's provider instead selects only AWS_SHARED_CREDENTIALS_FILE or ~/.aws/credentials in this case.

For example, with auth.profile.name=analytics, static credentials in the credentials file and a same-name config profile containing role_arn plus source_profile=analytics, Hadoop uses the static identity while the native SDK merges in the role and assumes it. That can change the identity or fail native reads that Hadoop can perform. Please retain the Hadoop/SDK provider distinction and select a credentials-only default for the Hadoop spelling, while preserving normal SDK defaults for the SDK spellings. A test of the selected file set when the override is absent would cover this case.

[dwsmith1983]

Could we preserve Hadoop's file selection when fs.s3a.auth.profile.file is unset too?

In 0c76e6c. The Hadoop profile provider arm reads only the credentials file, from fs.s3a.auth.profile.file, then AWS_SHARED_CREDENTIALS_FILE, then the default path, and never merges the config file, so the static identity wins in your example on both sides.

[dwsmith1983]

Please retain path-style addressing for dotted HTTPS buckets, including when no custom endpoint is configured, and add assertions on the resulting request URL.

Done. A bucket whose name contains a dot is addressed path-style whenever the endpoint is HTTPS, which covers the default AWS endpoint and a custom https:// one; over http:// it stays virtual-hosted, as the SDK does. The tests assert the endpoint and flag pair object_store receives for the default endpoint, an explicit HTTPS endpoint, an HTTP endpoint, and a plain bucket as the control. The addressing paragraph in datasources.md says the same. Apologies for the delay, this note and your earlier ones slipped past my sweep.

Please retain the Hadoop/SDK provider distinction and select a credentials-only default for the Hadoop spelling, while preserving normal SDK defaults for the SDK spellings.

Done. The profile metadata carries whether the provider is credentials-only; Hadoop's spelling is, the SDK spellings are not. With no file override, Hadoop's spelling now reads AWS_SHARED_CREDENTIALS_FILE or ~/.aws/credentials as a credentials-format file and nothing else, so a same-name role profile in the config file never applies; the SDK spellings keep the SDK defaults. A test covers the selected file with and without the override.

[sunchao]

Re-reviewed f9b1338a against a8e8157e. Both prior findings are addressed: dotted buckets now use path-style addressing over HTTPS, and Hadoop's profile provider now selects a credentials-only file without merging the SDK config.

Two new P2 findings are inline. The dotted-bucket guard also forces path style for custom HTTP endpoints because it runs before the scheme check. The credentials-only fallback uses native HOME instead of Hadoop's JVM user.home, so the two sides can select different files.

Validation used the current configuration functions in an isolated Rust probe, the offline AWS Java SDK 2.29.52 endpoint resolver, the locked SDK file-selection code, and Hadoop's declared Commons Lang 3.17.0 home-directory behavior. No account files, credential resolution, storage requests, full native/JNI tests, or benchmarks were used. Maintained Spark 3.5/4.0 configuration forwarding was checked. The 3.4/4.1 maintained-source gaps remain.

At September 15, 16:11 UTC, CI and CodeQL awaited approval with zero jobs. Only labeling passed. The current merge has the assigned base/head parents and the same tree as HEAD.

[sunchao]

Correctness

[P2] Apply the dotted-bucket guard after choosing the endpoint scheme

Could we limit this initial dotted-bucket fallback to the default HTTPS endpoint? With fs.s3a.endpoint=http://storage.example.test, bucket review.dotted.bucket, and path.style.access unset or false, this expression already sets the flag to false. normalize_endpoint then returns at its first path-style branch, before it can apply the scheme-sensitive rule. The native configuration produces http://storage.example.test/review.dotted.bucket, while Hadoop's AWS SDK resolver selects http://review.dotted.bucket.storage.example.test. This breaks a custom HTTP service that routes buckets by hostname. The new HTTP test calls normalize_endpoint(..., true) directly, bypassing the caller that supplies false. Please preserve virtual hosting for this HTTP case and cover it through extract_s3_config_options, including the resulting URL.

[dwsmith1983]

Please preserve virtual hosting for this HTTP case and cover it through extract_s3_config_options, including the resulting URL.

In 31fafaf. The dotted-bucket rule runs in extract_s3_config_options only when no custom endpoint is configured, since the default endpoint is HTTPS; a custom endpoint decides by its own scheme inside normalize_endpoint. The test goes through extract_s3_config_options and asserts http://review.dotted.bucket.storage.example.test for the HTTP case beside the HTTPS and default cases.

[sunchao]

Correctness

[P2] Resolve Hadoop's default credentials path from JVM user.home

Could we pass Hadoop's resolved default file into this branch instead of deriving it from the native process's HOME? When both fs.s3a.auth.profile.file and AWS_SHARED_CREDENTIALS_FILE are unset, Hadoop's provider uses SystemUtils.getUserHome(), which reads the JVM user.home property. For an executor launched with -Duser.home=/synthetic/jvm-home while HOME=/synthetic/env-home, Hadoop selects /synthetic/jvm-home/.aws/credentials but this code selects /synthetic/env-home/.aws/credentials. If HOME is absent, it selects /.aws/credentials even when the JVM has a valid home. A job can therefore load a different profile or fail native reads after Hadoop successfully loads its credentials. Please retain credentials-only loading while using the same resolved file on both sides, with a case where HOME and user.home differ.

[dwsmith1983]

Could we pass Hadoop's resolved default file into this branch instead of deriving it from the native process's HOME?

In f3758e5. The executor resolves fs.s3a.comet.default.profile.file against its own user.home or AWS_SHARED_CREDENTIALS_FILE when a native plan is created, native keeps it as a session extension and overlays it onto each scan's options, and the Hadoop provider arm takes a configured fs.s3a.auth.profile.file first, then that path. A driver and executor with different homes resolve the executor's.

[dwsmith1983]

Please preserve virtual hosting for this HTTP case and cover it through extract_s3_config_options, including the resulting URL.

Done. The dotted-bucket rule now runs in extract_s3_config_options only when no custom endpoint is configured, since the default AWS endpoint is HTTPS; a custom endpoint decides by its own scheme inside normalize_endpoint. The test now goes through extract_s3_config_options and asserts http://storage.example.test with review.dotted.bucket yields virtual hosting and the endpoint http://review.dotted.bucket.storage.example.test, next to the HTTPS and default-endpoint cases.

Could we pass Hadoop's resolved default file into this branch instead of deriving it from the native process's HOME?

Done. NativeConfig.extractObjectStoreOptions now forwards fs.s3a.comet.default.profile.file, resolved on the JVM as AWS_SHARED_CREDENTIALS_FILE or the user.home .aws/credentials, for every scheme on the fs.s3a.* surface. Native's Hadoop-provider arm takes a configured fs.s3a.auth.profile.file first, then that forwarded path; the native HOME fallback only remains for callers that forward nothing. Tests on both sides: the Scala one checks the forwarded value and that a blank env override falls back to user.home rather than HOME, and the native one checks the forwarded file is taken, a configured file wins, and the SDK spellings ignore both. Merged with main at a8e8157 first.

[sunchao]

Re-reviewed 31fafaff against 4479e722. The dotted HTTP bucket finding is fixed: the extraction-caller probe now matches the scheme-aware rule, including the original failing case.

The existing P2 profile-path finding remains for distributed execution. extractObjectStoreOptions runs during driver-side scan planning, so the new key freezes the driver's user.home or AWS_SHARED_CREDENTIALS_FILE into the plan. The executor's native provider consumes that path unchanged. With different driver/executor homes, the isolated JVM checks select /synthetic/driver-home/.aws/credentials for native code where executor-side Hadoop selects /synthetic/executor-home/.aws/credentials. Could we resolve the omitted default in the executor JVM before creating the native provider, while preserving explicit file overrides?

Validation used the full NativeConfig object in isolated JVMs, exact-source Rust configuration probes with a bounded URL test double, the actual Hadoop path selector, and the offline AWS SDK endpoint resolver. No full Spark/JNI tests, credential loading, storage requests or benchmarks ran. Maintained Spark 3.4/4.1 source gaps remain. At September 15, 18:28 UTC, CI and CodeQL awaited approval with zero jobs. Only labeling passed.

[dwsmith1983]

Could we resolve the omitted default in the executor JVM before creating the native provider, while preserving explicit file overrides?

Done. The driver no longer forwards anything for it. CometExecIterator.serializeCometSQLConfs, which runs on the executor when a native plan is created (the shuffle writer path included), now carries fs.s3a.comet.default.profile.file resolved against that executor's user.home or AWS_SHARED_CREDENTIALS_FILE; native keeps it as a session extension and overlays it onto each scan's object store options, and the Hadoop-provider arm takes a configured fs.s3a.auth.profile.file first, then that path. With different driver and executor homes the executor's wins. Tests: the serialized executor config carries the resolved path, planning on the driver forwards nothing, the overlay replaces a driver-serialized value and is a no-op without the extension, and the provider arm prefers an explicit file. Merged with main at 3868760.

[sunchao]

Rechecked f3758e59 against 4479e722, including the change since 31fafaff.

The remaining profile-path finding is addressed. The implicit file is now resolved in the executor JVM before native plan creation, then applied to Parquet and CSV scan options. Explicit global and per-bucket profile files still take precedence, and the SDK profile aliases remain separate. The prior endpoint/addressing fixes are unchanged. I found no new or remaining P1/P2 issues.

The focused reproduction now selects the executor's file for both differing user.home values and differing AWS_SHARED_CREDENTIALS_FILE values. The previous head selects the driver's file. This used the full NativeConfig, the exact serializer method with small Spark/protobuf test doubles, isolated native overlay/provider-selection code, and Hadoop's actual path selector. It did not run a full Spark/JNI workload or authenticate to storage.

CI and CodeQL for this head are still awaiting approval with zero jobs. Only labeling has passed. Maintained Spark 3.5/4.0 sources were checked. Maintained 3.4/4.1 sources remain unavailable, so this review does not claim coverage for those versions.

[sunchao]

Rechecked 629d913b against 8c229a70. This is a base-only merge: all 11 PR files and the full authored diff are unchanged from f3758e59, and all 61 changed paths match the new base. The executor-side profile-path fix, explicit-file precedence, provider distinction and endpoint/addressing fixes remain intact. No new or remaining P1/P2 findings. Keeping the existing approval.

Validation checked source and dependency equivalence, including 14 complete files across the configuration path. The prior synthetic probes were not rerun. CI and CodeQL still await approval with zero jobs. Only labeling passed. No full Spark/JNI or cloud validation is claimed. Maintained Spark 3.4/4.1 source gaps remain.

[sunchao]

Follow-up on unchanged 629d913b / 8c229a70: attempt 2 Preflight now fails Markdown formatting in datasources.md. One new P2 is inline. Cached Prettier 3.9.6 passes the base file and fails the head file, with the difference confined to the changed credentials table. CI does not record its formatter version, so I am not claiming an exact version match.

The executed merge d89e12fa has the reviewed head's exact tree. Required Checks fails downstream of Preflight, and no product build or test jobs ran in this attempt. The newer advertised merge 4c39c21d is a different tree and gets no validation credit from these jobs.

The authored diff and prior source fixes are unchanged. Keeping the existing approval while the formatting blocker is corrected. No new runtime validation was performed.

[sunchao]

Correctness

[P2] Format the changed credentials table to unblock Preflight

Could you run prettier --write docs/source/user-guide/latest/datasources.md for this table change? Attempt 2 Preflight fails Check markdown formatting on this file. Its checked-out merge d89e12fa has the reviewed head's exact tree. A read-only check with cached Prettier 3.9.6 passes the base file but fails the head/CI file, and the only formatting diff adjusts this credentials table after the longer profile description. The downstream Required Checks failure comes from Preflight, and product build/test jobs were skipped. This needs formatting before CI can validate the implementation.

[dwsmith1983]

Could you run prettier --write docs/source/user-guide/latest/datasources.md for this table change?

Done in 7b95c8f. prettier --check "**/*.md" with prettier 3.9.7 now passes on the full tree, and the branch is merged with main.

[sunchao]

Rechecked 7b95c8fbb602cdfd39e859ed9faf80f4a989e768 against 36146a87bf9ca9ca9e211b4372628ed2f9d8c8c6. The Markdown-formatting finding is fixed: cached Prettier 3.9.6 reproduces the prior-head failure, while the base, current head, and advertised merge pass for datasources.md. The current document is byte-identical to the formatter output expected for the previous head.

The new authored change is table formatting. All ten non-documentation authored changes are unchanged from the previous review. The inherited base changes preserve executor-side profile-file resolution, JNI transport, scan-option overlays, and the earlier provider/addressing fixes. No remaining P1/P2 findings in this follow-up.

CI and CodeQL currently require approval and each has zero jobs. Only the label workflow passed. This approval covers the source review and focused local formatter check. I did not run full-tree formatting, native/JNI tests, or live object-store authentication at this head.

[sunchao]

Rechecked 1f2858eb against 58ab5f61 after the main merge. All 11 authored files are byte-identical to 7b95c8fb, and the six inherited file changes match the base update. The executor-local profile-file resolution, Hadoop credentials-only loading and dotted-bucket endpoint fixes remain intact. No new or remaining P1/P2 findings.

The document still matches the previously verified Prettier output. This round used source comparisons, with no product tests or formatter rerun. CI and CodeQL require approval and have run zero jobs. The successful label workflow checked out base/main and provides no product test evidence. My existing approval remains unchanged.

[sunchao]

Summary

• Prior state and problem: Native S3 ignored Hadoop profile settings and effectively forced custom endpoints to use path-style addressing.
• Design approach: Carry profile settings explicitly, resolve default credentials paths on executors, and normalize endpoints together with their addressing mode.
• Correctness / compatibility analysis: No additional introduced P1/P2 issues found within this review. Two existing P2 blockers remain: per-role STS region selection and global STS fallback. The conflicting-source fixture selects eu-west-1 instead of Hadoop’s eu-central-1. With no region, native selects sts.us-east-1.amazonaws.com instead of sts.amazonaws.com. Both can prevent credential resolution under endpoint restrictions. These findings are not duplicated inline.
• Key design decisions: Separating Hadoop’s credentials-only provider from SDK aliases preserves file-selection behavior. NormalizedEndpoint and the executor session extension keep the implementation localized.
• Implementation sketch: Executor defaults cross JNI and overlay Parquet/CSV options before store lookup. Profile settings participate in cache identity. Added work occurs during construction, with Arrow ownership and the object-read loop unchanged. No separate P1/P2 performance issue was established.
• Behavioral changes worth calling out: Custom hostname endpoints now default to virtual hosting. The migration guide documents fs.s3a.path.style.access=true to retain previous addressing. IP endpoints and ineligible bucket names retain path style. The Hadoop provider’s version prerequisite is documented.
• Suggested improvements: Address the existing blockers by preserving each role’s region selection and Hadoop’s global STS fallback, with regression tests for both.

Reviewed the full 12-file diff from 93d1189beb2a2da1c4e2663de131c61713b51e48 to db7a9ac96dec7a9c30c21e42ad547a299c276bf9. The PR remains open and non-draft. Routed skills: review-comet-pr and review-comet-ffi-pr.

Exact-head CI: Comet CI and CodeQL require approval and have zero jobs. Only labeling passed.

Validation: All 58 native S3 tests passed with --no-default-features. Recompiled, source-matched probes using the locked Rust SDK versions reproduced both existing blockers, including failures with synthetic endpoint restrictions. Compared configuration forwarding across Spark 3.4.3, 3.5.9, 4.0.4, 4.1.3 and 4.2.0 with relevant Hadoop/AWS SDK sources. Rust formatting and diff checks passed. Full-feature native, Spark/JNI and Spark SQL suites, live storage requests, benchmarks and Markdown formatting were not run.

[andygrove]

I compared this branch against branch-1.1, since 1.1.0 is what people will upgrade from, and the addressing rule now matches the AWS SDK v2 rule that S3A uses, down to the IPv4 and adjacent-separator checks. The presigned-URL tests are a nice way to pin the request URLs without a live store. My comments below are mostly about the upgrade guide. CI also needs a green run before this merges, since no build or test job has run on any revision of this branch yet.

1. docs/source/user-guide/latest/migration-guide.md:93

This entry sits under "Upgrading to Comet 1.1.0" and says that Comet 1.1.0 follows Hadoop S3A. branch-1.1 is already at 1.1.0 with an RC tagged, though, and main is now 1.2.0-SNAPSHOT. Unless this PR is backported to branch-1.1 before 1.1.0 ships, 1.1.0 keeps the old addressing. Someone upgrading from 1.1.0 to 1.2.0 would read only the 1.2.0 section and miss this. My earlier comment asked for an entry for the release this lands in, and I should have said that means 1.2.0. Could you move it into a new "Upgrading to Comet 1.2.0" section and say that 1.1.0 and earlier addressed these endpoints path-style?

The entry also covers only a custom fs.s3a.endpoint, but reads with no endpoint change too. extract_s3_config_options now always passes VirtualHostedStyleRequest, so with only this set:

spark.hadoop.fs.s3a.endpoint.region=us-east-1

a read of s3a://my-bucket/key moves from https://s3.us-east-1.amazonaws.com/my-bucket/key to https://my-bucket.s3.us-east-1.amazonaws.com/key. With fs.s3a.path.style.access=true and no endpoint it moves the other way, from virtual-hosted to path-style. Native reads have used the path-style form on the default endpoint since the S3A translation was added in #1817, so this reaches every native S3 read on AWS. It matches what S3A does, so I don't expect it to break anyone, but a proxy or egress rule that matches on host names will see it. Could the entry and the PR description mention this case too? The native CSV scan builds its store the same way, so the entry could say native scans rather than the native Parquet scan.

2. spark/src/main/scala/org/apache/comet/objectstore/NativeConfig.scala:199

The new doc comment, COMET_DEFAULT_PROFILE_FILE_KEY and defaultSharedCredentialsFile went in between the existing scaladoc for extractObjectStoreOptions and the method itself. Scaladoc attaches only the comment right before a definition, so extractObjectStoreOptions loses its doc and the old comment is left dangling. The new comment also lands on the val rather than on defaultSharedCredentialsFile, which is what it describes. Could you move the new block above the extractObjectStoreOptions scaladoc?

3. docs/source/user-guide/latest/datasources.md:240

On the open threads about STS region selection, your reply says two differences from Hadoop's provider remain for assume-role profiles. When no region is found anywhere, native calls sts.us-east-1.amazonaws.com where Hadoop calls the global sts.amazonaws.com. When the role profile has no region, native uses the source_profile's region before the default region chain, where Hadoop goes straight to the chain. If we keep these, could this row say so? Someone whose STS egress is restricted would otherwise expect native reads to reach the same STS endpoint that Hadoop does.

[sunchao]

Summary

• Prior state and problem: Native S3 ignored Hadoop profile settings and incorrectly applied fs.s3a.path.style.access.
• Design approach: Carry profile settings explicitly, resolve default credentials paths on executors, and normalize endpoints together with their addressing mode.
• Correctness / compatibility analysis: No additional introduced P1/P2 issues found within this review. Two existing P2 blockers remain: per-role STS region selection and global STS fallback. The conflicting-source fixture selects eu-west-1 instead of Hadoop’s eu-central-1. With no region, native selects sts.us-east-1.amazonaws.com instead of sts.amazonaws.com. Both reproduced credential failures under synthetic endpoint restrictions. These findings are not duplicated inline.
• Key design decisions: Separating Hadoop’s credentials-only provider from SDK aliases preserves file-selection behavior. NormalizedEndpoint and the executor session extension keep the changes localized without unnecessary abstraction.
• Implementation sketch: Executor defaults cross JNI and overlay Parquet/CSV options before store lookup. Profile settings participate in cache identity. Added work occurs during construction. Arrow ownership and the object-read loop remain unchanged. No separate P1/P2 performance issue was established.
• Behavioral changes worth calling out: Eligible buckets now default to virtual hosting on both custom and default AWS endpoints. fs.s3a.path.style.access=true selects path-style addressing. IP endpoints and ineligible bucket names retain path style.
• Suggested improvements: Address the existing STS blockers by preserving each role’s region selection and Hadoop’s global fallback. The existing migration-guide concern also needs correction: the new entry claims this behavior ships in 1.1.0, but branch-1.1 retains the old implementation and this PR targets 1.2.0.

Reviewed the full 12-file diff from 93d1189beb2a2da1c4e2663de131c61713b51e48 to db7a9ac96dec7a9c30c21e42ad547a299c276bf9. The PR remains open and non-draft. Routed skills: review-comet-pr and review-comet-ffi-pr.

Exact-head CI: Comet CI and CodeQL require approval and have zero jobs. Only labeling passed.

Validation: All 58 native S3 tests passed with --no-default-features. Recompiled probes matched the current profile-construction source and reproduced both STS blockers using locked Rust dependencies and Java SDK 2.29.52. Checked upstream configuration forwarding for Spark 3.4.3, 3.5.9, 4.0.4, 4.1.3 and 4.2.0, plus Hadoop provider semantics. Rust formatting and diff checks passed. Full-feature native, Spark/JNI and Spark SQL suites were not run. JNI headers are unavailable. No live storage requests, benchmarks or Markdown formatting check ran.

[sunchao]

Summary

• Prior state and problem: Native S3 ignored Hadoop profile settings and incorrectly applied fs.s3a.path.style.access.
• Design approach: Carry profile settings explicitly, resolve default credentials paths on executors, pair endpoints with their addressing mode, and resolve supported role chains iteratively.
• Correctness / compatibility analysis: One new P2 affects source profiles containing both static keys and credential_process. Native selects different credentials from Hadoop. The existing per-role STS region and global STS fallback blockers remain for credential_source chains. Synthetic probes reproduced both residual endpoint differences. Their original static-source cases are fixed.
• Key design decisions: Separating Hadoop’s credentials-only provider from SDK aliases preserves file-selection behavior. The endpoint and session-default types are proportionate. The role-chain fallback must also preserve Hadoop’s credential-selection precedence.
• Implementation sketch: Executor defaults cross JNI and overlay Parquet/CSV options before store lookup. Configuration remains part of cache identity. Arrow ownership and the object-read loop are unchanged. No separate reproducible P1/P2 performance regression was established.
• Behavioral changes worth calling out: Compared against branch-1.1 at 4f5cf2db1e80bf94db8a4b79a870738d621f1262. Profile selection and eligible buckets’ default virtual hosting are intended changes. The migration guide now correctly covers Comet 1.2.0, custom endpoints and default AWS endpoints.
• Suggested improvements: Preserve Java’s credential precedence for mixed source profiles and complete per-role/global STS handling for delegated chains.

Reviewed the full 12-file diff from 432523fb59efe75b208ae5688eef6012c208a4d6 to c5594463bff59afeceb88e6932d73dd6ccf79cdf. The PR remains non-draft. Routed skills: review-comet-pr and review-comet-ffi-pr.

Exact-head CI: Comet CI, CodeQL and title validation require approval. CI and CodeQL have zero jobs. Only labeling passed.

Validation: All 66 native S3 tests passed using the exact-head --no-default-features binary with isolated AWS files and IMDS disabled. The bounded Cargo invocation compiled successfully but timed out during tests before that successful rerun. Rust formatting and diff checks passed. Compared upstream configuration forwarding for Spark 3.4.3, 3.5.9, 4.0.4, 4.1.3 and 4.2.0, Hadoop 3.4.2, and Java SDK 2.29.52. Source-matched Rust probes used locked dependencies and in-memory STS responses. Full-feature native, JVM/Spark SQL suites, live storage, benchmarks and Markdown formatting were not run.

[sunchao]

[P2] Preserve Hadoop’s credential precedence for mixed source profiles. With the newly supported ProfileAWSCredentialsProvider, select an assume-role [analytics] profile whose source_profile=source, where [source] contains static keys and credential_process. Hadoop’s Java SDK gives credential_process precedence. This branch instead rejects the custom chain and delegates to the Rust SDK, which prioritizes a source profile’s static keys. Native therefore signs AssumeRole with a different identity and fails when only the process identity can assume the role. Could we resolve mixed source profiles using Java’s precedence instead of delegating the entire chain, and add a regression test asserting the signing key?

Evidence: Reproduced with the verbatim current production profile-resolution block and locked aws-config 1.12.0, aws-runtime 1.9.2 and aws-sdk-sts 1.114.0. The fixture sets analytics.role_arn, analytics.source_profile=source and analytics.region=us-west-2. The source contains synthetic-source-key plus a credential_process returning synthetic-process-key. Native recorded an STS request signed by synthetic-source-key. Java SDK 2.29.52’s actual ProfileCredentialsUtils selected synthetic-process-key from the same fixture. An in-memory STS accepting only the process identity returned AccessDenied to native. Sources, fixture and outputs: /tmp/comet5872-c559-review/profile_probe.rs, JavaProfileOracle.java, java-mixed-process.credentials, profile_probe.log, java-oracle.log and restricted_profile_probe.log. No external STS requests were made.

[dwsmith1983]

Could we resolve mixed source profiles using Java's precedence instead of delegating the entire chain, and add a regression test asserting the signing key?

Changed approach in aa73a0d. Instead of matching more of the Java SDK in Rust, a bucket whose provider list names ProfileAWSCredentialsProvider now gets its credentials from the existing HadoopS3ACredentialProviderAdapter, which builds Hadoop's own provider list on the executor. The mixed source profile, credential_source chains, per-role regions and the global STS endpoint all resolve in the Java SDK, so native signs with whatever identity Hadoop picks.

Before switching I ran the real Hadoop 3.4.2 provider over 63 profile fixtures against the Rust walker; it still differed on 22, including this one, self-referencing roles, SSO with a role, and the list's error handling. The walker and the default-credentials forward are gone. HadoopProfileCredentialsProviderSuite reads from MinIO with only the profile provider set and checks the adapter served it; it passes on Spark 4.1 and fails with Unsupported credential provider when the routing is turned off.

[sunchao]

Summary

• Prior state and problem: Native S3 ignored Hadoop profile settings and incorrectly applied fs.s3a.path.style.access.
• Design approach: Carry profile settings explicitly, resolve default credentials paths on executors, pair endpoints with their addressing mode, and resolve supported role chains iteratively.
• Correctness / compatibility analysis: No additional introduced P1/P2 issues found within this review. Three existing P2 concerns remain substantiated: mixed source profiles select static credentials where Hadoop selects credential_process; delegated credential_source chains still lose individual role regions; and their no-region case uses the regional endpoint instead of Hadoop’s global STS fallback. The original static-source region cases are fixed. These concerns are not duplicated inline.
• Key design decisions: Separating Hadoop’s credentials-only provider from SDK aliases preserves file selection. The endpoint and session-default types keep configuration handling localized. Iterative role resolution avoids growing the stack, but delegation must preserve Java’s credential and endpoint semantics too.
• Implementation sketch: Executor defaults cross JNI and overlay Parquet/CSV options before store lookup. Configuration remains part of cache identity. New parsing occurs during setup and role requests during credential resolution. Arrow ownership is unchanged. No separate reproducible P1/P2 performance regression was established.
• Behavioral changes worth calling out: Compared with branch-1.1 at 68416099da4c69cb6b6433fccf9e7a60a93e39b3, profile support and default virtual hosting are intended changes. The migration guide now covers Comet 1.2.0, custom endpoints and default AWS endpoints. fs.s3a.path.style.access=true selects path-style addressing.
• Suggested improvements: Address the existing mixed-profile precedence and delegated-chain STS blockers before merging, with regression tests asserting signing identity and each request’s endpoint.

Reviewed the full 12-file diff from 432523fb59efe75b208ae5688eef6012c208a4d6 to c5594463bff59afeceb88e6932d73dd6ccf79cdf. The PR remains open and non-draft. Routed skills: review-comet-pr and review-comet-ffi-pr.

Exact-head CI: Comet CI, CodeQL and title validation require approval. CI and CodeQL have zero jobs. Only labeling passed.

Validation: All 66 native S3 tests passed with --no-default-features and isolated AWS settings. Recompiled source-matched Rust probes and Java SDK 2.29.52 reproduced the existing identity and endpoint differences. An in-memory STS reproduction confirmed AccessDenied when only the process identity was accepted. Checked upstream Spark 3.4.3, 3.5.9, 4.0.4, 4.1.3 and 4.2.0 configuration forwarding and Hadoop 3.4.2 provider semantics. Rust formatting and diff checks passed. Full-feature native validation was not run because JNI headers are unavailable. JVM/Spark SQL suites, live storage tests, benchmarks and Markdown formatting were not run.

[dwsmith1983]

Could you move it into a new "Upgrading to Comet 1.2.0" section and say that 1.1.0 and earlier addressed these endpoints path-style?

Moved in c559446. The entry also covers the no-endpoint case with your fs.s3a.endpoint.region=us-east-1 example in both directions, notes that host-matching proxy or egress rules will see the new hosts, and says native Parquet and CSV scans, since native Iceberg reads go through iceberg-rust's FileIO and are unchanged. The description has the same case.

Could you move the new block above the extractObjectStoreOptions scaladoc?

That block is gone in aa73a0d: Hadoop's profile provider now resolves through HadoopS3ACredentialProviderAdapter on the executor, so the JVM no longer forwards a default credentials path and NativeConfig.scala is back to main.

If we keep these, could this row say so?

Neither is kept. Hadoop's provider picks the STS region and endpoint now, so the row describes the routing instead, and the S3 credential provider guide covers the class-loader requirement, the explicit class and the blank opt-out. The AWS SDK ProfileCredentialsProvider names still resolve natively as on main; their differences from Hadoop are tracked in #6575.

[sunchao]

Summary

• Prior state and problem: Native S3 scans rejected Hadoop’s profile provider and incorrectly applied fs.s3a.path.style.access.
• Design approach: Route Hadoop profile credentials through the existing JVM adapter and return normalized endpoints together with their effective addressing mode.
• Correctness / compatibility analysis: No introduced P1/P2 issues found within this review. The previous profile-selection, credential-precedence and STS findings are addressed by delegating resolution to Hadoop. Checked relevant Spark configuration forwarding across 3.4.3, 3.5.9, 4.0.4, 4.1.3 and 4.2.0, plus Hadoop and AWS SDK behavior.
• Key design decisions: Explicit provider classes and blank opt-outs retain precedence. Reusing Hadoop’s provider list avoids maintaining another profile resolver in Rust. Bucket-specific configuration remains part of store identity.
• Implementation sketch: Provider lookup selects the adapter and forwards fs.s3a.* settings through the existing JNI bridge. Endpoint parsing occurs during store construction. Static profile credentials require a JNI lookup per request, while expiring credentials use the existing cache. No reproducible P1/P2 performance regression was established.
• Behavioral changes worth calling out: Compared with branch-1.1 at 992c806a7e38c2e88bd018aa5774164b0850e1fa, profile support and default virtual hosting are intended changes. The Comet 1.2.0 migration entry covers custom and default AWS endpoints, including fs.s3a.path.style.access=true for deployments requiring path-style requests. Hadoop version and class-loader requirements are documented.
• Suggested improvements: No additional P1/P2 changes requested. Previously reported concrete blockers are addressed, although their threads remain formally unresolved.

Reviewed the entire nine-file diff from 432523fb59efe75b208ae5688eef6012c208a4d6 to af0ced90eec6eba771f90e2ed5b152def4e2545a. The PR remains open and non-draft. Routed skills: review-comet-pr and review-comet-ffi-pr.

Exact-head CI: Comet CI, CodeQL and title validation require approval. CI and CodeQL have zero jobs. Only labeling passed.

Validation: All 53 native S3 tests passed with --no-default-features. All eight Java adapter tests passed when compiled separately against Spark 4.1.3, Hadoop 3.4.2 and AWS SDK 2.31.51. An offline comparison checked 44 request URLs against the Java SDK: 42 matched, and both differences were verified unchanged from base. Rust formatting and diff checks passed. Full-feature native, Maven reactor, Spark SQL, MinIO/JNI integration, benchmarks and Markdown formatting were not run.

[sunchao]

Summary

• Prior state and problem: Native S3 scans rejected Hadoop’s profile provider and incorrectly applied fs.s3a.path.style.access.
• Design approach: Resolve profiles through the existing Hadoop JNI adapter and return normalized endpoints together with their effective addressing mode.
• Correctness / compatibility analysis: No introduced P1/P2 issues found within this review. Previously reported concrete concerns are addressed, although their threads remain open. Checked configuration forwarding across Spark 3.4.3, 3.5.9, 4.0.4, 4.1.3 and 4.2.0, plus relevant Hadoop and AWS SDK behavior.
• Key design decisions: Explicit provider classes and blank opt-outs retain precedence. Reusing Hadoop’s provider list avoids maintaining a second profile resolver. Bucket-specific settings remain part of cache identity.
• Implementation sketch: Native provider lookup selects the adapter and forwards fs.s3a.* settings through the existing bridge. Endpoint processing occurs during store construction. Static credentials require per-request JNI lookups, while expiring credentials use the existing cache. No reproducible P1/P2 performance regression was established.
• Behavioral changes worth calling out: Compared with branch-1.1 at 992c806a7e38c2e88bd018aa5774164b0850e1fa, profile support and default virtual hosting are intended changes. The Comet 1.2.0 migration entry covers custom and default AWS endpoints. Hadoop version, class-loader requirements and fs.s3a.path.style.access=true are documented.
• Suggested improvements: No additional P1/P2 changes requested.

Reviewed the entire nine-file diff from fef94f6cd78b18151dff57b7a936798385356de5 to 177f019587c4e8951b021ed5fc10164999638ad5. The PR remains open and non-draft. Routed skills: review-comet-pr and review-comet-ffi-pr.

Exact-head CI: Comet CI and CodeQL require approval and have zero jobs. Only labeling passed.

Validation: All 53 native S3 tests passed with --no-default-features and isolated AWS settings. All eight Java adapter tests passed when compiled separately against Spark 4.1.3, Hadoop 3.4.2 and AWS SDK 2.31.51. Compared 44 offline request URLs with the Java SDK: 42 matched, and both differences were verified unchanged from base. Rust formatting and diff checks passed. Full-feature native, Maven reactor, Spark SQL, MinIO/JNI integration, real object-store tests, benchmarks and Markdown formatting were not run.

[sunchao]

Summary

• Prior state and problem: Native S3 scans rejected Hadoop’s profile provider and incorrectly applied fs.s3a.path.style.access.
• Design approach: Resolve profiles through the existing Hadoop credential adapter and pair normalized endpoints with their effective addressing mode.
• Correctness / compatibility analysis: No introduced P1/P2 issues found within this review. Previously reported concrete concerns are addressed, although their threads remain open. Checked configuration forwarding across Spark 3.4.3, 3.5.9, 4.0.4, 4.1.3 and 4.2.0, plus relevant Hadoop and AWS SDK behavior.
• Key design decisions: Explicit provider classes and blank opt-outs retain precedence. Reusing Hadoop’s provider list avoids maintaining a separate profile resolver. Bucket-specific settings remain part of cache identity.
• Implementation sketch: Native lookup selects the adapter and forwards fs.s3a.* through the existing JNI bridge. Endpoint processing occurs during store construction. Static credentials require per-request JNI lookups, while expiring credentials use the existing cache. Arrow ownership is unchanged. No reproducible P1/P2 performance regression was established.
• Behavioral changes worth calling out: Compared with branch-1.1 at 992c806a7e38c2e88bd018aa5774164b0850e1fa, profile support and default virtual hosting are intended changes. The Comet 1.2.0 migration entry covers custom and default AWS endpoints. Hadoop version, class-loader requirements and fs.s3a.path.style.access=true are documented.
• Suggested improvements: No additional P1/P2 changes requested.

Reviewed the entire nine-file diff from fef94f6cd78b18151dff57b7a936798385356de5 to 177f019587c4e8951b021ed5fc10164999638ad5. The PR remains open and non-draft. Routed skills: review-comet-pr and review-comet-ffi-pr.

Exact-head CI: Comet CI and CodeQL require approval and have zero jobs. Only labeling passed.

Validation: All 53 native S3 tests passed with --no-default-features and isolated AWS settings. All eight Java adapter tests passed after separate compilation against Spark 4.1.3, Hadoop 3.4.2 and AWS SDK 2.31.51. Compared 44 offline request URLs with the Java SDK: 42 matched, and both differences were verified unchanged from base. Rust formatting and diff checks passed. An initial native rebuild exceeded its 240-second limit before the normal-profile run passed. Full-feature native, Maven reactor, Spark SQL, MinIO/JNI integration, live storage and benchmarks were not run. Markdown formatting was not rerun because the formatter was unavailable.

[sunchao]

Summary

• Prior state and problem: Native S3 scans rejected Hadoop’s profile provider and incorrectly applied fs.s3a.path.style.access.
• Design approach: Resolve profiles through the existing Hadoop credential adapter and pair normalized endpoints with their effective addressing mode.
• Correctness / compatibility analysis: No introduced P1/P2 issues found within this review. Previously reported concrete concerns are addressed. Checked relevant Spark configuration forwarding across 3.4.3, 3.5.9, 4.0.4, 4.1.3 and 4.2.0, plus Hadoop and AWS SDK behavior.
• Key design decisions: Explicit provider classes and blank opt-outs retain precedence. Reusing Hadoop’s provider list avoids maintaining another profile resolver. Per-bucket settings remain part of cache identity.
• Implementation sketch: Native lookup selects the adapter and forwards fs.s3a.* through the existing JNI bridge. Endpoint processing occurs during store construction. Static profile credentials require per-request JNI lookups, while expiring credentials use the existing cache. Arrow ownership is unchanged. No reproducible P1/P2 performance regression was established.
• Behavioral changes worth calling out: Compared with branch-1.1 at 992c806a7e38c2e88bd018aa5774164b0850e1fa, profile support and default virtual hosting are intended changes. The Comet 1.2.0 migration entry covers custom and default AWS endpoints. Hadoop version, class-loader requirements and fs.s3a.path.style.access=true are documented.
• Suggested improvements: No additional P1/P2 changes requested. Earlier concrete blockers are addressed, although their threads remain formally unresolved.

Reviewed the entire nine-file diff from 33f21da181c14df7fa3e6a3104c3d7215810147e to 5f68625b3f7adb11cba0a7b9cab8ee77f4b8f8f8. The PR remains open and non-draft. Routed skills: review-comet-pr and review-comet-ffi-pr.

Exact-head CI: Comet CI and CodeQL require approval and have zero jobs. Only labeling passed.

Validation: All 53 native S3 tests passed with --no-default-features and isolated AWS settings. All eight Java adapter tests passed after separate compilation against Spark 4.1.3, Hadoop 3.4.2 and AWS SDK 2.31.51. Compared 44 offline request URLs with the Java SDK: 42 matched, and both differences were verified unchanged from base. Rust formatting and diff checks passed. Full-feature native, Maven reactor, Spark SQL, MinIO/JNI integration, live storage and benchmarks were not run. Markdown formatting was not checked because the formatter was unavailable.

[sunchao]

Summary

• Prior state and problem: Native S3 scans rejected Hadoop’s profile provider and incorrectly applied fs.s3a.path.style.access.
• Design approach: Resolve profiles through the existing Hadoop credential adapter and pair normalized endpoints with their effective addressing mode.
• Correctness / compatibility analysis: No introduced P1/P2 issues found within this review. Previously reported concrete concerns are addressed, although their threads remain formally unresolved. Checked relevant configuration forwarding across Spark 3.4.3, 3.5.9, 4.0.4, 4.1.3 and 4.2.0, plus Hadoop and AWS SDK behavior.
• Key design decisions: Explicit provider classes and blank opt-outs retain precedence. Reusing Hadoop’s provider list avoids maintaining another profile resolver. Per-bucket configuration remains part of cache identity.
• Implementation sketch: Native lookup selects the adapter and forwards fs.s3a.* through the existing JNI bridge. Endpoint processing occurs during store construction. Static profile credentials require per-request JNI lookups, while expiring credentials use the existing cache. Arrow ownership is unchanged. No reproducible P1/P2 performance regression was established.
• Behavioral changes worth calling out: Compared with branch-1.1 at 7b7eec69282a7abc33a0e8e687ca594c72f140ec, profile support and default virtual hosting are intended changes. The Comet 1.2.0 migration entry covers custom and default AWS endpoints. Hadoop version, class-loader requirements and fs.s3a.path.style.access=true are documented.
• Suggested improvements: No additional P1/P2 changes requested.

Reviewed the entire nine-file diff from 9dc8c3ca89962506078831f7064a25c75da83883 to 19ff59f543a1d76883896bf5869e68c8ac98c648. The PR remains open and non-draft. Routed skills: review-comet-pr and review-comet-ffi-pr.

Exact-head CI: Comet CI and CodeQL require approval and have zero jobs. Only labeling passed.

Validation: All 53 native S3 tests passed with --no-default-features and isolated AWS settings. All eight Java adapter tests passed after separate compilation against Spark 4.1.3, Hadoop 3.4.2 and AWS SDK 2.31.51. Compared 44 offline request URLs with the Java SDK: 42 matched, and both differences were reproduced unchanged at base. Rust formatting and diff checks passed. Full-feature native, Maven reactor, Spark SQL, MinIO/JNI integration, live storage and benchmarks were not run. Markdown formatting was not checked because the formatter was unavailable and its package download was blocked.

[sunchao]

Summary

• Prior state and problem: Native S3 scans rejected Hadoop’s profile provider and incorrectly applied fs.s3a.path.style.access.
• Design approach: Resolve profiles through the existing Hadoop credential adapter and pair normalized endpoints with their effective addressing mode.
• Correctness / compatibility analysis: No introduced P1/P2 issues found within this review. Earlier concrete concerns are addressed, although their threads remain open. Checked configuration forwarding against Spark 3.4.3, 3.5.9, 4.0.4, 4.1.3 and 4.2.0 sources, plus relevant Hadoop and AWS SDK behavior.
• Key design decisions: Explicit provider classes and blank opt-outs retain precedence. Delegating profile resolution to Hadoop avoids maintaining a separate Rust resolver. Per-bucket configuration remains part of cache identity.
• Implementation sketch: Native lookup selects the adapter and forwards fs.s3a.* through the existing JNI bridge. Endpoint processing occurs during store construction. Static profile credentials require per-request JNI lookups, while expiring credentials use the existing cache. Arrow ownership is unchanged. No reproducible P1/P2 performance regression was established.
• Behavioral changes worth calling out: Compared with branch-1.1 at e9efd9f764ee0a59b7898ff028d6985d4a7a28e1, profile support and default virtual hosting are intended changes. The Comet 1.2.0 migration entry covers custom and default AWS endpoints. Hadoop version, class-loader requirements and fs.s3a.path.style.access=true are documented.
• Suggested improvements: No additional P1/P2 changes requested.

Reviewed the entire nine-file diff from 8c783aa88104616dcf0f7876b4f7a9ba71e2bf31 to 76ed24967d5f81407db03c7357a7d42ef063b049. The PR remains open and non-draft. Routed skills: review-comet-pr and review-comet-ffi-pr.

Exact-head CI: Comet CI and CodeQL require approval and have zero jobs. Only labeling passed.

Validation: All 53 native S3 tests passed with --no-default-features and isolated AWS settings. All eight Java adapter tests passed after separate compilation against Spark 4.1.3, Hadoop 3.4.2 and AWS SDK 2.31.51. Compared 44 offline request URLs using current-source probes and locked dependencies: 42 matched the Java SDK, and both differences were reproduced unchanged at base. Rust formatting and diff checks passed. Full-feature native, Maven reactor, Spark SQL, MinIO/JNI integration, live storage, benchmarks and Markdown formatting were not run.

[sunchao]

Summary

• Prior state and problem: Native S3 scans rejected Hadoop’s ProfileAWSCredentialsProvider and inverted fs.s3a.path.style.access, effectively forcing custom endpoints into path-style addressing.
• Design approach: Route Hadoop profile credentials through the existing JVM adapter and return normalized endpoints together with their effective addressing mode.
• Correctness: Checked provider precedence, bucket overrides, blank opt-outs, boolean parsing, and final request URLs against Hadoop and AWS SDK behavior. Delegating profile resolution to Hadoop addresses the earlier file-selection, credential-precedence, and STS concerns. No introduced P1/P2 issues found within this review.
• Compatibility analysis: Checked configuration forwarding against Spark 3.4.3, 3.5.9, 4.0.4, 4.1.3, and 4.2.0 sources. Hadoop’s profile provider requires 3.4.2+. The default dependency now matches Spark 4.1, while other profiles retain their previous versions. Class-loader requirements and unsupported delegation-token configurations are documented.
• Key design decisions: Explicit Comet provider classes and blank opt-outs take precedence over automatic routing. SDK profile-provider aliases retain their existing native behavior. Full forwarded configuration remains part of store identity.
• Implementation sketch: lookup_provider_class selects the adapter, which builds Hadoop’s provider list on the executor through the existing credential bridge. Endpoint normalization applies hostname and scheme eligibility before configuring object_store. Tests cover routing, profile resolution, and addressing.
• Performance: Added configuration parsing occurs during store construction. Static profile credentials require per-request JNI resolution because they carry no expiry. Expiring credentials use the existing cache. No reproducible P1/P2 performance regression was established, and no benchmark was run.
• Design: Reusing Hadoop’s provider list keeps credential semantics with their existing implementation and avoids maintaining another profile resolver in Rust. The addressing changes remain localized.
• Abstraction & complexity: NormalizedEndpoint usefully keeps the endpoint and addressing flag consistent. Existing adapter and bridge abstractions suffice. Arrow ownership and batch transfer are unchanged.
• Behavioral changes worth calling out: Compared with branch-1.1 at e9efd9f764ee0a59b7898ff028d6985d4a7a28e1, profile support and default virtual hosting are intended changes. The Comet 1.2.0 migration entry covers custom and default AWS endpoints, including fs.s3a.path.style.access=true for deployments requiring path-style requests.
• Suggested improvements: No additional change meeting the P1/P2 reporting bar was identified. Earlier concrete blockers are addressed, although discussion threads remain formally open.

Reviewed the entire nine-file diff from b56349697b786ff2ad1c1bcf6ecf45b809af5f30 to 143ea1d15c393ba4d5f4dbc0b10475e96ad064a3. The PR remains open and non-draft. Read existing reviews, conversation comments, inline comments, and threads. Routed skills: review-comet-pr and review-comet-ffi-pr.

Exact-head CI: Comet CI and CodeQL require approval and have zero jobs. Only labeling passed. There is no product CI verdict.

Validation: All 53 native S3 tests passed with --no-default-features. All eight Java adapter tests passed after separate compilation against Spark 4.1.3, Hadoop 3.4.2, and AWS SDK 2.31.51. Source-matched base/head probes compared 44 offline request URLs: 42 matched the Java SDK, and two differences were reproduced unchanged at base. Rust formatting, diff checks, and suite registration checks passed. Full-feature native, Maven reactor, Spark SQL, MinIO/JNI integration, live object-store tests, benchmarks, and Markdown formatting were not run.