Repository navigation
Conversation
Gabriel39
requested review from
924060929,
CalvinKirs,
Mryange and
yiguolei
as code owners
November 6, 2026 04:45
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Contributor
Author
|
run buildall |
Contributor
Author
|
/review |
13 tasks
### Background The native MongoDB connector registers its provider through Java ServiceLoader. Service descriptors support hash comments and are checked by the license workflow. ### What problem does this PR solve? Issue Number: apache#70251 Related PR: apache#70327 (introduced the service descriptor) PR Type: Bugfix Problem Summary: The license workflow rejects the new provider descriptor because it has no ASF license header. Add the same hash-comment header used by the other connectors. ### Details - Frequency: every license check of the original descriptor. - Root cause: the new META-INF/services resource contained only the provider name. - Why existing tests missed it: ServiceLoader accepts the unlicensed descriptor; runtime discovery tests do not validate source licensing. - Fix: add the ASF header as hash comments and document why that syntax is valid. - Other paths and branches: the existing connector descriptors already carry headers. ### Testing Skywalking-eyes v0.8.0 passed the PR incremental check with zero invalid files. All 26 MongoDB connector unit tests passed, including ServiceLoader discovery. FE Checkstyle passed with zero violations during Maven validate. ### Release note None. ### Check List (For Author) - Behavior changed: No. - Does this need documentation: No. - Pre-submission status: Ready; local license, unit and style checks pass. ### AI Disclosure - Tool: Codex (GPT-6). - Generated: the descriptor comment and commit message. - Human review: pending maintainer review.
Contributor
Author
|
run buildall |
Contributor
Author
|
/review |
Contributor
|
Codex automated review failed and did not complete. Error: You’ve hit your usage limit. Visit https://chatgpt.com/settings/usage to purchase more credits or try again at Nov 11th, 2026 6:35 AM. Please inspect the workflow logs and rerun the review after the underlying issue is resolved. |
…versions ### Background Native MongoDB scans decode BSON through Doris DataTypeSerDes. VARBINARY length is metadata in existing readers, while the new BSON path added a length check. ### What problem does this PR solve? Issue Number: apache#70251 Related PR: apache#70327 (introduces the native reader) PR Type: Bugfix Problem Summary: BSON ObjectId, BinData and raw documents can be rejected solely because their payload exceeds the declared VARBINARY length. Restore the existing metadata semantics and preserve all bytes. Expand regression coverage for NULL, missing, boundary and invalid values, and explain the BSON/reader/scan-descriptor contracts. ### Details - **Change map:** 106 added production/comment lines, 169 test/fixture lines, 60 generated regression-result lines and 2 documentation lines; 24 removed lines. - **Frequency:** whenever a valid BSON binary payload exceeds a declared length. - **Root cause:** the BSON-only SerDe length member enforced a restriction absent from other VARBINARY readers. - **Why existing tests missed it:** the previous BSON test expected this rejection; it did not compare the existing VARBINARY length semantics. - **Fix:** remove the length argument/check while retaining BSON tag/subtype checks, exact byte copying, and mismatch rollback. Document borrowed-value lifetimes, reader lifecycle, and every TMongoScanDesc field. - **Other paths and branches:** existing VARBINARY readers retain their behavior; no wire field IDs, types or requiredness change. ### Testing The two focused ObjectId/raw-BSON tests failed before the fix and passed after it. All 137 selected BE tests passed through run-be-ut.sh. All 26 MongoDB FE connector tests passed, with zero Checkstyle violations. The complete ASAN BE build passed. The SQL regression generated expected output and independently compared it against a recreated MongoDB 8.0.20 fixture: one suite passed, zero failed/fatal/skipped. Coverage includes explicit NULL versus absent keys, numeric extrema/nonfinite/ subnormal values, decimal overflow and rounding, datetime boundaries and precision, ObjectId/BinData bytes and subtype mismatches, strings, nested values and document limits. Clang-format 16, clang-tidy and build hygiene passed. ### Release note MongoDB binary decoding preserves full ObjectId, BinData and document bytes regardless of the declared VARBINARY length. ### Check List (For Author) - Behavior changed: Yes (see Details). - Does this need documentation: Yes; connector README updated. - Pre-submission status: Ready; local checks passed, CI pending. ### AI Disclosure - Tool: Codex (GPT-6). - Generated: code, comments, tests, fixtures, documentation and this message. - Human review: pending maintainer review.
Contributor
Author
|
run buildall |
Contributor
Author
|
/review |
### Background The native MongoDB Catalog discovers logical namespaces through the Java driver and reads BSON through the native driver and Doris SerDes. Auto schema mode must retain every declared value through a complete typed schema or document fallback. ### What problem does this PR solve? Issue Number: apache#70251 Related PR: apache#70327 (introduces the affected native paths) PR Type: Bugfix Problem Summary: Wide BSON validation can allocate tens of MiB outside query memory accounting. Metadata continuation can reach the wrong mongos, URI routing criteria are lost, and valid unsupported validators can either hide fields or fail auto resolution. Bound traversal scratch, delegate metadata cursor ownership to the pinned driver, preserve full read preferences, and reject unrepresentable validators consistently. ### Details - **Change map:** 131 added production/build lines, 298 test/fixture lines, 51 generated regression-result lines and 17 documentation lines; 146 removed lines. - **Frequency:** wide nested documents; paginated metadata across multiple mongos; URI tag/max-staleness settings; tuple, pattern and conditional-field validators. - **Root cause:** a width-sized untracked worklist, independent raw cursor commands, mode-only read preferences and an incomplete typed validator grammar. - **Why existing tests missed it:** standalone metadata fakes assumed that session identity implied server affinity; routing tests checked only mode names, schema tests missed valid tuple/pattern forms, and traversal had no allocation probe. - **Fix:** use a fixed-depth iterator stack; retain driver-owned BatchCursor connection sources and cleanup; copy complete URI preferences in FE and BE; turn tuple and schema-bearing constructs into the existing whole-collection fallback in auto mode and explicit errors in validator mode. Preserve URI precedence, metadata primary reads, permissions and object kinds. - **Other paths and branches:** recursively check root/nested/array-element schemas; change no existing connector path, Thrift field layout or capability negotiation. ### Testing Focused tests reproduced the original schema/routing failures and a 48 MiB BSON validation allocation. The old live implementation reproduced CursorNotFound while listing 140 collections across two mongos routers. All 31 FE connector tests and 142 selected BE tests passed; the refactored depth test passed a focused rerun. Complete FE and ASAN BE builds passed, with zero FE Checkstyle violations. Clang-format 16, clang-tidy and header hygiene passed for affected files. The MongoDB SQL suite generated expected output and independently compared it: one suite passed, zero failed/fatal/skipped, including fallback values, view-only permissions, a 900,000-container scalar scan and metadata batch boundaries. The fixed two-router implementation completed 20 full 140-name listings. Matching URI tags returned the scalar value; unmatched tags timed out instead of reading an ineligible member. Maximum staleness preservation is covered by both driver unit tests; this is not a failover or soak qualification. ### Release note Preserve MongoDB routing criteria and metadata cursor affinity, fall back completely for unsupported validator schemas, and bound wide BSON validation scratch memory. ### Check List (For Author) - Behavior changed: Yes (see Details). - Does this need documentation: Yes; connector and fixture READMEs updated. - Pre-submission status: Ready; local checks passed, CI pending. ### AI Disclosure - Tool: Codex (GPT-6). - Generated: implementation, comments, tests, fixtures, documentation and this message. - Human review: pending maintainer review.
Contributor
Author
|
run buildall |
Contributor
Author
|
/review |
Contributor
Author
Local pipeline review — ✅ PASSschema: doris-repo-review/v1
status: PASS
pr: apache/doris#70327
commit: 2134c7569c7ae93fa981fc1695ccfed680726235
base: 59c49656240451ed1caab1e69e69bbe0be8c61aa
reviewed_at: 2026-11-06T15:35+08:00
reviewer: Gabriel39
model: gpt-6-astra
effort: xhigh
findings: {blocker: 0, major: 0, minor: 0, nit: 0}
rounds: 1
converged: trueNotes for maintainers
Reviewed locally with the |
Contributor
|
Codex automated review failed and did not complete. Error: Review recovery stopped: shared time budget or helper timeout exhausted Please inspect the workflow logs and rerun the review after the underlying issue is resolved. |
Contributor
FE UT Coverage ReportIncrement line coverage |
Contributor
TPC-H: Total hot run time: 28812 ms |
Contributor
TPC-DS: Total hot run time: 154360 ms |
Contributor
ClickBench: Total hot run time: 24.09 s |
924060929
approved these changes
Nov 9, 2026
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Background
The native MongoDB dependency preparation (#70249) provides the FE Java driver and BE C driver, but does not register a usable Catalog. Doris needs a native metadata and scan path that preserves BSON types and reads collection, view, and time-series namespaces directly.
What problem does this PR solve?
Issue Number: #70251
Related PR: #70249 (client dependencies)
PR Type: Feature
Problem Summary:
Implement D01, D03, and D04 of the native MongoDB Catalog roadmap, plus the minimal single-Split descriptor needed to connect them. Users can create a
type=mongodbCatalog and query ordinary collections, standard views, and time-series measurements through a native C++ reader. FE pins explicit, validator, or document schemas; concreteDataTypeSerDeimplementations convert BSON without losing int64 precision, ObjectId bytes, or binary subtype identity.Details
findaccess; internal time-series buckets are hidden. All SQL filters, LIKE/REGEXP, joins, windows, aggregates, sorting, and LIMIT are evaluated in Doris. Predicate/projection pushdown, parallel range planning, common snapshots, and shared-source admission remain separate work items.TMongoScanDesc. BE owns bounded client-pool leases and cursors with RAII and closes them on EOF, early termination, cancellation, and errors. BSON conversions validate physical tags and exact numeric/date ranges; compound columns roll back on failure. The BSON helpers, MongoFileReader lifecycle, and all TMongoScanDesc fields document their ownership, units, scope, and constraints. Metadata pagination uses the pinned Java driver core operation and its server-affine cursor while retaining authorized/name-only listing and namespace kinds. BE inherits complete URI read preferences, including tags and max staleness. BSON validation uses a depth-bounded iterator stack; unsupported validator structures fall back as a whole in auto mode and fail clearly in validator mode. Canonical Extended JSON and raw BSON provide document access. Reader profiles expose read/decode/filter time, rows, and BSON bytes.CREATE CATALOG mongo_native PROPERTIES ("type"="mongodb", "mongodb.uri"="mongodb://mongo.example:27017", "mongodb.databases"="sample"). See the connector README for explicit schemas, credentials, supported mappings, and fixture setup. ObjectId usesVARBINARY(12); useFROM_BINARYfor supported hex/string analysis because direct VARBINARY operators retain their existing engine restrictions. Declared VARBINARY length remains schema metadata; BSON decoding preserves full ObjectId, BinData, and document payloads.autochooses a namespace's explicit mapping, a fully representable ordinary-collection validator, or document mode; it never samples source rows. All mapped fields are nullable and are not declared keys. Mismatches fail by default; optional top-level null substitution retains atomic nested writes. Dates use UTC. Batch size and query timeout use session settings; connection/read defaults belong to the Catalog.Testing
All checks below ran locally:
build.sh --fe) and complete ASAN BE build (build.sh --be).run-fe-ut.sh.run-be-ut.sh: 142 passed across BSON SerDe/reader/real-driver error paths and existing decoded-value/name/VARBINARY coverage. The focused ObjectId/raw-BSON length tests failed before the fix and passed afterward. New coverage verifies bounded BSON validation memory, depth and element limits, and preservation of complete URI read preferences; the depth-boundary test was rerun after its final refactor.CursorNotFoundon the old implementation; the updated implementation completed 20 consecutive full listings. Matching URI read-preference tags read successfully; unmatched tags correctly timed out instead of silently routing elsewhere. Unit tests also verify preservation of URI max staleness.Notes for Reviewers
Start with
MongoSchema,MongoScanPlanProvider,mongodb_reader.cpp, and the BSON SerDe tests. The regression suite covers all three logical object kinds, schema refresh, historical validator violations, source permissions, exact/special BSON values, nested rollback, SQL analysis, batch boundaries, and repeated early cursor closure. The current local integration environment uses MongoDB 8.0.20; MongoDB 7, full replica-set/mongos topology qualification (including failover and live staleness behavior), TLS/authentication matrices, performance, and soak evidence remain in the tracking issue. The two-mongos check above is a focused metadata-cursor and tag-routing test.Release note
Add a native read-only MongoDB Catalog supporting explicit/validator/document schemas, native BSON decoding, and single-cursor reads of collections, standard views, and time-series measurements.
Check List (For Author)
AI Disclosure