FANNG1 commented on PR #67157: URL: https://github.com/apache/doris/pull/67157#issuecomment-5453962752
@zhangstar333 Thanks for the review. All four P1 comments are addressed in 7205f2a and c67c552: 1. **Anonymous mode** — emit `allow_anonymous=true` when both keys are blank. 2. **Addressing style** — translate `oss.use_path_style` to `addressing_style`. I state both values explicitly rather than only `path`: lance snapshots the host's `OSS_`/`AWS_` environment variables into the same config map before storage options are applied, so leaving the default implicit would let an exported `OSS_ADDRESSING_STYLE` outrank an explicit catalog setting. 3. **Warehouse** — normalize the qualified `oss://bucket.oss-<region>.aliyuncs.com/path` form down to the bucket, and reject an object-store root with no bucket. OSS-HDFS (`oss-dls`) is exempt, since there the qualified authority is the required spelling. 4. **Redaction** — all provider failures now go through the runtime-option sanitizer, the cause is always rebuilt, and base-scoped spellings are matched by suffix. Note the pinned lance keys per-base configuration by a nested map rather than a flat dotted option, so the suffix match is defensive rather than a documented contract. Verified end to end against a real Alibaba Cloud OSS bucket on a native arm64 cluster. Both namespace implementations return `count(*) = 4, sum(amount) = 100`, with matching rows, type mapping and predicate pushdown: - **dir namespace** over an `oss://` warehouse with Doris OSS properties; - **REST namespace** against a real Apache Gravitino 1.3.0 `lance-rest` service, with **no static OSS credentials on the Doris catalog at all** — Gravitino vends the bare `endpoint` / `access_key_id` / `access_key_secret` / `region` spellings, which this PR normalizes before handing them to the BE; - the qualified `oss://bucket.<endpoint>/path` warehouse form also scans correctly, and `oss:/path` is rejected up front. Query profiles confirm the scans ran on the BE, not the FE: `FILE_SCAN_OPERATOR(table_name=all_types)` reports `RowsProduced: sum 4` on both paths. One gap worth flagging: `allow_anonymous` is covered by unit tests only. I don't have an anonymously readable OSS bucket to exercise it end to end, so its correctness rests on lance forwarding unrecognized options to OpenDAL plus OpenDAL's OSS config accepting that flag. On the docs — good point, I'll open a separate PR on `apache/doris-website` for the lance-catalog page (both the English and zh-CN copies), adding an OSS section alongside the existing S3 one, the `oss://` warehouse form, and how namespace-vended credentials interact with static ones. -- 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]
