Skip to content

[refactor](scan) Remove legacy file scanner and deprecate rollout option - #69723

Merged
Gabriel39 merged 12 commits into
apache:masterfrom
Gabriel39:dev/remove-file-scanner-v1
Nov 2, 2026
Merged

Gabriel39 merged 12 commits into
apache:masterfrom
Gabriel39:dev/remove-file-scanner-v1

Conversation

@Gabriel39

@Gabriel39 Gabriel39 commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

What problem does this PR solve?

File scans maintained both the legacy scanner/readers and the V2 reader stack. Route query scans, schema inference, file loads and external row-ID fetches through FileScannerV2, migrate standalone Push and Iceberg delete-file consumers, and remove the retired implementation and tests.

  • Keep enable_file_scanner_v2 as a deprecated no-op, with the existing optional Thrift field emitted/defaulted to true.
  • Preserve ORC session/adaptive batch caps, stripe dictionary rejection, lazy predicate-column reuse and sequential CSV/TEXT/JSON S3 prefetch.
  • Filter matching ORC dictionary IDs before string expansion in lazy, eager and predicate-only scans. Retain residual predicates, NULL semantics, pruning fences, physical column identity and physical row positions.
  • Restore ORC serialized-tail caching using the existing V1 FileMetaCache key policy, including its file-size fallback when mtime is absent. Keep V2 payload types separate. ORC footer counters have format-qualified names and publish once across repeated close; native Parquet cache identity remains unchanged.
  • Bound standalone position-delete reads to the native Parquet batch limit. Broker source projections follow validated src_slot_ids order, and malformed ORC dictionary offsets return CORRUPTION.
  • Retain eight active V2 position-delete system-table regressions and the operator shared-cache/shard-limit tests in maintained suites.
  • Preserve all six Hive nested STRUCT/ARRAY/MAP access-path scenarios in V2 across ORC/Parquet and name/position mapping, including separate predicate-column IDs and complete-versus-projected row comparisons.
  • Preserve active-file error context and the upstream Arrow column/type context without changing original validation status codes.

Related PR: #69668

The supported rolling-upgrade direction remains BE first, then FE. Old-FE/new-BE requests without semantic COUNT arguments retain their ordinary-read fallback. New-FE/old-BE nullable COUNT is outside that documented upgrade direction; the deprecated scanner option does not provide a legacy routing gate.

The branch resolves the modify/delete conflict with master at 339e087. The old Arrow reader stays deleted, and its upstream error-context fix is preserved in the V2 reader.

Release note

File scans always use FileScannerV2. enable_file_scanner_v2 is deprecated and has no effect. Unsupported operations fail in their reader.

Validation

  • 565 selected BE ASAN tests passed on the rebased code: 190 ORC, 75 Parquet, 118 Iceberg reader, 21 delete-file helper, 28 scanner, 43 CSV, 43 Arrow load, 18 load-reader, 11 JSON load, 11 position-delete system-table, 4 metadata-cache and 3 JSON-prefetch tests.
  • The Hive follow-up passed 182 related BE ASAN tests, including 24 nested-path matrix cases. Selected values, parent null maps and ARRAY/MAP offsets match full scans; KEYS-only MAP expectations retain the existing V2 value-column fallback.
  • The new 102401-row position-delete case failed before the batch-cap fix and now checks every visited position.
  • The selective dictionary case reproduced about 256 MiB of rejected-string expansion before the fix. It now bounds materialized strings and checks returned values and physical condition-cache granules with lazy materialization enabled, disabled, and no follower column.
  • Warm ORC footer opens perform zero physical pread calls. Versioned keys, the legacy versionless file-size fallback, disabled caching and repeated close are covered. Both ORC/Parquet profile initialization orders retain independent footer counters.
  • Arrow Date64 validation retains INVALID_ARGUMENT and reports the input column and types.
  • The cache/profile follow-up passed 311 related BE ASAN tests (278 reader/cache/profile and 33 scanner/operator/push tests). Two valid operator cache tests were preserved from the retired fixture.
  • FE core and all five affected connector modules pass Checkstyle. clang-format 16 passes on all 60 affected surviving C/C++ files. Build hygiene and changed-line clang-tidy pass.

External-service regression suites and full CI are not fresh results from this update. No end-to-end performance benchmark is claimed. CHAR dictionary pruning remains outside the restored STRING/VARCHAR path.

Check List (For Author)

  • Unit Test
  • External-service regression test on the updated head
  • Documentation synchronized

Check List (For Reviewer who merge this PR)

  • Confirm release note
  • Confirm tests and documentation
  • Add branch pick label

@Gabriel39
Gabriel39 requested a review from 924060929 as a code owner October 28, 2026 05:23
@hello-stephen

Copy link
Copy Markdown
Contributor

Thank you for your contribution to Apache Doris.
Don't know what should be done next? See How to process your PR.

Please clearly describe your PR:

  1. What problem was fixed (it's best to include specific error reporting information). How it was fixed.
  2. Which behaviors were modified. What was the previous behavior, what is it now, why was it modified, and what possible impacts might there be.
  3. What features were added. Why was this function added?
  4. Which code was refactored and why was this part of the code refactored?
  5. Which functions were optimized and what is the difference before and after the optimization?

@Gabriel39

Copy link
Copy Markdown
Contributor Author

run buildall

@Gabriel39

Copy link
Copy Markdown
Contributor Author

/review

@github-actions

Copy link
Copy Markdown
Contributor

Codex automated review failed and did not complete.

Error: workspace routing discovery unauthorized (401)
Workflow run: https://github.com/apache/doris/actions/runs/40637791173

Please inspect the workflow logs and rerun the review after the underlying issue is resolved.

Gabriel39 added a commit to Gabriel39/incubator-doris that referenced this pull request Oct 28, 2026
### What problem does this PR solve?

Problem Summary: The deprecated Thrift option still defaults to false even though file scans always use V2. Default the existing optional field to true, preserving its field ID, type and deprecation comment.

Related PR: apache#69723

### Release note

None

### Check List (For Author)

- Test: Generated C++ and Java with Thrift 0.24.0; verified the C++ constructor and Java constructor/clear defaults are true. git diff --check passed.
- Behavior changed: No. File scans already ignore this deprecated option and always use V2.
- Does this need documentation: No.
@Gabriel39

Copy link
Copy Markdown
Contributor Author

run buildall

@Gabriel39

Copy link
Copy Markdown
Contributor Author

/review

@github-actions

Copy link
Copy Markdown
Contributor

Codex automated review failed and did not complete.

Error: workspace routing discovery unauthorized (401)
Workflow run: https://github.com/apache/doris/actions/runs/40640200286

Please inspect the workflow logs and rerun the review after the underlying issue is resolved.

@Gabriel39

Copy link
Copy Markdown
Contributor Author

/review

@github-actions

Copy link
Copy Markdown
Contributor

Codex automated review failed and did not complete.

Error: workspace routing discovery unauthorized (401)
Workflow run: https://github.com/apache/doris/actions/runs/40642752112

Please inspect the workflow logs and rerun the review after the underlying issue is resolved.

@hello-stephen

Copy link
Copy Markdown
Contributor

BE UT Coverage Report

Increment line coverage 88.24% (90/102) 🎉

Increment coverage report
Complete coverage report

Category Coverage
Function Coverage 66.60% (31354/47078)
Line Coverage 51.33% (328237/639504)
Region Coverage 46.97% (267155/568806)
Branch Coverage 48.77% (125573/257485)

Gabriel39 added a commit to Gabriel39/incubator-doris that referenced this pull request Oct 28, 2026
### What problem does this PR solve?

Problem Summary: SHOW VARIABLES adds a deprecated_ prefix to the retained
scanner option, so suites using its old display name fail before scanning.
Read saved values through @@enable_file_scanner_v2 and use its scalar result
column, including the equivalent ES regression outside the failed group.

Restore the ORC RowID-seek fault point in the V2 reader's existing exception
boundary. Cover error conversion, empty error output and retry without
consuming the requested RowID. Keep the integration test's false-option
setting to verify that phase two still uses V2.

The already-muted workload-policy suite retains its existing V2 audit and
non-blocking checks and drops its removed V1-only branch. Existing V2 remote
scan accounting is unchanged.

Related PR: apache#69723

### Release note

None

### Check List (For Author)

- Test: Reproduced the missing variable result and the failing RowID fault
  test before the fix. All 180 ORC reader tests passed. A live-Doris regression
  probe verified variable capture and restoration. All 14 modified Groovy
  suites passed syntax and AST conversion. clang-format 16, clang-tidy,
  build hygiene and git diff --check passed.
- Full external-service suites require the CI fixtures and were not run locally.
- Behavior changed: No. Only regression compatibility and fault injection change.
- Does this need documentation: No.
@Gabriel39

Copy link
Copy Markdown
Contributor Author

run buildall

@Gabriel39

Copy link
Copy Markdown
Contributor Author

/review

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

One P2 regression should be addressed before the legacy ORC path is removed: the mandatory V2 reader ignores session/adaptive batch caps. This affects ordinary Hive ORC scans that previously used V1 when enable_file_scanner_v2=false. No additional correctness finding was established in the remaining changed paths. There were no existing inline threads or additional user focus points.

Checkpoint conclusions from static inspection:

Checkpoint Conclusion
Goal and evidence Query scans, schema inference, loads and row-ID fetches route to V2; false-option compatibility and reader-error tests were inspected. ORC parity is incomplete in the inline finding.
Scope, clarity and reuse The broad deletion is focused on retiring V1; retained metadata/JDBC/JNI/writer helpers and new standalone reader adapters reuse surviving contracts.
Concurrency and locks No new threads or lock protocol; scanner scheduling retains task attachment and exception handling, and scanner/profile ownership remains local-state scoped.
Lifecycle and initialization Push closes its reader before referenced state is destroyed and uses abort_split between ranges; row fetch closes its TableReader. No new cross-TU static initialization dependency found.
Configuration The deprecated option remains settable/readable and is intentionally ignored; ORC batch limits need the fix reported inline.
Compatibility Internal V1 APIs are removed without a new persisted data format. New FE sends true to older BEs; new BE does not depend on option presence/value.
Thrift/protobuf Complete TQueryOptions definitions compared on HEAD and branch-4.2/4.1/4.0/3.1: field 1002 retains its optional bool identity where present, with no newly introduced ID collision or required field. Other inherited cross-branch collisions predate this PR. No protobuf changes.
Parallel paths Query/load/Push/WAL/row-ID/delete-file paths and native/JNI/hybrid dispatch were traced; no additional verified issue.
Special conditions Native Hudi delta logs, query-only connector loads and unsupported Remote Doris Variant reads fail explicitly; retired table/format guards remain.
Functional and negative tests Inspected coverage includes multi-range Push conversions, absent file-column counts, ORC delete NULLs, row-ID seek errors/retry and option parity. An ORC batch-cap case is missing.
Expected results No regression output files changed; retained V2 split-boundary assertions and changed expectations were inspected. No tests were executed.
Observability V2 publishes query remote-byte deltas consumed by workload policies. The changed regression's contrary comment is stale and its cancellation assertion is weak; no production accounting regression established.
Transactions and persistence No new journal/replay representation or FE failover state; WAL retains version/header column identity checks.
Writes and atomicity Push still feeds the existing prepared-rowset transaction path; no new commit/rollback or crash-atomicity defect established.
FE-BE propagation SessionVariable and stream-load planning preserve the existing true wire option; nullable COUNT retains semantic argument identities and conservative BE fallback.
Data and error invariants Standalone delete scans isolate file identity/predicates and reject actual NULLs; external row fetch preserves absolute positions and validates counts. No visible-version or MoW delete-bitmap change.
Memory and performance One P2 finding: fixed 4096-row ORC reads can exceed former bounded memory use. Byte sizes are static derivations, not benchmark results.
Other checks No retained compiled dependency on the removed sources was found. The Paimon deletion-vector extraction preserves the prior implementation.

Coverage and limits: all 226 changed paths were covered by the main review and complementary reviewer scopes, including deletion inventories, retained callers and replacement mechanisms. This was static only: no builds, tests, benchmarks, source edits or submodule initialization. External-writer/JNI interoperability, every removed decoder/corrupt-input combination, and malformed historical Push rejection-accounting equivalence were not established through execution. No supported producer was found for the speculative malformed-Push issue, so it is not an inline finding. No additional user focus was supplied. The tiny-stripe coalescing candidate was withdrawn: the old reader already bypassed its installed cache, so its removal does not prove a new request regression.

Review converged after 3 rounds of static review. The validation limits above remain.

Review converged after 3 round(s).

@hello-stephen

Copy link
Copy Markdown
Contributor

Cloud UT Coverage Report

Increment line coverage 🎉

Increment coverage report
Complete coverage report

Category Coverage
Function Coverage 76.65% (2101/2741)
Line Coverage 65.99% (38778/58759)
Region Coverage 53.70% (36370/67729)
Branch Coverage 57.21% (11849/20710)

Gabriel39 added a commit to Gabriel39/incubator-doris that referenced this pull request Oct 28, 2026
### What problem does this PR solve?

Related PR: apache#69723

Problem Summary: ORC V2 inherited the no-op batch-size setter and decoded
4096 rows at a time, so forcing V2 dropped session and adaptive memory
limits for scans previously using V1. Apply the cap to SDK batches before
decoding, keep it across stripe ranges, and preserve the reader cursor
when the cap changes. RowID reads remain limited to one row.

Require the remote-scan workload policy to cancel V2 queries using the
existing query IO counters. Remove the obsolete audit-only branch and
comments claiming that V2 does not publish remote scan bytes.

### Release note

ORC file scans honor session and adaptive row batch limits to reduce
per-batch decoding memory for wide rows.

### Check List (For Author)

- Test: Three batch-cap tests reproduced the original issue. All 204 ORC
  and FileScannerV2 ASAN unit tests passed, followed by a successful
  strengthened wide-payload stripe test. A live remote-byte policy
  cancellation probe passed with both values of the compatibility option.
  Groovy syntax/AST conversion, clang-format 16, changed-line clang-tidy,
  build hygiene and git diff --check passed.
- The full Hive external suite requires CI fixtures and was not run locally.
- Behavior changed: Yes. ORC decoding respects supplied row batch limits,
  and regression coverage requires workload-policy cancellation.
- Does this need documentation: No.
@Gabriel39

Copy link
Copy Markdown
Contributor Author

run buildall

@Gabriel39

Copy link
Copy Markdown
Contributor Author

/review

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Static review of head c2669ea identified three P2 performance regressions when former V1 queries are forced onto V2: lost ORC dictionary stripe pruning, duplicate ORC lazy predicate materialization, and lost sequential S3 prefetch for CSV/TEXT/JSON. Each has a separate inline finding with a concrete reachable path. No new P0/P1 finding was verified.

Existing thread 4398580838 (ORC physical batch limits) is fixed at this head: the physical batch honors session/adaptive caps, stripe transitions retain the cap, and RowID fetch remains one row. It is not reposted. There are no existing P0/P1 comments to retain as blockers. No additional user focus was supplied.

The reviewed execution chain is FE scan/load plan -> FileScanLocalState/shared SplitSource -> FileScannerV2 -> TableReader/LoadTableReader -> TableColumnMapper -> physical FileReader -> final table/destination block. External RowID materialization returns through FileScannerV2::read_by_rows; historical Push and standalone Iceberg position-delete rewriting enter the table-reader layer directly.

Applicable critical checkpoint conclusions:

Checkpoint Conclusion
Goal and proof Query, load and RowID entry points consistently select V2, but the three inline performance regressions leave the migration incomplete in those workloads. New/retained tests were inspected as source evidence only.
Scope and reuse The large deletion is focused on retiring V1; retained JDBC/JNI, metadata, writers and deletion-vector helpers still have callers. Load and delete adapters reuse the intended table/mapper layers.
Concurrency and locks Scanner workers share the existing locked split source; per-reader batch/split state is sequential. No new lock order or heavy FE metadata-lock operation was found. Removing S3 background prefetch is covered inline.
Lifecycle and static initialization Checked Push init/abort/close/destructor ordering, IOContext/stats lifetimes, JNI partial-open cleanup, EOF/error transitions and ORC RowReader position retention. No new cross-translation-unit static initialization dependency was found.
Configuration The existing scanner variable remains settable, stored and forwarded under its stable name, with deprecated display. No new mutable configuration is introduced; actual Rust/JNI capability gates remain.
Compatibility and formats The existing wire option is emitted true for older backends. Old-FE COUNT requests with unknown arguments use the conservative fallback. WAL headers/payload and writer formats are unchanged. Historical Push was checked against the retained request protocol.
Thrift/protobuf IDs Compared touched TQueryOptions on HEAD and release branches 3.1, 4.0, 4.1, 4.2. Field 1002 remains the same optional boolean wherever present; no introduced ID reuse, enum collision or required field.
Parallel paths Checked normal/RowID/load scans, eager/streaming/reuse connector planning, native/JNI/Rust hybrids, ORC lazy/sequential/stripe transitions, Parquet encodings/page/cache paths and standalone/query delete readers.
Conditional checks Concrete readers reject unsupported modes; native Hudi does not silently ignore delta logs, query-only connectors reject loads, and Remote Doris rejects unsupported Variant carriers. Suspicious bitmap conversion, inherited delete metadata and mixed-Hive/Hudi producer concerns were traced and dismissed with evidence.
Test coverage Inspected new cap/seek, Push multi-range/omitted-count, delete NULL and nested Variant cases; retained decoder/schema/delete/null/EOF suites; and all changed regression expectations. Deleted-test behaviors were compared with maintained mechanisms and representative assertions. The three lost performance mechanisms need focused work-count/prefetch regression coverage.
Test results No changed .out result artifacts. No build, test, benchmark, runtime probe, formatting mutation or submodule initialization was performed in this review.
Observability Existing V2 IO/resource/profile accounting and workload-policy counters remain connected; the strengthened policy case requires cancellation. ORC dictionary optimization metrics disappear with the lost path described inline.
Persistence and failover No new edit-log or persisted transaction state is introduced. Stable session-variable replay is preserved. No change to internal visible-version or MoW bitmap protocols was found.
Writes and atomicity Push conversion remains before the existing rowset flush/build/commit sequence and propagates failure statuses. Bitmap binary casts retain raw deserialization, and split abort preserves destination expressions. No additional verified write/atomicity issue was found.
FE/BE variable passing Ordinary query and dedicated stream-load options retain field 1002=true. Nullable COUNT ExprIds are preserved through SlotIds and global mapper positions; no new transmitted variable is added.
CPU, memory and complexity Physical ORC caps are fixed; native decoder buffers, lazy row alignment and delete-key ownership were reviewed. The inline findings identify provable extra O(rows) decode/copy work and removed asynchronous I/O, without claiming a measured slowdown.
Other issues Deleted-header reference audit found no surviving literal production include. A hygiene self-test still names a deleted positive probe, weakening that maintenance check; it does not establish a configure or production failure. No other actionable defect was verified.

Review limits: static source review only. Runtime fixture parity, mixed-version deployments, external Hive/Hudi/Paimon/ES/S3/JVM integration and performance measurements were not executed or inferred. Current Spark production planning is retired, so historical Push variants were evaluated against retained protocol consumers and fixtures rather than a live old-FE producer. These validation limits are separate from the explicit review convergence result appended by the submission helper.

Review converged after 3 round(s).

Comment thread be/src/format/orc/vorc_reader.cpp Resolved
Comment thread be/src/format/csv/csv_reader.cpp Resolved
@hello-stephen

Copy link
Copy Markdown
Contributor

Cloud UT Coverage Report

Increment line coverage 🎉

Increment coverage report
Complete coverage report

Category Coverage
Function Coverage 76.65% (2101/2741)
Line Coverage 66.05% (38813/58759)
Region Coverage 53.74% (36397/67729)
Branch Coverage 57.27% (11861/20710)

@hello-stephen

Copy link
Copy Markdown
Contributor
TPC-H: Total hot run time: 29341 ms
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpch-tools
Tpch sf100 test result on commit c2669ea4efafeb838b1e7a97bedfb50abbea50ab, data reload: false

------ Round 1 ----------------------------------
============================================
q1	17592	5396	5318	5318
q2	2154	348	243	243
q3	10218	1573	842	842
q4	4688	493	339	339
q5	7463	869	546	546
q6	205	209	159	159
q7	815	822	608	608
q8	9258	1545	1228	1228
q9	5733	4416	4423	4416
q10	6924	1305	1039	1039
q11	451	255	228	228
q12	640	449	281	281
q13	18074	3629	3041	3041
q14	280	275	243	243
q15	q16	757	717	656	656
q17	1361	779	583	583
q18	7111	6224	6150	6150
q19	1118	1116	645	645
q20	397	353	232	232
q21	5671	2283	2384	2283
q22	374	311	261	261
Total cold run time: 101284 ms
Total hot run time: 29341 ms

----- Round 2, with runtime_filter_mode=off -----
============================================
q1	5971	5928	5911	5911
q2	744	554	570	554
q3	5203	5424	4942	4942
q4	2118	2264	1608	1608
q5	5271	5643	5817	5643
q6	305	221	155	155
q7	2383	2086	2049	2049
q8	3258	2810	2785	2785
q9	8210	8015	7955	7955
q10	3885	3705	3287	3287
q11	638	472	537	472
q12	777	748	534	534
q13	3737	3852	3334	3334
q14	312	320	291	291
q15	q16	720	723	660	660
q17	8549	7438	7032	7032
q18	12971	12219	12967	12219
q19	1071	862	880	862
q20	2150	2174	1933	1933
q21	6017	4882	5044	4882
q22	532	471	408	408
Total cold run time: 74822 ms
Total hot run time: 67516 ms

@hello-stephen

Copy link
Copy Markdown
Contributor
ClickBench: Total hot run time: 24.37 s
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/clickbench-tools
ClickBench test result on commit ec07842fc0642e9d1f23cde53336d3435a5f628b, data reload: false

query1	0.01	0.00	0.00
query2	0.09	0.05	0.05
query3	0.26	0.08	0.09
query4	1.60	0.10	0.11
query5	0.23	0.24	0.24
query6	1.19	0.67	0.66
query7	0.05	0.01	0.01
query8	0.06	0.04	0.04
query9	0.41	0.36	0.34
query10	0.57	0.57	0.61
query11	0.20	0.15	0.15
query12	0.19	0.15	0.15
query13	0.50	0.49	0.48
query14	1.03	1.02	1.00
query15	0.63	0.61	0.62
query16	0.36	0.30	0.34
query17	1.10	1.13	1.11
query18	0.21	0.19	0.19
query19	2.04	2.04	2.00
query20	0.02	0.02	0.01
query21	15.48	0.20	0.14
query22	4.88	0.06	0.05
query23	16.29	0.33	0.12
query24	3.07	0.49	0.61
query25	0.13	0.04	0.05
query26	1.61	0.22	0.15
query27	0.05	0.05	0.05
query28	3.79	0.73	0.34
query29	12.53	4.15	3.24
query30	0.27	0.14	0.10
query31	2.78	0.56	0.32
query32	3.22	0.60	0.50
query33	3.20	3.19	3.25
query34	15.35	4.01	3.38
query35	3.32	3.34	3.33
query36	0.72	0.56	0.47
query37	0.11	0.08	0.07
query38	0.05	0.04	0.04
query39	0.04	0.03	0.03
query40	0.17	0.15	0.14
query41	0.08	0.03	0.03
query42	0.04	0.02	0.03
query43	0.04	0.04	0.04
Total cold run time: 97.97 s
Total hot run time: 24.37 s

### What problem does this PR solve?

Related PR: apache#69723

Problem Summary: ORC and Parquet registered the same flat counter names under
different format parents, so the first reader acquired both formats' footer
statistics. Qualify the ORC names and verify both registration orders. Restore
the pre-existing ORC FileMetaCache key policy and the original native Parquet
key builder, removing the newly introduced shared stable-key API. Preserve two
still-valid operator cache ownership and shard-limit tests in a maintained suite.

### Release note

Keep ORC and Parquet footer statistics attributable to their respective formats.

### Check List (For Author)

- Test: 311 related BE ASAN unit tests; restored operator cases retested after
  cleanup; clang-format 16; build hygiene; changed-line clang-tidy
- Behavior changed: ORC footer counters use format-qualified names; ORC cache
  identity retains the legacy policy and native Parquet identity is unchanged
- Does this need documentation: Yes (review guide updated in this commit)
@Gabriel39

Copy link
Copy Markdown
Contributor Author

run buildall

@Gabriel39

Copy link
Copy Markdown
Contributor Author

/review

@github-actions github-actions Bot left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reviewed PR #69723 at exact head ea9057f against base 339e087. I read the complete authoritative diff and changed-file list, all required AGENTS.md files, the code-review guidance, existing review threads, and the shared ledger. The review converged after two bounded rounds: the scanner/load/control-plane, native Parquet/ORC migration, and table-format/lifecycle risk scopes all returned NO_NEW_VALUABLE_FINDINGS. No new verified defect remains, so there are no inline comments. The initial risks R1-R8 were each dismissed with evidence. The provisional ORC virtual-column candidate was disproved by the request-builder projection invariant and virtual-column decode branches; the Paimon row-ID/cache candidate was unreachable because Paimon does not advertise the required lazy-materialization capability. Existing P1 IDs 4414682259, 4420555519, 4422958987, and 4430592029 were revalidated against this head and are fixed, pre-existing, or explicitly dismissed in the supplied thread context, so none remains blocking and no duplicate is reposted. The user supplied no additional review focus beyond the complete review. Scanner factory and split lifecycle, load source-slot/default/path handling, row-ID fetch eligibility, Parquet/ORC decoding and dictionary/lazy paths, table-format deletes and caches, text/JSON/Arrow/JNI cleanup, FE planner/protocol compatibility including optional Thrift field 1002, build/header/profile changes, and retained test coverage were all inspected without a new reachable correctness, lifecycle, compatibility, performance, or observability issue. This was a static-only review: builds, tests, benchmarks, and runtime validation were not executed.

Review converged after 2 round(s).

@hello-stephen

Copy link
Copy Markdown
Contributor

Cloud UT Coverage Report

Increment line coverage 🎉

Increment coverage report
Complete coverage report

Category Coverage
Function Coverage 76.65% (2101/2741)
Line Coverage 65.98% (38777/58772)
Region Coverage 53.81% (36453/67743)
Branch Coverage 57.24% (11861/20722)

@hello-stephen

Copy link
Copy Markdown
Contributor
TPC-H: Total hot run time: 28314 ms
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpch-tools
Tpch sf100 test result on commit ea9057f8ad0b154ef1a2e977e8d874ac042905a5, data reload: false

------ Round 1 ----------------------------------
============================================
q1	16705	5088	5020	5020
q2	2119	311	263	263
q3	9898	1361	816	816
q4	4622	460	333	333
q5	7440	790	527	527
q6	180	180	150	150
q7	764	802	588	588
q8	9384	1336	1077	1077
q9	5431	4201	4156	4156
q10	6822	1287	1007	1007
q11	431	239	217	217
q12	644	405	286	286
q13	18077	3618	3009	3009
q14	276	279	249	249
q15	q16	705	718	666	666
q17	1175	743	569	569
q18	7007	6152	6121	6121
q19	1109	943	602	602
q20	376	333	225	225
q21	5095	2190	2413	2190
q22	351	303	243	243
Total cold run time: 98611 ms
Total hot run time: 28314 ms

----- Round 2, with runtime_filter_mode=off -----
============================================
q1	5578	5621	5581	5581
q2	676	566	524	524
q3	5213	5357	4930	4930
q4	1961	2120	1552	1552
q5	5262	5146	5289	5146
q6	282	211	158	158
q7	2438	2042	1872	1872
q8	3005	2734	2736	2734
q9	8063	7938	7902	7902
q10	3753	3685	3421	3421
q11	620	433	418	418
q12	676	696	549	549
q13	3568	3804	3363	3363
q14	317	310	285	285
q15	q16	705	701	653	653
q17	8183	7014	6874	6874
q18	13012	12229	12776	12229
q19	958	775	764	764
q20	2145	2140	1947	1947
q21	5586	4649	4683	4649
q22	506	449	408	408
Total cold run time: 72507 ms
Total hot run time: 65959 ms

@hello-stephen

Copy link
Copy Markdown
Contributor
TPC-DS: Total hot run time: 150875 ms
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/tpcds-tools
TPC-DS sf100 test result on commit ea9057f8ad0b154ef1a2e977e8d874ac042905a5, data reload: false

query5	4324	579	434	434
query6	423	206	190	190
query7	4805	448	234	234
query8	324	170	149	149
query9	8763	3178	3186	3178
query10	472	303	255	255
query11	5977	3850	3575	3575
query12	149	92	88	88
query13	1249	449	320	320
query14	6124	3956	3640	3640
query14_1	3473	3420	3411	3411
query15	206	195	182	182
query16	948	446	396	396
query17	852	662	529	529
query18	2423	419	305	305
query19	194	165	137	137
query20	85	82	81	81
query21	205	128	108	108
query22	12876	12940	12846	12846
query23	13451	12675	12450	12450
query23_1	12371	12533	12608	12533
query24	7222	812	475	475
query24_1	480	488	495	488
query25	533	432	368	368
query26	1269	254	144	144
query27	2797	436	270	270
query28	4529	2005	2017	2005
query29	1553	609	463	463
query30	307	217	189	189
query31	1013	934	818	818
query32	140	94	94	94
query33	532	295	246	246
query34	886	848	508	508
query35	733	751	653	653
query36	814	819	713	713
query37	127	95	86	86
query38	1773	1714	1674	1674
query39	793	787	746	746
query39_1	734	728	747	728
query40	225	120	100	100
query41	90	69	66	66
query42	88	86	81	81
query43	347	366	307	307
query44	1317	707	691	691
query45	178	178	165	165
query46	835	950	563	563
query47	3044	2843	2648	2648
query48	298	317	221	221
query49	572	400	286	286
query50	668	272	207	207
query51	10391	10568	10668	10568
query52	76	79	67	67
query53	190	207	148	148
query54	239	191	193	191
query55	74	63	61	61
query56	221	206	198	198
query57	1664	1607	1556	1556
query58	281	261	253	253
query59	2170	2177	1986	1986
query60	295	236	225	225
query61	147	147	141	141
query62	383	336	279	279
query63	183	152	156	152
query64	2686	926	767	767
query65	3302	3252	3238	3238
query66	1754	418	303	303
query67	20347	20153	20091	20091
query68	3103	980	598	598
query69	388	288	227	227
query70	843	742	728	728
query71	287	238	211	211
query72	2742	2570	2254	2254
query73	510	521	296	296
query74	4654	4538	4334	4334
query75	2315	2290	1897	1897
query76	2174	1019	613	613
query77	335	381	268	268
query78	9713	9719	9150	9150
query79	964	886	512	512
query80	552	410	328	328
query81	538	324	263	263
query82	555	134	101	101
query83	312	201	182	182
query84	324	118	98	98
query85	814	412	365	365
query86	316	237	218	218
query87	2004	1984	1784	1784
query88	3747	2759	2745	2745
query89	332	284	240	240
query90	1916	163	162	162
query91	169	150	124	124
query92	100	89	87	87
query93	998	994	565	565
query94	445	317	254	254
query95	573	313	359	313
query96	672	479	235	235
query97	2411	2424	2329	2329
query98	158	159	156	156
query99	773	763	662	662
Total cold run time: 233361 ms
Total hot run time: 150875 ms

@hello-stephen

Copy link
Copy Markdown
Contributor
ClickBench: Total hot run time: 25.16 s
machine: 'aliyun_ecs.c7a.8xlarge_32C64G'
scripts: https://github.com/apache/doris/tree/master/tools/clickbench-tools
ClickBench test result on commit ea9057f8ad0b154ef1a2e977e8d874ac042905a5, data reload: false

query1	0.00	0.00	0.00
query2	0.14	0.09	0.08
query3	0.35	0.12	0.14
query4	1.61	0.14	0.14
query5	0.27	0.26	0.25
query6	1.16	0.64	0.64
query7	0.04	0.00	0.00
query8	0.09	0.07	0.06
query9	0.45	0.36	0.37
query10	0.58	0.56	0.59
query11	0.32	0.18	0.18
query12	0.32	0.19	0.18
query13	0.50	0.52	0.50
query14	0.93	0.93	0.93
query15	0.67	0.58	0.60
query16	0.35	0.36	0.36
query17	1.04	1.00	1.04
query18	0.29	0.27	0.29
query19	1.97	1.85	1.84
query20	0.02	0.01	0.02
query21	15.40	0.34	0.30
query22	4.95	0.13	0.12
query23	15.86	0.50	0.30
query24	2.34	0.53	0.37
query25	0.16	0.11	0.10
query26	0.76	0.26	0.21
query27	0.10	0.10	0.09
query28	3.52	0.88	0.40
query29	12.58	4.36	3.41
query30	0.36	0.17	0.14
query31	2.76	0.58	0.32
query32	3.23	0.61	0.48
query33	2.95	3.03	3.04
query34	15.88	4.13	3.47
query35	3.37	3.36	3.36
query36	0.59	0.53	0.48
query37	0.12	0.10	0.09
query38	0.08	0.06	0.06
query39	0.06	0.06	0.05
query40	0.21	0.16	0.16
query41	0.10	0.06	0.05
query42	0.06	0.05	0.05
query43	0.07	0.06	0.06
Total cold run time: 96.61 s
Total hot run time: 25.16 s

@hello-stephen

Copy link
Copy Markdown
Contributor

BE UT Coverage Report

Increment line coverage 88.46% (92/104) 🎉

Increment coverage report
Complete coverage report

Category Coverage
Function Coverage 67.12% (31958/47616)
Line Coverage 51.93% (335821/646680)
Region Coverage 47.50% (273160/575108)
Branch Coverage 49.31% (128693/260971)

@hello-stephen

Copy link
Copy Markdown
Contributor

BE Regression && UT Coverage Report

Increment line coverage 88.35% (91/103) 🎉

Increment coverage report
Complete coverage report

Category Coverage
Function Coverage 77.63% (35810/46130)
Line Coverage 62.91% (402384/639615)
Region Coverage 59.54% (340820/572426)
Branch Coverage 60.64% (157674/260026)

@924060929 924060929 left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This PR retires the legacy file scanner and makes FileScannerV2 unconditional. FE changes accordingly deprecate the routing option and remove obsolete scanner gates from connector planning and nullable COUNT pushdown.

Static FE review is complete for ea9057f against 339e087. No actionable P0/P1/P2 issue was established within FE and the necessary FE/BE contract boundaries. COUNT argument propagation, old-FE fallback, connector capability guards, query/load options, and optional Thrift field 1002 were checked. The existing nullable COUNT compatibility comment assumes new FE with old BE, reversing the supported BE-first upgrade order.

Recommendation: APPROVE within this review scope. This is not a full BE implementation approval. No builds or tests were run. The supplied current-head records show P0, FE UT, and compilation success, but FE coverage failure; required-check policy is unknown, so merge readiness is not asserted.

Automated FE-focused static review (GPT-6.1 Sol, medium). No build or tests were run.

@morningman

Copy link
Copy Markdown
Contributor

@Gabriel39 a heads-up on overlap with two open PRs, so we can agree on the merge order.

@Gabriel39
Gabriel39 merged commit 49da878 into apache:master Nov 2, 2026
47 of 48 checks passed
xylaaaaa added a commit to xylaaaaa/doris that referenced this pull request Nov 3, 2026
…torage-client

master removed the legacy file scanner (apache#69723): be/src/exec/scan/file_scanner.{h,cpp} and the
V1 readers be/src/format/{csv/csv_reader,json/new_json_reader,orc/vorc_reader,
parquet/vparquet_reader,text/text_reader}.cpp. They are deleted here as in master.

- The V1 rejection of Azure SAS / OAuth2 (FileScanner::_check_storage_access and its test in
  file_scanner_v2_test.cpp) goes with the V1 scanner: every scan now runs on FileScannerV2,
  which serves typed Azure access.
- The V1 readers' copy of TFileScanRangeParams.azure_client_config has its counterpart on the V2
  paths: TableReader, fetch_file_schema (now used for every schema RPC format), the Iceberg
  reader's delete files, the deletion vector reader and parquet_meta all set it; the Iceberg
  delete file helper now reads through TableReader with the delete sink's scan params.

obj_storage_file_reader.cpp takes master's read IO trace (`s3_get` records with io_ctx) and
atomic statistics under the ObjStorageFileReader names.

Tests from master moved to the typed client API: s3_file_reader_test.cpp (ObjClientConfig,
set_client_for_test, ObjStorageFileReader, AwsSdkManager), json_reader_test.cpp and
csv_reader_test.cpp (ObjClientFactory::set_client_creator_for_test with ObjClientConfig).
Gabriel39 added a commit that referenced this pull request Nov 4, 2026
### What problem does this PR solve?

Issue Number: N/A

Related PR: #69672, #69723

External ES regression suites fail during setup or retain expectations
from the previous indexed-value predicate behavior:

- Read the deprecated scanner option through `SELECT
@@enable_file_scanner_v2`, whose compatibility name remains valid when
SHOW prefixes deprecated names.
- On ES 8.3.3, fall back from the specifically unsupported
`_source.mode` mapping syntax to `enabled:false`, retaining the
disabled-source row and alias checks. Other HTTP failures remain fatal;
supporting ES8 versions still exercise the mode syntax.
- Compare LIKE, REGEXP, NULL and `level` predicates against unfiltered
rows materialized into internal tables, then update only the affected
runner-generated golden blocks.
- Make the date-string equality test explicitly use source reads, test
midnight as the matching value, and retain 08:00 as a negative control.
- Check that a residual predicate prevents early termination while a
safe numeric docvalue predicate still permits LIMIT pushdown.

### Release note

None

### Check List (For Author)

- Test
  - [x] Regression test
  - [ ] Unit Test
  - [x] Manual test

Validation uses fresh FE and ASAN BE artifacts built from this PR commit
(`6386f5f83a51b548c86ab3cff83d53cb48fb18df`) with the standard
`./build.sh --fe --be -j64` entry point. The complete build exited
successfully, and both finished artifact versions match that commit. No
production source is changed by this PR.

- On the same fresh binaries and reset ES 5.6.16, 6.8.23, 7.17.5 and
8.3.3 fixtures, the parent suites/golden files reproduced all ten
failures; the final PR suites passed all ten in normal comparison, with
zero failed, fatal or skipped suites. The passing run executed 2,154
golden checks and 1,798 independent query comparisons, without
force-generation or further golden edits.
- The complete `test_es_query` passed all four ES versions and both
scroll settings, including its unchanged complex-array golden blocks
(276 result checks and 20 independent comparisons).
- Independent SQLite/Python evaluation of the unchanged raw fixture JSON
passed 38 comparisons and covered all 17 changed golden blocks. Real ES
requests also confirmed that `ignore_above` omits matching index values
while retaining source strings, and that the constant-keyword index
matches documents whose source `level` is absent.
- Captured ES search requests verified the date modes: the original
four-column projection reads source, returns midnight values and rejects
the 08:00 predicate. A date-only docvalue projection correctly returns
08:00 in the +08:00 session. The revised test explicitly fixes the
source contract and retains the 08:00 negative control.
- The complete fallback suite additionally passed normal comparison on
ES 8.11.3: 304 golden checks and 304 independent comparisons. The actual
mappings and document responses confirmed absent source for both the ES
8.3.3 `enabled:false` fallback and the supported ES 8.11.3
`mode:disabled` branch.
- The existing `test_es_file_scanner_v2` compatibility suite also passed
on the fresh build across all four fixture versions. The deprecated
option is accepted; this does not claim two distinct physical reader
implementations on current master.
- Direct variable probes confirmed the old exact SHOW name returns no
rows, the wildcard SHOW name exposes its deprecated prefix, and the
scalar compatibility name remains readable.
- `git diff --check` passed. ES9 compatibility is not claimed.

- Behavior changed:
- [x] No production behavior change; regression fixtures and
expectations are corrected.
  - [ ] Yes.

- Does this need documentation?
  - [x] No.
  - [ ] Yes.

### Check List (For Reviewer who merge this PR)

- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label
liaoxin01 added a commit to liaoxin01/doris that referenced this pull request Nov 5, 2026
### What problem does this PR solve?

Related PR: apache#70160, apache#69723

Problem Summary: Master always uses FileScannerV2 and exposes the deprecated
rollout option under a prefixed SHOW name. The WAL schema-change case queries
the old name, gets no rows and fails before testing replay. Remove the obsolete
global-option setup and restoration.

### Release note

None

### Check List (For Author)

- Test: Regression test test_group_commit_schema_change_wal on the local integrated ASAN cluster (RUNNING, FINISHED and CANCELLED); existing expected output unchanged.
- Behavior changed: No
- Does this need documentation: No
xylaaaaa added a commit to xylaaaaa/doris that referenced this pull request Nov 5, 2026
### What problem does this PR solve?

Issue Number: close apache#69884

Related PR: apache#69904

Problem Summary: apache#69723 removed the legacy file scanner. Every file scan
now uses FileScannerV2, and enable_file_scanner_v2 is kept only as a
deprecated session variable that nothing reads. Four Delta suites still
set it: basic, features and the benchmark to true, and the Hive gateway
suite to false. None of these settings has any effect, so the Hive
suite's false setting does not check anything. Remove all four. SET
statements produce no query output, so the expected .out files are
unchanged. The Hive suite still sets file_split_size=128 so that one
file becomes many ranges sharing the same deletion vector.

### Release note

None

### Check List (For Author)

- Test: No local run. The diff deletes only SET statements; regression runs are left to CI.
- Behavior changed: No
- Does this need documentation: No
xylaaaaa added a commit to xylaaaaa/doris that referenced this pull request Nov 5, 2026
### What problem does this PR solve?

Issue Number: close apache#69884

Related PR: apache#69904

Problem Summary: apache#69723 removed the legacy file scanner. Every file scan
now uses FileScannerV2, and enable_file_scanner_v2 is kept only as a
deprecated session variable that nothing reads. Four Delta suites still
set it: basic, features and the benchmark to true, and the Hive gateway
suite to false. None of these settings has any effect, so the Hive
suite's false setting does not check anything. Remove all four. SET
statements produce no query output, so the expected .out files are
unchanged. The Hive suite still sets file_split_size=128 so that one
file becomes many ranges sharing the same deletion vector.

### Release note

None

### Check List (For Author)

- Test: No local run. The diff deletes only SET statements; regression runs are left to CI.
- Behavior changed: No
- Does this need documentation: No
yiguolei pushed a commit that referenced this pull request Nov 6, 2026
### What problem does this PR solve?

Issue Number: None

Related PR: #65730 #66608 #66805 #66841 #66999 #67005 #67011 #67110
#67268 #67289 #67325 #67650 #67921 #68028 #68335 #68375 #68615 #68690
#68813 #68815 #68707 #68649

Problem Summary:

**In short.** This ports the Lance BE read path of branch-4.1 /
branch-4.2 to master, in its branch-4.2 form as of `cabcf45c1af`, plus
the BE side of three 4.x changes merged after that snapshot: the search
filter `prefilter` flag and profile layout (#68813), distance bounds for
`vector_search` (#68815), and the split-level index rule of #68707 that
#68815 relies on. It gathers the BE parts of the PRs listed above.
Master links `liblance_c` but has no Lance reader, so every
`FORMAT_LANCE` scan fails today. With this PR the BE reads Lance
datasets through FileScannerV2: plain scans, vector search and full-text
search, predicate and runtime-filter pushdown, the schema fetch of a
local Lance TVF, and the second phase of TopN lazy materialization. The
PR is BE only. The FE connector plugin and the regression suites come in
separate PRs, so until then nothing on master plans a Lance scan, and
the BE unit tests carry the proof.

Original authors: @zhangstar333 @Gabriel39 @wangzhaobo957-cloud @Jay-ju
@FANNG1

**Background.** Lance shipped in 4.1.4: the Lance catalog, `local()` /
`s3()` with `format=lance`, and the `vector_search` / `full_text_search`
TVFs. On master the FE side is being rebuilt as a connector plugin
(`fe-connector-lance`), and the BE needs the reader from 4.x. The thrift
ids are the branch-4.1 / 4.2 ones, including `TExternalSearchRequest.5`
(`prefilter`) and `TVectorSearchParams.6/7` (distance bounds) added
after the snapshot, so a BE built from master also reads the Lance scan
ranges and search requests that a 4.x FE sends during a rolling upgrade.
The one exception is `TVectorSearchOptions` 6, branch-4.2's
`approx_mode` (#69852, merged into branch-4.2 after the snapshot and not
ported): it is reserved in a comment, and a master BE ignores it, so
during a rolling upgrade a 4.2 FE that asks for `fast` or `accurate`
gets Lance's default (`normal`).

**The problem, and what it cost.**

| | master today | with this PR |
|---|---|---|
| A `FORMAT_LANCE` scan | FileScannerV2 has no Lance reader, so the scan
fails with "FileScannerV2 does not support file format" | read by
`LanceTableReader` in FileScannerV2 |
| `fetch_table_schema` for a local Lance TVF | not handled | returns the
Arrow schema mapped to Doris types; Null leaves keep `NULL_TYPE` |
| The row location of a Lance row in TopN lazy materialization |
`GlobalRowLoacationV2` only has a 32-bit row id, but a Lance row id is a
64-bit id within the snapshot | a version 1 layout carries the 64-bit
row id; phase two reads the rows with the dataset's `take_rows` |

**How this PR fixes it.**

- Reader: `be/src/format_v2/lance/*` and
`be/src/format_v2/table/lance_reader.*`, as on branch-4.2 except one
local renamed for master's `-Wshadow`. The unit tests construct
`RuntimeState(TQueryOptions(), globals)`, because master removed the
globals-only constructor.
- FileScannerV2: the reader dispatch and `_to_file_format` handle
`FORMAT_LANCE` / `"lance"`; `FileFormat::LANCE` is added to its two
exhaustive switches. Since #69723 every file scan uses FileScannerV2, so
nothing else routes Lance scans (on branch-4.2 a Lance query fails with
NotSupported when `enable_file_scanner_v2` is off).
- Row locations: `ROW_VERSION {FILE_LOCAL_ROW_ID = 0,
LANCE_DATASET_ROW_ID = 1}` and a union layout for
`GlobalRowLoacationV2`. Version 0 keeps its byte layout, which static
asserts pin. `PRequestBlockDesc.row_id` becomes `uint64` (a proto2
varint, so the wire format is compatible). The materialization operator
decodes row locations by version, rejects an unknown version or a wrong
size, and stamps `row_location_version`. `RowIdStorageReader` validates
the counts and the version, range-checks internal row ids against
`uint32`, and groups Lance rows by plan node, snapshot version and
dataset URI, so each dataset is read with one `take_rows`. Master's
segment batching, scan-slot dedup and fetch concurrency stay as they
are.
- Thrift (`PlanNodes.thrift`): the external search request body
(`TVectorElementType`, `TVectorMetric`, `TSearchVector`,
`TVectorSearchParams` 1-7, `TFtsCoverageMode`, `TFtsQueryType`,
`TFtsMatchOperator`, `TFullTextSearchParams`, `TSearchFilterFormat`,
`TSearchFilter`, `TVectorSearchOptions`, `TExternalSearchQuery`),
`TExternalSearchRequest` 2-5 and `TLanceFileDesc` 4-6, all with the
branch-4.1 / 4.2 ids. The enums spell out their 4.x values. No id is
reused. On master, #70075 reserved `TExternalSearchRequest` 2-5 in
comments and added 6 (`paimon_options`); this PR replaces those comments
with the fields and leaves 6 as it is. `TVectorSearchOptions` 6 is
reserved in a comment for branch-4.2's `approx_mode` (#69852).
- After the snapshot (one commit each):
- #68813: `prefilter` decides whether the TVF search filter runs before
(true) or after (unset / false) candidate selection, for vector and
full-text search. The profile groups the Lance counters under
`LanceTiming`, `LanceIO`, `LanceIndex`, `LanceVectorIndex`,
`LanceScalarIndex`, `LanceExecutionPlan`, `LanceScan`, `LanceSearchPlan`
and `LanceDataCache`, uses the 4.x counter names, and registers every
scan metric before the statistics callback is installed, so the
callback, which Lance invokes from its execution streams, never adds
counters. #68813's FE tweak that hides `TUnit.NONE` labels in
`AggCounter` belongs to the FE port.
- #68707 (part): a vector split searches exactly the index segments FE
planned for it, and a split without segments is searched flat whatever
`use_index` says; a split with segments under `use_index = false` is
rejected. 4.x FEs plan segments only when `use_index` allows them.
- #68815: `distance_lower_bound` (inclusive) and `distance_upper_bound`
(exclusive) are validated (finite FLOAT values, lower < upper at FLOAT
precision, single-vector only) and installed with
`lance_scanner_set_distance_range()`, which master's pinned lance-c
provides.
- Review fixes:
- A nested array goes to the Doris SerDes without a copy only when it
and every descendant start at offset 0 and reference exactly their child
ranges. Lance's legacy (v1) decoder can return a struct or list that is
tight at its own level over a Boolean child with a bit offset (a batch
that starts mid-byte in the bitmap), and the SerDes reject that under
Arrow input validation.
- A zero-field `STRUCT<>`, which Lance decodes, is reported as an
unsupported column at any nesting level, like the other unsupported
Lance types. It used to be typed as a STRUCT that `ColumnStruct` cannot
hold (its row count comes from its first child).
- A Lance Decimal256 field with precision <= 38 is narrowed to
Decimal128 by a safe Arrow cast before decoding. Doris types it
DECIMAL32/64/128I by precision, and those SerDes read only Decimal128
arrays; the BE used to crash on the first value, also for nested leaves.
Only values a reader can see are cast: before a nested leaf is narrowed,
every child slot that no visible row references (under a NULL list,
fixed-size-list, map or struct row, or outside a sliced parent) is
marked NULL, so a hidden value does not fail the cast and a visible
value outside the declared precision still does; map keys, which cannot
be NULL, stay checked.
- FLOAT / DOUBLE `>=` runtime filters stay residual-only. Lance compares
floats by totalOrder, so a pushed `x >= c` dropped negative-NaN rows
that Doris keeps.
- Runtime filters that mention INT64_MIN stay residual-only. The pinned
Lance SQL parser reads `-9223372036854775808` as the negation of a
magnitude that it parses as i64 first; the overflow turns the literal
into a Float64, which the planner cannot coerce to Int64 or Decimal, so
a pushed IN, `>=` or `<=` filter on an INT64 or scale-zero decimal
column failed planning and the query failed. Every other i64 is still
pushed.
- The shared Arrow DATE32 decoding
(`DataTypeDateV2SerDe::read_column_from_arrow`) rejects days outside
0000-01-01..9999-12-31 instead of returning a non-null `0000-00-00`,
skips null slots and adds the epoch in 64 bits: the DATE32 range part of
branch-4.1 / 4.2 #68649. It also changes ADBC, Arrow stream load and
paimon-rust reads.
- Child values hidden by a NULL list, fixed-size-list, map or struct row
are not decoded: the array, map and struct SerDes give the element SerDe
a validity bitmap that also clears those slots
(`mask_arrow_children_hidden_by_null_parents()`). Arrow leaves them
unspecified, and Lance's v2.1 miniblock encoding keeps them, so a hidden
10000-01-01 under a NULL fixed-size list used to fail the DATE32 range
check above. Visible values are unchanged; this applies to ADBC, Arrow
stream load and paimon-rust as well. The masks come from ExecEnv's
tracked Arrow pool. A nested Lance array that is compacted before
decoding is masked the same way, at every level, before its
`ValidateFull()`, so a hidden invalid UTF-8 string, out-of-precision
decimal, Date64 or time no longer fails the batch while a visible one
still does. Since #69697 the same masks keep hidden DATETIMEV2 /
TIMESTAMPTZ values out of its range checks. A mask costs at most one
pass per batch: a NULL row that owns no child slots, or only NULL ones,
allocates nothing, and Arrow stream load masks each retained IPC batch
once (`mask_arrow_descendants_not_visible()`) before it hands row ranges
to the SerDes, so its per-block reads allocate nothing. The one
per-block mask left is for map keys under a NULL map row that still owns
entries, since a MapArray cannot hold NULL keys; Arrow's map builder
writes NULL map rows without entries.
- Core: `to_protobuf` keeps `NULL_TYPE` for a NULL literal type, so a
Null leaf in ARRAY / MAP / STRUCT no longer comes back as BOOLEAN. The
nullable serde reads an Arrow Null array as NULLs without touching
buffers that the array does not have.
- Configs, caches and metrics, with the 4.x names and defaults:

  | Config | Default |
  |---|---|
  | `lance_index_cache_size_bytes` | 10 GiB |
  | `lance_metadata_cache_size_bytes` | 1 GiB |
  | `enable_lance_data_cache` | true |
  | `lance_data_cache_path` | `${DORIS_HOME}/lance_data_cache` |
  | `lance_data_cache_disk_capacity_bytes` | 100 GiB |
  | `lance_data_cache_read_block_size_bytes` | 1 MiB |
  | `lance_io_buffer_size_bytes` (mutable) | 2 GiB |
  | `lance_batch_readahead` (mutable) | 5 |
  | `lance_fragment_readahead` (mutable) | 5 |

The BE-wide Lance session (index cache, metadata cache and the optional
Foyer disk cache) is created by the first Lance scan, so a BE that never
reads Lance creates no cache directory. The first six configs take
effect after a restart. A negative cache size (and, while the data cache
is enabled, a negative disk capacity or read block size) fails the first
Lance scan with a message that names the setting, as does any data-cache
value lance-c rejects; the rest of the BE keeps serving. Ten
`lance_session_*` metrics report the two in-memory caches. With the
defaults, the disk tier keeps 51,200 sparse 2 MiB block files open from
the first Lance scan on (100 GiB in blocks of twice the read block
size), the same as on branch-4.1 / 4.2. Size
`lance_data_cache_disk_capacity_bytes` and
`lance_data_cache_read_block_size_bytes` against the BE's open-file
limit, or set `enable_lance_data_cache = false`.

The index and metadata caches live in lance-c memory: they are not
charged to a MemTracker or registered with CacheManager, so memory GC,
the cache capacity adjustment under memory pressure and
`/api/clear_cache/all` do not shrink them, and `enable_lance_data_cache
= false` does not turn them off. lance-c has no call to clear, evict or
resize them, so size them with the two configs. The disk tier keys
blocks by Lance's store prefix (`scheme$bucket` for S3, GCS and OSS,
without the endpoint) and the object path
(`thirdparty/patches/lance-c-foyer.patch`). Several endpoints that serve
the same bucket name and data-file path with different bytes (data files
registered under chosen names or replaced in place; Lance's writers use
random file names) share entries, also across a BE restart; such a
deployment should set `enable_lance_data_cache = false`. Both are as on
branch-4.1 / 4.2 and tracked as follow-ups.

A Lance scan's native working set, that is the I/O buffered up to
`lance_io_buffer_size_bytes` per scanner, the decode readahead and the
operators of a search plan such as the top-k of a flat vector search, is
allocated inside lance-c and is charged to neither the query nor its
workload group. It counts toward the BE process memory only, so memory
GC, which ranks queries by tracked memory, does not attribute it to the
Lance query. lance-c accepts no memory budget, so on BEs with little
memory headroom size `lance_io_buffer_size_bytes`,
`lance_batch_readahead` and `lance_fragment_readahead` for the number of
concurrent Lance scanners. As on branch-4.1 / 4.2; tracked as a
follow-up.
- Build: macOS links `-framework IOKit`. This is the first time objects
of `liblance_c.a` are pulled into `doris_be`, so the binary and the link
time grow. With this PR `util/rle_encoding.h` reaches 231 TUs and
`util/threadpool.h` 589, the same as master `315c2e00331` alone (limits
275 and 660 since #69717), and `gen_cpp/internal_service.pb.h` 562
(master alone 559, limit 660); the reach budgets are unchanged.

Not included: the Lance index metadata scanner (`LANCE_INDEX_ENTRIES`);
the FE side, including the rolling-upgrade guard that keeps Lance lazy
materialization off while an old BE could decode a version 1 row
location as version 0; the regression suites; the rest of #68707
(searching historical versions, tags and branches, rejecting fragment
ids missing from the snapshot, naming the snapshot in errors and the
profile). The `InitReaderAvgTime` / `GetBlockAvgTime` profile fix that
#67005 carried goes in a separate PR.

**Results.**

| | master | with this PR |
|---|---|---|
| A `FORMAT_LANCE` query | fails before reading anything | read by
`LanceTableReader` in FileScannerV2 |
| A Lance row in the second phase of TopN lazy materialization | cannot
be represented | a version 1 row location, read back with `take_rows` |
| A row location of an unknown version or size in phase two | decoded as
version 0 | rejected with an error |
| An Arrow DATE32 day outside 0000-01-01..9999-12-31 (ADBC, Arrow stream
load, paimon-rust, Lance) | read as a non-null `0000-00-00` | the read
fails with `InvalidArgument` |

BE unit tests (ASAN, macOS arm64, against master's pinned lance-c), on
the head rebased onto master `378404eeb12`: 1029 cases in 54 suites run,
1027 pass and 2 Parquet microbenchmarks skip by design. They cover the
Lance suites (107: `LanceTableReader*` 83, `LanceNestedNullTest` 10,
`LanceRuntimeFilterHelperTest` 10, `LanceSessionManagerTest` 4); the
row-location protocol (`MaterializationSharedStateTest`,
`RowIdStorageReaderTest`, and the new `RowIdFetchRequestValidationTest`:
count mismatch, unknown version, a Lance version on an internal mapping,
an internal row id of 2^32); `FileScannerV2Test` (28); the ORC and
Parquet reader suites (`NewOrcReaderTest`, `NewParquetReaderTest`,
`ParquetScanTest` and the other `Parquet*` suites), whose row ids now go
through `file_local`; and the Arrow / protobuf / DATE serde suites with
the new `ArrowParentNullsTest`, plus the Iceberg and Parquet reader
suites that decode Arrow dates. Reverting any of the review fixes, the
#68813 flag, the #68707 split rule or the #68815 bounds makes its new
tests fail. The sixth review round adds 13 cases (`ArrowParentNullsTest`
10, `LanceTableReaderTypeTest` 3) and extends one; with them the Arrow
SerDe, Lance, ADBC and `IcebergV2ReaderTest` suites run 317 cases: 310
pass and the 7 ADBC SQLite cases skip without a local SQLite driver. The
ninth review round adds
`LanceRuntimeFilterHelperTest.KeepsInt64MinFiltersResidualOnly` and
`LanceTableReaderFilterTest.KeepsInt64MinRuntimeFiltersOutOfLanceScanner`:
both pass with the 106 Lance cases (`Lance*`) on the branch rebased onto
`339e087b11f`, and restoring `parse_int64()` in either check makes
exactly these two fail. On the head rebased onto master `315c2e00331`,
with the per-block mask fix (`42cf4e3e5d1`), 328 cases in 24 suites
pass: the Arrow SerDe and validation suites, `ArrowParentNullsTest`
(15), `LoadArrowPipeTest` (47), the DATETIMEV2 / TIMESTAMPTZ SerDe
suites, `ArrowColumnToDorisColumnTest`, `ArrowBlockConvertorTest`,
`AdbcReaderTest`, `RemoteDorisV2ReaderTest`, `PaimonRustTableReaderTest`
and the Lance suites. Allocating at the first NULL row of each call
again fails the new `ArrowParentNullsTest` case and the four new
`LoadArrowPipeTest` block cases, dropping the per-batch mask fails the
LIST and STRUCT block cases, and returning the child unmasked fails the
new timestamp case, four `ArrowParentNullsTest` cases and
`LanceTableReaderTypeTest.IgnoresDate32PayloadHiddenByNullParents`.

### Release note

Add nine `lance_*` BE configs and `lance_session_*` BE metrics. They
take effect once the FE can plan Lance scans. Reading an Arrow DATE32
value outside 0000-01-01..9999-12-31 through ADBC, Arrow stream load or
paimon-rust now fails instead of returning 0000-00-00.

### Check List (For Author)

- Test <!-- At least one of them must be included. -->
    - [ ] Regression test
    - [x] Unit Test
- `run-be-ut.sh --run --filter=...` with the suites listed under
Results: 420 of 420 pass on macOS arm64 (ASAN) on the content before the
rebases, and the Lance and Arrow SerDe suites pass again with the
nested-offset, zero-field struct, hidden-child, hidden-Decimal256 and
compaction-mask fixes (the ADBC SQLite cases skip without a local SQLite
driver). Rebased onto master `43f9ceb48d7`: #69668 conflicted in the
scanner selection and in `fetch_table_schema`, which now hands every
file format to `format::fetch_file_schema()`, so the Lance branch runs
ahead of that call; #69646 added a TopN row-group test that reads the
version 0 row location fields, which now go through `file_local` (the
earlier rebase onto `5caaeb3e3d4` resolved #69614 and #69601 in the
scanner selection, the `FileFormat` enum, its test and the reach budget
table). The 95 TUs that this PR changes or that master changed since
`5caaeb3e3d4` were syntax-checked against the rebased tree with the UT
build flags, and CI runs the suites again. Rebased again onto master
`8d88f1cc6f7`: the only conflict was the budget table, where #65858
moved the `util/rle_encoding.h` baseline to 828, and the 163 TUs that
this PR changes or that master changed since `43f9ceb48d7` pass the same
syntax check. Rebased again onto master `339e087b11f`: the only conflict
was the budget table again, where #67292 moved the `util/rle_encoding.h`
baseline to 834; #69872 added Arrow type checks ahead of the NULL-parent
masking in the map and struct readers and merged cleanly. Of the 114
sources that this PR changes or that master changed since `8d88f1cc6f7`,
111 pass the same syntax check. The other three are master's: two new
#67292 files under `storage/segment` fail the same way on master alone,
since they compile only inside their unity batch, and this PR adds no
source to that target; the #69818 benchmark is not in the UT build and
includes only `util/simd` headers that this PR does not touch. Reverting
any of the review fixes, the #68813 flag, the #68707 split rule or the
#68815 bounds makes its new tests fail. Rebased again onto master
`378404eeb12`, which removed the legacy file scanner (#69723): the
conflicts were in the scanner-selection code and tests that #69723
deleted, so this PR's Lance lines there went with them (including its
`LanceQueriesAlwaysUseScannerV2` test), in the profile keys of
`materialization_opertor.cpp`, and in the row fetch of
`rowid_fetcher.cpp`, whose non-Lance path is now only
`FileScannerV2::read_by_rows`; the deleted V1 ORC / Parquet row id tests
took this PR's `file_local` rename with them, and the
`util/rle_encoding.h` budget commit is dropped because master is no
longer near that limit. Of the 100 sources that this PR changes or that
master changed since `339e087b11f`, 99 pass the same syntax check; the
other, `format_v2/orc/orc_reader.cpp`, fails the same way on master
alone. On the rebased head the suites listed under Results run 1029
cases: 1027 pass and 2 microbenchmarks skip. The test-utility fix that
`DataTypeDateTimeV2SerDeTest` needed on macOS (#69695) is now on master.
Rebased again onto master `315c2e00331`: the only conflict was
`TExternalSearchRequest` in `PlanNodes.thrift`, where #70075 had
reserved fields 2-5 in comments; the two commits that add the fields
replace those comments, and field 6 stays #70075's `paimon_options`.
#69717 cut transitive includes from shared headers: of the 160 sources
that this PR changes or that master changed since `378404eeb12`, 156
pass the same syntax check; the two `jemalloc_hook` tests have no
compile command on macOS, `runtime/memory/jemalloc_hook.cpp` uses glibc
aliases that macOS cannot build (#69819), and
`format_v2/table_reader.cpp` alone fails the same way on master alone
while its unity batch compiles. The full `doris_be_test` builds and
links on the rebased head; `ArrowParentNulls*` and `Lance*` (121 cases)
pass before the mask fix, and the 328 cases above with it.
- `build-support/check-build-hygiene.sh` (on this branch rebased onto
master `315c2e00331`), clang-format 16 and
`build-support/run-clang-tidy.sh` on the changed files: all pass; on the
changed lines clang-tidy (llvm 22) reports only
`readability-inconsistent-ifelse-braces` on the `DORIS_CHECK` lines of
the masking functions, the macro's expansion that master's existing
`DORIS_CHECK`s report too.
    - [ ] Manual test (add detailed scripts or steps below)
    - [ ] No need to test or manual test. Explain why:
- Behavior changed:
    - [ ] No.
- [x] Yes. `PRequestBlockDesc.row_id` is `uint64`; phase two of TopN
lazy materialization rejects row locations of an unknown version or
size; `FORMAT_LANCE` scans run on FileScannerV2; an Arrow DATE32 day
outside the Doris DATE range fails the read instead of becoming
`0000-00-00`.
- Does this need documentation?
    - [x] No. Lance is documented with the FE connector.
- [ ] Yes. <!-- Add document PR link here. eg:
apache/doris-website#1214 -->

### Check List (For Reviewer who merge this PR)

- [ ] Confirm the release note
- [ ] Confirm test cases
- [ ] Confirm document
- [ ] Add branch pick label <!-- Add branch pick label that this PR
should merge into -->

---------

Co-authored-by: zhangstar333 <[email protected]>
Co-authored-by: Gabriel <[email protected]>
Co-authored-by: zhaobo wang <[email protected]>
Co-authored-by: jay <[email protected]>
Co-authored-by: FANNG <[email protected]>
Co-authored-by: Claude Opus 5.5 (1M context) <[email protected]>
Co-authored-by: zy-kkk <[email protected]>
Gabriel39 pushed a commit that referenced this pull request Nov 6, 2026
…t_generation (#70308)

### Background

An auto-partition table with `partition.retention_count = N` keeps only
its N latest historical partitions: the FE's `DynamicPartitionScheduler`
drops older ones on every run, once per
`dynamic_partition_check_interval_seconds` (600 by default).
`partition_p0/auto_partition/test_auto_new_recycle` lowers that interval
to 1 s and checks the partition count after each load or ALTER.
`audit/test_sql_digest_generation` checks that the audit log gets an
`sql_digest` only for queries slower than the FE config
`sql_digest_generation_threshold_ms`, once with the threshold at 0 and
once at 100000. Both suites are in group `nonConcurrent`.

### What problem does this PR solve?

Issue Number: None
Related PR: #65375 and #65646 (the branch-4.1 recycle-suite fixes by
@shuke987, ported here), #65343 (the branch-4.1 digest-suite fix by
@shuke987, ported here), #65435 (master's earlier move of the recycle
suite's day-boundary guard)
PR Type: Test

Problem Summary:

Both suites rely on timing the product does not promise. The recycle
suite checks the partition count after a fixed `sleep(8000)` (5 s after
an ALTER), while the drop happens asynchronously at an unbounded time
after the config change; branch-4.1's NonConcurrent pipeline failed at
the first such check repeatedly (#65375). The digest suite's threshold-0
check needs the query's elapsed time to be above 0 ms. This PR gives
both suites their branch-4.1/4.2 form: the recycle suite polls the
partition count every second, for at most 1200 s, and widens its
day-boundary guard to cover the poll; the digest query adds `sleep(1)`.
Both files become identical to branch-4.2.

### Details

- **Failure:** On master's public NonConcurrent pipeline neither suite
has failed on its own since mid-July: of about 8,000 runs each, the
recycle suite failed 3 times and the digest suite 87 times, all in
builds whose cluster had died (most suites failing). The failures that
motivated the fixes are on branch-4.1's pipeline (#65375); #65343 made
the digest query deterministic there.
- **Cause:** `ADMIN SET FRONTEND CONFIG` sets the scheduler's new
interval at once, but the scheduler sleeps in 5-second slices and
notices the change only at the next slice. It then runs
`executeDynamicPartition` over every registered table before the drop.
So the drop lands 0 to 5 s plus one full pass after the config change,
and a pass longer than about 3 s fails the 8-second check. In the digest
suite, `AuditLogHelper` writes a digest only when `elapseMs >
sql_digest_generation_threshold_ms`, and nothing bounds the elapsed time
of `select *` on a two-row table from below (9 to 15 ms locally). With
`sleep(1)` on each row it takes about 2 s, still far below the second
check's 100000 ms.
- **Stability evidence:** The recycle checks now wait for the drop and
fail only when the count misses the target for 1200 s. The poll runs on
the suite thread, because the framework sets
`Awaitility.pollInSameThread()` (#68338), so it does not leak
connections. Shortening one of master's waits to 1 s reproduces the
branch-4.1 failure shape, and the ported suite passes; mutants show that
both suites still catch wrong results (see Testing).

### Testing

All runs used `./run-regression-test.sh --run -d
partition_p0/auto_partition -s test_auto_new_recycle` or `--run -d audit
-s test_sql_digest_generation` against a local single-FE, single-BE
cluster built from master.

| Suite | Version | Result |
|---|---|---|
| `test_auto_new_recycle` | master | passes |
| `test_auto_new_recycle` | this PR | passes; the polls reached the
target after 1, 2, 1, 3 and 3 s |
| `test_auto_new_recycle` | master, with the wait before the
10-partition check cut to 1 s | fails as #65375 describes: `expected:
<101> but was: <10>` (master's `assertEquals` has its arguments swapped,
so 101 partitions were still there) |
| `test_auto_new_recycle` | this PR, expecting 11 partitions, poll
capped at 20 s | fails as it should: `ConditionTimeoutException` |
| `test_sql_digest_generation` | master | passes |
| `test_sql_digest_generation` | this PR | passes; the query took 2,024
and 2,025 ms |
| `test_sql_digest_generation` | this PR, second threshold 1000 ms |
fails as it should: `Count of sql_digest changed` |

The PR NonConcurrent pipeline runs both suites; cloud_p0 skips
`nonConcurrent` suites.

### Notes for Reviewers

- The day-boundary guard before the final `now()`-based section goes
from 100 s to 1300 s, as in #65646, so that it covers the 1200 s poll
plus the insert and checks. It sleeps only when that section would
otherwise start within 1300 s of midnight.
- Four more suites fixed in the same branch-4.1 batch need nothing on
master: #65340 (master #68449 enables the publish debug point on all
FEs, superseding the open port #65556), #65929 (master #68329 fixed the
actual cause, a connection leak), #65885 (master #69723 removed the V1
scanner whose message it also accepted), and #67363 (ported as #67402).

### Release note

None

### Check List (For Author)

- Behavior changed: No
- Does this need documentation: No
- Pre-submission status: Ready

### AI Disclosure

- Tool: Claude Code (Claude Opus 5.5)
- Generated: the port (both suites taken from branch-4.2), the local
verification runs, and this description
- Human review: not stated
- Prompts: The author asked to continue moving branch-4.2's changes back
to master, where six branch-4.1 test fixes were recorded as one task.
After the check showed that master already covers four of them and has
not failed on the other two, the author chose to port those two exactly
as branch-4.2 has them and to verify them on a local cluster.

Co-authored-by: shuke <[email protected]>
Co-authored-by: Claude Opus 5.5 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

6 participants