Gabriel39 commented on issue #66492: URL: https://github.com/apache/doris/issues/66492#issuecomment-6017054822
Following up on the [design proposal](https://github.com/apache/doris/issues/66492#issuecomment-6011275985), I suggest narrowing Phase 1 to **multiple datasets with identical schemas**, with **any `Dataset.open()` failure failing the query**. Schema union can be implemented in a subsequent PR. The following are design risks and proposed requirements, rather than claims that every item is an existing implementation bug. 1. **Fail the query on any dataset-open failure.** An open failure may mean a permission error, timeout, corrupt metadata, or unsupported format; skipping it can silently return incomplete results. Fail for both explicit paths and glob matches, including ordinary directories matched by a glob. Report the failing path, operation, and underlying cause, without exposing credentials. Distinguish “no paths matched” from “a matched path could not be opened.” Update the proposal and tests to remove the skip-on-failure behavior. 2. **Define strict schema equality for Phase 1.** Validate all datasets in FE before dispatching scans. Compare the logical source schema recursively: exact field names/case, order, types and type parameters, nullability, and nested structure. Include decimal precision/scale, timestamp unit/timezone, and fixed-size list dimensions where applicable; comparing only converted Doris types can hide source differences. Dataset-local internal field IDs need not be equal. On mismatch, report both datasets, the field path, and the conflicting definitions. Preserve the existing strict BE converter checks; do not add NULL filling, type promotion, case merging, or nested schema union in this PR. 3. **Audit dataset-specific state and cache identities.** Identical logical schemas do not imply identical field IDs, fragment IDs, or index state. Keep each fragment associated with its dataset and version. Check whether metadata, reader, runtime-filter, and index-related caches are safe to share; state tied to a snapshot must distinguish dataset URI/version and any other relevant context. Verify how pushed expressions bind fields rather than assuming they are interchangeable. If a reader is reused across datasets, reset dataset-specific scanner/schema/binding state. This is an implementation audit requirement, not an assertion that the current caches are incorrect. 4. **Bound planning resources before materializing all results.** The proposed 1000-dataset cap is not a memory bound. In the referenced baseline, [`S3ObjStorage.listDirectories()`](https://github.com/apache/doris/blob/78aa9c996accd114a6e0f5943d7882ee1a34011a/fe/fe-core/src/main/java/org/apache/doris/fs/obj/S3ObjStorage.java#L228) traverses all pages and accumulates directory entries before returning. A post-listing cap is too late for a large prefix. Apply incremental limits to candidate enumeration, listing requests, and elapsed time. Apply the dataset cap across the entire TVF invocation, not separately to each input glob. Also budget total fragments/splits, plan size, and schema size: even one dataset can have many fragments. Bound metadata-open concurrency and account for heap, off-heap, and native allocations. Define precisely what each limit counts. 5. **Make cancellation and resource cleanup part of the execution model.** Listing, metadata opening, and retries should respect the query deadline/cancellation, with finite request timeouts. On one failure, cancel remaining work and close already-opened handles/allocators. Cover success, failure, timeout, and cancellation paths. Avoid a thread pool per dataset or an unbounded task queue; bound concurrent metadata work and release its resources promptly. 6. **Define deduplication for overlapping inputs.** For example, an explicit dataset URI and a glob may identify the same dataset. I suggest dataset-set semantics: safely normalize identities, deduplicate before opening/pinning versions, and read each dataset once. Do not deduplicate records or distinct datasets with identical contents. Use deterministic traversal order so reference-schema selection and diagnostics do not depend on listing order. 7. **Use an unambiguous multi-path representation.** Comma splitting conflicts with both object keys and brace patterns such as `s3://example-bucket/{a,b}`. Consider preserving the existing single-path `uri` behavior and adding a `uris` parameter containing a JSON array string; `Map<String, String>` does not prevent this representation. Define how `uri` and `uris` interact, parse glob syntax separately for each element, and specify literal wildcard escaping and brace-enumeration scope. Reject unsupported `**` and malformed patterns explicitly. Document path-level and trailing-slash behavior without breaking existing single-path inputs. 8. **Pin one coherent metadata snapshot per dataset.** Schema, version, and fragments must come from the same snapshot, rather than separate opens of latest. Resolve the dataset set once per query and reuse the resolved identities/versions for execution retries. If a pinned version is removed or becomes unreadable, fail rather than falling back to latest. Document that per-dataset snapshot consistency does not provide a transactionally consistent snapshot across datasets. 9. **Align path parsing, authorization, and actual access.** Listing, deduplication, authorization checks, and Lance open must agree on the object identity. Define the source of the authorized prefix and check scheme, bucket, and path-segment boundaries rather than a raw string prefix. Do not apply local-filesystem `../` normalization to object keys if it changes the addressed object. Keep credentials and signed URL secrets out of diagnostics. 10. **Validate each storage provider instead of assuming automatic coverage.** Normalizing a URI to `s3://` does not establish that FE listing and native Lance open agree on endpoint, authentication, addressing, and pagination. Preserve the provider information needed to build storage options. Claim support only for combinations validated through the full listing → metadata open → BE scan path. Reject unsupported cross-provider/cross-credential combinations during planning. 11. **Preserve existing single-dataset behavior.** Keep single-path parsing and routing compatible, retain the existing single-dataset `local()` path, and explicitly reject multi-dataset `local()` in this phase. Cover `s3()`, `file()` delegation, empty datasets/no fragments, `COUNT(*)`, LIMIT, and cancellation. With strict schema equality, no relaxation of the existing BE column checks should be necessary for schema union. Suggested minimum acceptance coverage: - Same-schema datasets produce the same results as separate scans combined with `UNION ALL`, after input dataset deduplication. - Any open failure fails the query and identifies the path; no candidate is silently skipped. - Schema mismatches fail in FE with precise differences, including nested/type-parameter differences. - Overlapping explicit/glob inputs read each dataset once; parsing tests cover commas, braces, escaping, and invalid patterns. - Large listings with few matches still hit enumeration budgets; few datasets with many fragments still hit planning budgets. - Cancellation and partial metadata-open failure release resources and stop outstanding work. - Concurrent dataset updates preserve pinned-version reads; deleted pinned versions fail explicitly. - Results remain equivalent with applicable filter pushdown enabled/disabled and with runtime filters, including datasets with different internal field IDs or index layouts. - Existing single-dataset and `local()` behavior remains compatible; each claimed provider passes the end-to-end path. This keeps the first implementation focused on path expansion, deduplication, strict schema validation, per-dataset version pinning, and bounded scan planning. NULL filling, type promotion, case reconciliation, and nested schema evolution can be designed and tested separately in the schema-union PR. -- 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]
