[fix](catalog) Restore DLF support for Iceberg and Paimon - #67545
[fix](catalog) Restore DLF support for Iceberg and Paimon#67545Gabriel39 wants to merge 5 commits into
Conversation
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
|
run buildall |
|
/review |
There was a problem hiding this comment.
I found ten correctness and lifecycle issues in the restored DLF paths and am requesting changes. The highest-impact failures are inconsistent credential aliases, incompatible endpoint/region validation, cross-catalog/stale Paimon client reuse, unreleased Iceberg DLF resources, unintended CREATE TABLE routing, mixed-format table discovery, and a no-op explicit connectivity test.
Critical checkpoint conclusions:
- Goal and proof: The patch restores provider registration and catalog construction for Iceberg/Paimon DLF, but the end-to-end goal is not yet met because valid credential/endpoint forms fail, catalog replacement can reuse or leave clients open, and several runtime operations violate their contracts. The added tests prove dispatch and small property/helper cases only.
- Scope and focus: The changes are DLF-focused, but they reintroduce a sizeable legacy catalog implementation instead of reusing Iceberg HiveCatalog semantics; the resulting deviations are called out inline.
- Concurrency: No new locks or explicit worker threads are added. Caffeine itself is thread-safe, but Paimon's JVM-static pool creates cross-catalog/configuration isolation failures rather than a lock-order bug. No deadlock path was found.
- Lifecycle: Not correct. Iceberg connector closure reaches only BaseMetastoreCatalog's metrics close, leaving DLF FileIO and metastore clients open; Paimon's static pool also survives catalog generations.
- Configuration and dynamic changes: DLF aliases and endpoint derivation are added. ALTER/recreation with the same DLF catalog id is unsafe because the Paimon cache identity excludes endpoint/credential/proxy settings; the metastore/storage alias sets disagree; and endpoint-only DLF configuration cannot supply native OSS's required region.
- Compatibility and rolling upgrade: No FE/BE wire format, storage format, or symbol contract changes were found. The Hive2/Hive3 DLF shade dependencies and relocations match their respective plugin families, but backward-compatible property aliases are incomplete at the storage boundary.
- Parallel paths: Iceberg and Paimon providers are registered in both ServiceLoader files. The parallel OSS and OSS-HDFS binders both miss the same DLF secret/token aliases, while only OSS-HDFS understands DLF endpoints for region derivation; the new Iceberg DLF remote flavor is also omitted from the metastore connectivity-probe set.
- Conditional checks: The type-routing and OSS-provider gates are understandable, but endpoint conversion is not idempotent, endpoint-or-region validation disagrees with storage requirements, and the connectivity-test condition excludes DLF. No other conditional-path defect survived review.
- Test coverage: Unit tests cover provider dispatch, validation, derived endpoints, flavor resolution, and two endpoint conversions. They do not exercise STS alias propagation, endpoint-only storage binding, same-id catalog/ALTER isolation, catalog close, SQL DDL, mixed-format listing, namespace metadata/errors, explicit connectivity failure, or already-prefixed endpoints.
- Test results: I did not run builds or tests because this review runner explicitly forbids them. No generated regression result files are changed.
- Observability: Existing connector exception/log boundaries are generally adequate, but the unintended CREATE TABLE path replaces a stable unsupported-operation result with an unrelated URI/I/O failure. No new metrics are necessary for the intended feature.
- Transactions and persistence: No Doris EditLog or transaction-state changes are present. External DLF metadata writes are only relevant through the newly exposed CREATE TABLE path, which must be gated or fully implemented.
- Data writes: The intended restoration is primarily read/catalog access. CREATE TABLE is nevertheless reachable and fails before metastore commit after constructing a malformed metadata path; this is called out inline.
- FE/BE variables: No new thrift variables or protocol fields are introduced. Existing BE storage credentials are affected by the missing token alias propagation.
- Performance: No new hot-loop or asymptotic issue was found. The material resource risks are the static pool's incorrect reuse and the Iceberg client's missing deterministic closure after catalog replacement.
- Other issues: Mixed-format table filtering, namespace location/error semantics, and S3-compatible endpoint normalization have separate inline comments.
- User focus: No additional focus was supplied; the complete PR was reviewed.
- Review completion: Complete for head d11010b after three full rounds, including normal and risk-focused convergence; every candidate was accepted, merged, or dismissed with evidence.
TPC-H: Total hot run time: 16778 ms |
TPC-DS: Total hot run time: 83196 ms |
ClickBench: Total hot run time: 14.87 s |
### What problem does this PR solve? Issue Number: None Related PR: apache#67545 Problem Summary: Restored DLF catalogs did not keep metastore and OSS credential aliases aligned, could reuse stale Paimon DLF clients across catalog configurations, omitted endpoint-derived OSS regions, leaked catalog-owned resources, exposed non-Iceberg tables, dropped namespace metadata, skipped connectivity checks, and lost legacy error contracts. This change aligns DLF storage binding, isolates client pools with a configuration fingerprint, restores Iceberg-compatible metadata and lifecycle behavior, rejects unsupported DLF table creation before remote access, and adds regression coverage. ### Release note Improve Iceberg and Paimon DLF catalog compatibility, validation, metadata listing, and resource cleanup. ### Check List (For Author) - Test: Regression test and Unit Test - Ran test_dlf_catalog locally in generated-output and comparison modes. - Ran 209 focused FE unit tests with zero failures or errors. - Ran Connector SPI tests and packaged Iceberg/Paimon connectors with Maven install. - Ran the full FE build and FE Checkstyle validation. - Behavior changed: Yes. DLF aliases, endpoint inference, connection checks, read-only DDL validation, mixed-format filtering, namespace metadata, pool isolation, and cleanup now follow the supported contracts. - Does this need documentation: No
|
run buildall |
|
/review |
TPC-H: Total hot run time: 16648 ms |
There was a problem hiding this comment.
I found nine distinct issues on head 02284619614253ec8855f1e0bc58a5c1ba636ea2 and am requesting changes. The highest-impact failures are provider-independent disclosure of a newly accepted session-token alias and Paimon's no-op explicit DLF connectivity test. The remaining issues affect rooted DLF namespaces, namespace-scale table listing, OSS-HDFS endpoint normalization, Iceberg base catalog state, public/VPC endpoint parity, CREATE/IF NOT EXISTS ordering, and directory-plugin classloader isolation.
Critical checkpoint conclusions:
- Goal and proof: The patch substantially restores Iceberg and Paimon DLF provider registration, clients, storage binding, packaging, read-side catalog access, connection validation, and unsupported catalog DDL handling. The ordinary paths have focused unit/regression coverage, but the nine inline counterexamples show the end-to-end goal is not yet complete.
- Scope and focus: All 47 changed paths are cohesive with DLF restoration or the required connector SPI, shade/assembly, and test infrastructure. The generic provider preflight is the broadest change and accounts for the two framework-level findings.
- Concurrency: Iceberg catalog publication, generation leases, checked-out DLF clients, cache eviction, ALTER reset, and concurrent close were traced. Existing synchronization and retirement/drain behavior prevent a distinct race, deadlock, lock-order, or use-after-close issue.
- Lifecycle and static state: FileIO, catalog, cached client pools, individual clients, metrics reporter, authentication scope, and TCCL were followed through initialization, partial failure, reset, and close. Partial-construction cleanup remains covered by the existing review thread; the new defects are dropped base state and an unpinned provider callback, not a separate lifetime race. Paimon's library-owned static cache is identity-scoped and expiry-closed.
- Configuration and dynamic changes: Legacy/canonical credentials, STS, endpoint/region/public selection, OSS versus OSS-HDFS, arbitrary Iceberg options, raw-map replay, and ALTER recreation were checked. These are catalog-lifecycle properties rather than live process settings. The remaining configuration defects are the rooted-namespace mapping, degraded secret masking, endpoint conversion, lost base options, and boolean-parser mismatch.
- Compatibility: The default SPI method, frozen surface, API-major 6-to-7 transition, and plugin manifests are aligned; no journal, storage, Thrift, or FE/BE wire format changes are present. The remaining compatibility issue is that a compatible directory provider's new callback runs outside its defining TCCL.
- Parallel paths: Iceberg/Paimon, native OSS/OSS-HDFS, endpoint/region, static/session credentials, loaded/degraded providers, root/no-root namespaces, CREATE/CTAS/IF NOT EXISTS, and built-in/directory plugins were reviewed separately. Their residual asymmetries are represented by the inline findings or hard-fenced existing threads.
- Conditional checks: Flavor/provider gates and normal DLF rejection are understandable. The unsound branches are the case-sensitive endpoint conversion, inconsistent boolean parsing, and branch-insensitive CREATE preflight ordering.
- Test coverage: Tests cover normal aliases, STS propagation, lowercase endpoints, literal booleans, provider dispatch, API/version surfaces, pool identity, mixed-format filtering, namespace errors, close behavior, loaded-provider masking, direct CREATE rejection, and Iceberg connection failure. Missing counterexamples map directly to the nine findings: rooted namespaces, large mixed namespaces, degraded masking, mixed-case OSS-HDFS, Paimon
test_connection=true, custom base properties, nonliteral truthy values, CTAS/IF NOT EXISTS ordering, and a directory-provider CREATE callback. - Test results: I did not run builds or tests because this review runner explicitly prohibits them. Static inspection found the checked-in generated regression oracle deterministic and consistent. At the final live check, style, license, secret, dependency, and change-detection checks passed; FE UT, compile, and performance were still pending.
- Error handling and observability: DLF interruptions restore the flag; remote construction/probe failures retain causes and side-specific context; close aggregates failures. Remaining wrong outcomes are the empty rooted catalog, false Paimon connectivity success, wrong CREATE/no-op ordering, and directory-helper error, all covered inline. No additional metric was substantiated.
- Transactions and persistence: Catalog properties continue through the existing raw-property journal/replay and connector reset flow; no new EditLog or transaction protocol is introduced. The degraded persisted-property display issue is called out inline.
- Data writes and modification: DLF catalog/namespace DDL remains unsupported and no new Doris transaction format is added. A final hypothesis that generic existing-table DML violated this contract was dismissed: the exact pre-refactor DLF implementation already used writable
HiveTableOperations, installed the generic Iceberg transaction manager, and exposed the same sinks, so those commits are restored behavior rather than a new regression. The remaining mutation-adjacent defect is the branch-aware CREATE/IF NOT EXISTS ordering. - FE/BE variables: No new cross-process field exists. Existing bound storage maps carry endpoint, credentials, token, and test location; the two remaining route inconsistencies are described inline.
- Performance and memory: Lazy construction and pooling are appropriate. The namespace-wide full-object DLF batch is the only substantiated new scale/peak-allocation issue; no additional hot-lock, retry amplification, or per-row cost survived review.
- Other/security:
SECURITY.mdand the repository threat model were applied. The provider-independent STS-token display path is the only additional security-sensitive issue; no authentication, authorization, tenancy, or unsafe-network-boundary finding survived. This Java-only diff has no BE nullable-column, allocator, cross-TU, or memory-order checkpoint. - User focus: No additional review focus was supplied; the complete PR was reviewed.
- Review completion: Complete after three rounds of normal and separate risk-focused review. Every candidate is accepted here, hard-duplicate-fenced by a live thread, or dismissed with concrete code/history evidence; no suspicious point remains unresolved.
TPC-DS: Total hot run time: 81267 ms |
ClickBench: Total hot run time: 14.58 s |
FE UT Coverage ReportIncrement line coverage |
FE Regression Coverage ReportIncrement line coverage |
### What problem does this PR solve? Issue Number: None Related PR: apache#67545 Problem Summary: Follow-up review found that DLF table listing still needed bounded metadata filtering, Iceberg base catalog properties were not retained, one token alias was not masked statically, public-endpoint boolean parsing diverged between metadata and storage, Paimon connectivity checks did not exercise DLF and storage, mixed-case DLF OSS-HDFS endpoints were misconverted, and CREATE TABLE preflight ordering and plugin classloading were incomplete. This change fixes those contracts and preserves IF NOT EXISTS no-op behavior. ### Release note Complete DLF validation, connectivity, metadata listing, credential masking, and CREATE TABLE compatibility for Iceberg and Paimon. ### Check List (For Author) - Test: Regression test and Unit Test - Ran the focused Iceberg, Paimon, filesystem, plugin, DDL-routing, and CTAS unit tests. - Ran test_dlf_catalog locally: 1 suite, 0 failed, 0 fatal. - Ran Maven install for Iceberg and Paimon connectors. - Ran the full FE build and FE Checkstyle validation. - Behavior changed: Yes. DLF connection validation, metadata batching, endpoint parsing, masking, and DDL preflight now enforce the supported contracts. - Does this need documentation: No
|
run buildall |
|
/review |
There was a problem hiding this comment.
Review status: changes requested on exact head 05a951692df6e46c379e7eb8dd92a9c5bf993bc2.
New finding:
- P1: Paimon DLF now accepts an OSS-HDFS-only binding, but the default native reader cannot route ordinary
oss://bucket/...warehouse, data, or deletion-vector paths to it. A separate inline comment is attached atPaimonConnector.java:494.
Existing exact-head threads remain applicable, so I did not duplicate them inline: production Paimon CREATE validation still skips both storage legs; Iceberg CREATE validation still skips its BE probe; and DLF catalog construction can still strand FileIO on a client-pool constructor failure.
Critical checkpoint conclusions:
- Goal and proof: The change substantially restores DLF metadata, property, packaging, and scan support, but does not complete the goal because ordinary Paimon DLF-on-OSS-HDFS native reads fail before I/O. The added unit and p0 tests do not exercise that real adapter-selection seam.
- Scope and clarity: The patch is broad because it restores two connector stacks and packaging, but the changes are internally focused and generally follow existing abstractions. The accepted routing condition is the material clarity/correctness exception.
- Concurrency: Lazy catalog publication uses synchronized/volatile state; Iceberg resource leases retain active generations; client pools and filesystem caches use their established concurrency controls. I found no changed lock-order or deadlock defect, and setup work is not added to scan loops.
- Lifecycle: Connector replacement, cache retirement, checked-out clients, and normal close are covered. The pre-publication FileIO cleanup gap remains, but is already covered by the live partial-initialization thread. No C++ cross-TU static initialization is involved.
- Configuration: The new keys are catalog properties rather than dynamically reloadable process configuration. CREATE, ALTER, and replay re-enter the binders; changed identities separate catalog/client generations. No restart-sensitive dynamic-config issue was found.
- Compatibility: The ConnectorProvider surface change is paired with API major 7, surface-baseline updates, manifest stamping, and fail-closed version gating. No storage-format or FE-BE wire-format change is introduced; incompatible external plugins are rejected explicitly.
- Parallel paths: Iceberg/Paimon, FE/BE probes, native/JNI scans, and OSS/OSS-HDFS were traced. The Paimon OSS-HDFS native path is the new P1; the two production probe omissions are already live-thread duplicates.
- Conditional logic: CREATE/CTAS/IF NOT EXISTS ordering and provider TCCL conditions are documented and preserve no-op/race behavior.
hasDlfCompatibleStorageaccepts OSS-HDFS without ensuring that plain OSS URIs can select that binding, which is the inline issue. - Test coverage: Unit tests cover aliases, endpoint derivation, TCCL, pool identity, DDL rejection, lifecycle, and masking; remote suites cover real DLF and native/JNI reads. Missing real lightweight-validation contexts and an OSS-HDFS-only plain-URI oracle leave the known probe gaps and new P1 unproved.
- Test results: The modified regression output is consistent with the corresponding negative and masking queries. No incorrect handwritten expectation was found.
- Observability: Existing exceptions retain catalog/location context and the routing failure names the unresolved schema. No new hot INFO logging or critical metric requirement was identified.
- Transactions and persistence: CREATE/ALTER/replay and IF NOT EXISTS race ordering were traced. ALTER validates a detached candidate before journaling, and this patch adds no new edit-log payload or failover-sensitive transaction state.
- Data writes: DLF catalog-structure writes remain rejected before remote work; CTAS executes its sink only when this statement owns table creation. Existing-table Iceberg commit behavior is preserved, with no new atomicity or crash-leak issue found beyond the live lifecycle thread.
- FE-BE propagation: Storage credentials/configuration continue through the existing typed maps and thrift paths; no new wire variable is added. The skipped BE validation legs are already covered by existing threads.
- Performance: Configuration hashing and catalog setup are bounded, DLF table listing is chunked to 100 descriptors, and no new CPU/memory hot-path regression was found. The accepted issue is correctness, not performance.
- Other issues: API/shade/ServiceLoader closure, masking, authentication, TCCL restoration, namespace/error mapping, and security boundaries were rechecked. No additional distinct finding survived duplicate fencing; this is a correctness/availability review, not a security-vulnerability report.
Convergence: Round 2 completed with three normal full reviews and a separate adversarial risk pass, all ending NO_NEW_VALUABLE_FINDINGS beyond the accepted/duplicate-fenced records. No additional user focus was supplied, so the full PR was reviewed.
Validation note: static review only, as required by the runner prompt; no local builds or tests were run. At submission time, style, license, secrets, coverage, BE UT, and Cloud UT were passing; compile, FE UT, performance, and the code-review check were still pending.
TPC-H: Total hot run time: 16748 ms |
TPC-DS: Total hot run time: 81438 ms |
ClickBench: Total hot run time: 14.75 s |
FE UT Coverage ReportIncrement line coverage |
FE Regression Coverage ReportIncrement line coverage |
### What problem does this PR solve? Issue Number: None Related PR: apache#67545 Problem Summary: In an OSS-HDFS-only catalog, plain oss:// bucket paths were classified as native OSS because the path itself did not contain an oss-dls endpoint. Native OSS was intentionally absent from the catalog adapter map, so URI normalization failed before scans; retaining only the adapter selection would also lose the Jindo/HDFS identity at the connector string boundary. Fall back to the selected OSS-HDFS adapter when native OSS is absent and embed its configured endpoint in normalized plain bucket URIs so data and deletion-vector paths remain FILE_HDFS downstream. ### Release note Fix DLF-backed OSS-HDFS scans for plain oss:// bucket paths. ### Check List (For Author) - Test: Unit Test and Regression test - DefaultConnectorContextNormalizeUriTest and OssHdfsPropertiesTest - external_table_p0/test_dlf_catalog - Behavior changed: Yes. Plain oss:// paths in OSS-HDFS-only catalogs are normalized through Jindo and routed as FILE_HDFS. - Does this need documentation: No
|
run buildall |
|
/review |
|
Codex automated review failed and did not complete. Error: Selected model is at capacity. Please try a different model. Please inspect the workflow logs and rerun the review after the underlying issue is resolved. |
|
/review |
FE UT Coverage ReportIncrement line coverage |
|
Codex automated review failed and did not complete. Error: Review step was failure (possibly timeout or cancelled) Please inspect the workflow logs and rerun the review after the underlying issue is resolved. |
There was a problem hiding this comment.
Request changes: three distinct storage-routing regressions remain on this head: malformed OSS-HDFS normalization for native-qualified paths (P1), an Iceberg DLF connectivity probe that targets the wrong native-OSS bucket (P1), and mixed-case OSS-HDFS identity loss at BE dispatch (P2). Existing live-thread concerns were treated as duplicate fences and were not reposted.
Critical checkpoint conclusions:
- Goal and proof: The PR broadly restores DLF provider registration, aliases, Iceberg catalog support, Paimon proxying, and private dependency closures. Unit and P0 coverage exercises the baseline paths, but the three accepted URI shapes are not covered end to end, so the restoration is not yet correct for all supported inputs.
- Scope: The 63-file change is large but focused on the DLF restoration and its SPI/filesystem/test plumbing. The remaining defects sit at the shared URI and connectivity seams.
- Concurrency: Iceberg generation/resource tracking, Caffeine client pools, and Paimon's JVM-static pool were traced through concurrent use and retirement. No distinct race, lock-order, or deadlock issue survived; the partial-construction cleanup concern is already covered by live thread 3942659426.
- Lifecycle: Initialization, statement/table leases, ALTER/DROP generation rotation, eviction, close, and failure cleanup were reviewed. No new lifecycle issue survived beyond existing threads.
- Configuration and dynamic behavior: Legacy/canonical credentials, token masking, endpoint/region/public-VPC derivation, and ALTER rebinding were checked. No dynamic global configuration was added; catalog regeneration picks up property changes. Existing topology/boolean concerns remain in their live threads.
- Compatibility: The SPI v7 surface/version gate rejects older providers before use, and both Hive 2 and Hive 3 private shades contain the required relocated DLF/Thrift/Tea closure. No storage-format or new serialized protocol change is introduced.
- Parallel paths: Iceberg and Paimon data, position/equality-delete and deletion-vector scans, Iceberg writes, and FE/BE connectivity probes were traced. MAIN-1 and MAIN-2 cross several of these paths; MAIN-3 is isolated to the new Iceberg DLF validation probe.
- Special conditions: Provider priority, OSS versus OSS-HDFS exclusivity, endpoint case/idempotence, CREATE/CTAS, and IF NOT EXISTS paths were reviewed. Remaining preflight/TCCL variants are already covered by live threads 3943032141 and 3943032138.
- Test coverage: The PR adds broad provider/property/catalog/DDL/connectivity unit tests and a P0 catalog suite. It lacks mixed-case DLS data/delete/write dispatch, native-OSS-qualified OSS-HDFS normalization, and endpoint-qualified native-OSS connectivity cases corresponding to the three findings.
- Test results: The checked-in
.outmatches the two single-row SHOW CREATE cases and carries the generator header. Current CI shows COMPILE, FE UT, CheckStyle, Clang Formatter, license, secrets, and dependency review passing;vault_p0is failing and the other regression groups are still pending. No local build or test was run because the review prompt forbids it. - Observability: Probe failures log the catalog and distinguish FE from compute-node failures; no new metric appears necessary. The accepted routing defects can nevertheless surface as misleading access failures after constructing the wrong path/bucket.
- Transactions and persistence: Catalog replay, property persistence, and ALTER generation behavior use the existing mechanisms; there is no new EditLog or transaction-format change, and no distinct persistence issue was found.
- Data writes and atomicity: Iceberg commit semantics are unchanged. The routing defects can prevent access before a successful write/commit, but no additional partial-write or atomicity defect was found.
- FE-BE transmission: No new Thrift field is added. Existing file-type, normalized-location, and backend-property seams are used, but MAIN-1 misclassifies a path and MAIN-2/MAIN-3 send incorrect locations for specific supported forms.
- Performance: Scan-scoped normalization memoizes adapter construction, listing work is bounded, and no distinct CPU, allocation, or hot-path regression survived review.
- Error handling: Provider and probe failures propagate as connector failures with context; no ignored Status path was introduced. The accepted bugs are incorrect URI construction/classification rather than swallowed errors.
- Memory safety and BE nullable handling: No BE code or nullable-column logic changes in this PR; no new tracked-allocation or ownership issue was found.
- Other issues: The apparent missing-Tea shade closure was disproved against the exact vendor shaded classifier. All other candidates were either dismissed with code evidence or deduplicated against live threads.
User focus: No additional review focus was specified; the full PR was reviewed.
Review completion: capped/incomplete after the maximum three rounds because MAIN-3 was newly accepted in Round 3. Every currently known candidate has a final disposition and the three accepted issues are reported here, but the review contract does not permit a fourth convergence round.
Issue Number: None Related PR: apache#67545 Problem Summary: Restored DLF catalogs did not keep metastore and OSS credential aliases aligned, could reuse stale Paimon DLF clients across catalog configurations, omitted endpoint-derived OSS regions, leaked catalog-owned resources, exposed non-Iceberg tables, dropped namespace metadata, skipped connectivity checks, and lost legacy error contracts. This change aligns DLF storage binding, isolates client pools with a configuration fingerprint, restores Iceberg-compatible metadata and lifecycle behavior, rejects unsupported DLF table creation before remote access, and adds regression coverage. Improve Iceberg and Paimon DLF catalog compatibility, validation, metadata listing, and resource cleanup. - Test: Regression test and Unit Test - Ran test_dlf_catalog locally in generated-output and comparison modes. - Ran 209 focused FE unit tests with zero failures or errors. - Ran Connector SPI tests and packaged Iceberg/Paimon connectors with Maven install. - Ran the full FE build and FE Checkstyle validation. - Behavior changed: Yes. DLF aliases, endpoint inference, connection checks, read-only DDL validation, mixed-format filtering, namespace metadata, pool isolation, and cleanup now follow the supported contracts. - Does this need documentation: No
### What problem does this PR solve? Issue Number: None Related PR: apache#67545 Problem Summary: Follow-up review found that DLF table listing still needed bounded metadata filtering, Iceberg base catalog properties were not retained, one token alias was not masked statically, public-endpoint boolean parsing diverged between metadata and storage, Paimon connectivity checks did not exercise DLF and storage, mixed-case DLF OSS-HDFS endpoints were misconverted, and CREATE TABLE preflight ordering and plugin classloading were incomplete. This change fixes those contracts and preserves IF NOT EXISTS no-op behavior. ### Release note Complete DLF validation, connectivity, metadata listing, credential masking, and CREATE TABLE compatibility for Iceberg and Paimon. ### Check List (For Author) - Test: Regression test and Unit Test - Ran the focused Iceberg, Paimon, filesystem, plugin, DDL-routing, and CTAS unit tests. - Ran test_dlf_catalog locally: 1 suite, 0 failed, 0 fatal. - Ran Maven install for Iceberg and Paimon connectors. - Ran the full FE build and FE Checkstyle validation. - Behavior changed: Yes. DLF connection validation, metadata batching, endpoint parsing, masking, and DDL preflight now enforce the supported contracts. - Does this need documentation: No
### What problem does this PR solve? Issue Number: None Related PR: apache#67545 Problem Summary: In an OSS-HDFS-only catalog, plain oss:// bucket paths were classified as native OSS because the path itself did not contain an oss-dls endpoint. Native OSS was intentionally absent from the catalog adapter map, so URI normalization failed before scans; retaining only the adapter selection would also lose the Jindo/HDFS identity at the connector string boundary. Fall back to the selected OSS-HDFS adapter when native OSS is absent and embed its configured endpoint in normalized plain bucket URIs so data and deletion-vector paths remain FILE_HDFS downstream. ### Release note Fix DLF-backed OSS-HDFS scans for plain oss:// bucket paths. ### Check List (For Author) - Test: Unit Test and Regression test - DefaultConnectorContextNormalizeUriTest and OssHdfsPropertiesTest - external_table_p0/test_dlf_catalog - Behavior changed: Yes. Plain oss:// paths in OSS-HDFS-only catalogs are normalized through Jindo and routed as FILE_HDFS. - Does this need documentation: No
### What problem does this PR solve? Issue Number: None Related PR: apache#67545 Problem Summary: OSS-HDFS normalization appended the selected DLS endpoint to already qualified native OSS authorities, producing an invalid bucket name. It also preserved mixed-case DLS authorities that downstream file-type dispatch did not recognize, while the Iceberg DLF connectivity probe bypassed storage normalization. Extract the real bucket from qualified OSS authorities, rebuild the canonical DLS authority through the selected storage binding, and use that binding for Iceberg probe locations. ### Release note Normalize qualified native OSS and OSS-HDFS paths before DLF scan, write, and connectivity dispatch. ### Check List (For Author) - Test: Unit Test and Regression test - OssHdfsPropertiesTest, DefaultConnectorContextNormalizeUriTest, and IcebergConnectorTestConnectionTest - external_table_p0/test_dlf_catalog - Behavior changed: Yes. Qualified OSS authorities are canonicalized through the selected OSS-HDFS endpoint, including mixed-case inputs. - Does this need documentation: No
5993f0e to
272bed6
Compare
|
run buildall |
|
/review |
Gabriel39
left a comment
There was a problem hiding this comment.
Addressed the qualified OSS normalization and DLF connectivity probe findings.
|
Codex automated review failed and did not complete. Error: Your access token could not be refreshed because your refresh token was already used. Please log out and sign in again. Please inspect the workflow logs and rerun the review after the underlying issue is resolved. |
TPC-H: Total hot run time: 16706 ms |
TPC-DS: Total hot run time: 82402 ms |
ClickBench: Total hot run time: 14.55 s |
What problem does this PR solve?
Issue Number: None
Related PR: None
Problem Summary:
The connector metastore SPI refactor removed DLF provider registration and the plugin-local DLF client path, so existing Iceberg and Paimon catalogs using
dlfcan no longer be created or replayed on master.This PR restores DLF support for both connectors:
S3FileIO.ProxyMetaStoreClient, while preserving legacy alias-only OSS and OSS-HDFS storage configurations.Release note
Restore Aliyun DLF metastore support for Iceberg and Paimon catalogs.
Check List (For Author)
Test
IcebergWritePlanProviderTest.planMergePreservesExplicitlyEmptyReadAcrossConcurrentFirstAppendcase. The focused rerun reproduces the same failure, and this PR does not modify that code path.Behavior changed:
Does this need documentation?
Check List (For Reviewer who merge this PR)