Skip to content

[test](be) Drop the std::set::end() dereference in load_data_from_csv - #69695

Merged
morningman merged 1 commit into
apache:masterfrom
morningman:b2m-t37e-test-util-set-end
Oct 29, 2026
Merged

morningman merged 1 commit into
apache:masterfrom
morningman:b2m-t37e-test-util-set-end

Conversation

@morningman

Copy link
Copy Markdown
Contributor

What problem does this PR solve?

Issue Number: None

Related PR: #48788

Problem Summary:

In short. load_data_from_csv() in the BE test utilities dereferences std::set::end() in an assertion. With libc++ (macOS) that reads past the set on the stack, so under ASAN every BE unit test suite that loads a CSV fixture aborts while loading its fixtures and takes the rest of the doris_be_test run with it. This removes the assertion; it only repeated the size check on the line above.

Background. #48788 added load_data_from_csv() (be/test/testutil/test_util.cpp), which the column, data type and SerDe unit tests use to load their CSV fixtures. It checks that the SerDes, the columns and the requested column indexes line up: serders.size() == columns.size(), serders.size() == idxes.size(), and then serders.size() == *idxes.end().

The problem, and what it cost. Dereferencing the end iterator of a std::set is undefined behavior. In libstdc++ the end iterator points at the tree header, whose value slot overlaps the element count, so on Linux the last assertion compares against idxes.size() again and passes. In libc++ it reads outside the set object: ASAN reports a stack-buffer-overflow, and with abort_on_error the whole doris_be_test process stops. The suites of all 24 test files that load CSV fixtures (DataTypeDateTimeV2SerDeTest, DataTypeNumberSerDeTest, ColumnStringTest, ...) cannot run on macOS, and a test filter that includes one of them ends the run there. CI runs on Linux only, so it never failed there.

How this PR fixes it. Remove the assertion. The size check right above it already guarantees one index per SerDe. No production code changes. #63474, open since May, carries the same removal among its macOS build fixes; this PR takes only this line, so the test fix does not wait for the rest.

Results.

macOS arm64, ASAN Linux
Before DataTypeDateTimeV2SerDeTest aborts in SetUpTestSuite with a stack-buffer-overflow passes (the assertion repeats the size check)
After the 24 CSV-fixture suites (364 tests) pass unchanged

Release note

None

Check List (For Author)

  • Test
    • Regression test
    • Unit Test
      • macOS arm64 (ASAN): the 24 suites whose test files load CSV fixtures pass, 364 tests. With the assertion back, DataTypeDateTimeV2SerDeTest fails in SetUpTestSuite with an ASAN stack-buffer-overflow at that assertion. FunctionCastToDecimalPerfTest, a performance test that also loads a CSV file, was not run.
    • Manual test (add detailed scripts or steps below)
    • No need to test or manual test. Explain why:
  • Behavior changed:
    • No.
    • Yes.
  • Does this need documentation?
    • No.
    • Yes.

Check List (For Reviewer who merge this PR)

  • Confirm the release note
  • Confirm test cases
  • Confirm document
  • Add branch pick label

### What problem does this PR solve?

Issue Number: None

Related PR: apache#48788

Problem Summary:

`load_data_from_csv()` in `be/test/testutil/test_util.cpp` asserts
`serders.size() == *idxes.end()` right after asserting
`serders.size() == idxes.size()`. Dereferencing `std::set::end()` is
undefined behavior. With libstdc++ (the Linux CI) the end iterator is the
tree header, whose value slot overlaps the element count, so the assertion
repeats the size check above it. With libc++ (macOS) it reads past the set
object on the stack: under ASAN every BE unit test suite that loads a CSV
fixture (`DataTypeDateTimeV2SerDeTest`, `DataTypeNumberSerDeTest`,
`ColumnStringTest` and the other suites of 24 test files) reports a
stack-buffer-overflow while loading the fixture, and `abort_on_error` stops
the whole `doris_be_test` run.

The assertion adds nothing to the size check, so this removes it. No
production code changes.

### Release note

None

### Check List (For Author)

- Test: Unit Test
    - macOS arm64 (ASAN): the 24 CSV-fixture suites (364 tests) pass with
      this change; with the assertion back, `DataTypeDateTimeV2SerDeTest`
      fails in `SetUpTestSuite` with an ASAN stack-buffer-overflow at that
      assertion.
- Behavior changed: No (test utility only)
- Does this need documentation: No

Co-Authored-By: Claude Opus 5.5 <[email protected]>
@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?

@morningman

Copy link
Copy Markdown
Contributor Author

run buildall

@morningman

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 #69695 at 580f925. No actionable regressions found in the complete supplied diff. Removing the dereference of std::set::end() eliminates undefined behavior; both meaningful SerDe/column/index cardinality assertions remain.

Critical checkpoints:

  1. Goal and evidence: The deletion addresses the invalid iterator access directly. Existing DataTypeDateTimeV2SerDeTest, DataTypeNumberSerDeTest, and ColumnStringTest fixture setup exercises the changed global helper through load_columns_data_from_file. Runtime results were not independently verified.
  2. Scope and clarity: The change is limited to the invalid assertion and its diagnostic in one test utility.
  3. Concurrency: No threads, shared-state accesses, locks, or lock ordering change.
  4. Lifecycle: No ownership, resource lifetime, or static initialization change.
  5. Configuration: No configuration is added or changed.
  6. Compatibility: No production symbols, storage formats, dependencies, or GLIBC requirements are added.
  7. Thrift/protobuf: No generated definitions or field IDs change.
  8. Parallel paths: Both regular-file and directory wrappers reach the repaired helper. The separate CommonDataTypeTest member loader already omits this assertion.
  9. Conditional checks and invariants: The remaining fatal size assertions preserve the meaningful preconditions. Selected CSV positions map to dense destination indexes; valid nonzero selections such as {2} are supported. The deleted end iterator dereference cannot establish a maximum-index bound.
  10. Test coverage: Existing fixture-loading tests cover this path, including global-helper callers with single and multiple selected fields. No new tests are added; builds, tests, sanitizer runs, and benchmarks were prohibited by the static-only review contract.
  11. Expected results: No expected-result files are added or changed.
  12. Observability: Existing diagnostics remain sufficient for this assertion removal; no new execution path requires metrics or logs.
  13. Transactions and persistence: No transaction, replay, failover, or persistent metadata behavior changes.
  14. Data writes and atomicity: Only test fixture loading is affected; production data writes, visibility/version rules, and delete bitmap invariants are unaffected.
  15. FE/BE propagation: No cross-process variables or protocol paths change.
  16. Performance and memory: Removing the invalid expression adds no allocations or work and removes an invalid memory access.
  17. Other issues: Caller, wrapper, parallel-loader, and edge-case inspection found no additional introduced issue. Empty-input handling and malformed-fixture behavior already present in the helper are outside this deletion's changes.

No additional user review focus was provided. There are no existing inline comments or independently confirmed existing P0/P1 blockers to carry forward. Both assigned reviewers reported NO_NEW_VALUABLE_FINDINGS after their final rechecks. The review converged in round 1; all candidates and risk focus R1 are disposed, with no remaining static coverage gaps. Validation is limited to static inspection.

Review converged after 1 round(s).

@hello-stephen

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

------ Round 1 ----------------------------------
============================================
q1	17595	5196	5063	5063
q2	2224	335	269	269
q3	10190	1394	843	843
q4	4691	468	346	346
q5	7457	839	538	538
q6	184	186	156	156
q7	780	807	619	619
q8	9294	1368	1148	1148
q9	5439	4204	4186	4186
q10	6837	1292	1044	1044
q11	441	251	231	231
q12	645	403	278	278
q13	18091	3646	3039	3039
q14	271	280	256	256
q15	q16	755	719	662	662
q17	1204	750	587	587
q18	7085	6196	6137	6137
q19	1257	941	603	603
q20	394	332	225	225
q21	5124	2126	2283	2126
q22	357	291	254	254
Total cold run time: 100315 ms
Total hot run time: 28610 ms

----- Round 2, with runtime_filter_mode=off -----
============================================
q1	5626	5630	5835	5630
q2	688	566	550	550
q3	5310	5378	5001	5001
q4	1988	2133	1552	1552
q5	5264	5194	5832	5194
q6	241	195	149	149
q7	2362	2071	1899	1899
q8	3110	2815	2639	2639
q9	8120	8041	8070	8041
q10	3743	3654	3284	3284
q11	608	453	398	398
q12	673	689	518	518
q13	3719	3827	3255	3255
q14	314	304	281	281
q15	q16	691	732	654	654
q17	8494	7529	6966	6966
q18	12928	12131	13011	12131
q19	913	813	801	801
q20	2136	2154	1916	1916
q21	5620	4651	4784	4651
q22	491	476	434	434
Total cold run time: 73039 ms
Total hot run time: 65944 ms

@hello-stephen

Copy link
Copy Markdown
Contributor
TPC-DS: Total hot run time: 151655 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 580f9252c4a0bc5ba9ec65792fff057f203989f9, data reload: false

query5	4302	591	443	443
query6	433	215	189	189
query7	4802	438	224	224
query8	319	170	152	152
query9	8789	3205	3213	3205
query10	494	313	260	260
query11	5872	3859	3549	3549
query12	149	107	88	88
query13	1247	447	323	323
query14	6098	3966	3668	3668
query14_1	3412	3421	3409	3409
query15	210	220	182	182
query16	964	436	421	421
query17	845	660	540	540
query18	2437	421	318	318
query19	212	166	137	137
query20	85	84	87	84
query21	214	131	107	107
query22	12950	13115	12828	12828
query23	13422	12921	12587	12587
query23_1	12883	12609	12518	12518
query24	7380	827	492	492
query24_1	488	492	481	481
query25	535	423	362	362
query26	1274	259	142	142
query27	2760	437	283	283
query28	4628	2070	2035	2035
query29	1565	582	419	419
query30	298	222	182	182
query31	995	927	809	809
query32	137	89	85	85
query33	512	290	229	229
query34	898	857	505	505
query35	722	772	638	638
query36	821	799	726	726
query37	111	93	83	83
query38	1764	1709	1698	1698
query39	793	781	748	748
query39_1	739	725	734	725
query40	228	116	91	91
query41	67	62	62	62
query42	84	82	79	79
query43	356	366	316	316
query44	1298	697	705	697
query45	180	174	170	170
query46	825	953	574	574
query47	2760	2952	2780	2780
query48	302	306	219	219
query49	570	397	290	290
query50	672	275	214	214
query51	10171	10389	10410	10389
query52	78	80	68	68
query53	185	206	156	156
query54	278	195	189	189
query55	69	66	63	63
query56	238	216	223	216
query57	1663	1600	1562	1562
query58	280	264	261	261
query59	2145	2169	2010	2010
query60	287	234	224	224
query61	143	137	145	137
query62	391	332	309	309
query63	181	155	155	155
query64	2751	970	782	782
query65	3343	3304	3258	3258
query66	1777	412	293	293
query67	20301	20034	20031	20031
query68	3090	1005	628	628
query69	409	309	243	243
query70	830	769	777	769
query71	304	241	207	207
query72	2703	2573	2365	2365
query73	517	539	304	304
query74	4624	4510	4320	4320
query75	2362	2294	1918	1918
query76	2317	1012	599	599
query77	350	383	282	282
query78	9663	9691	9238	9238
query79	990	858	546	546
query80	1161	439	382	382
query81	559	323	277	277
query82	589	140	109	109
query83	311	203	238	203
query84	311	131	103	103
query85	876	434	376	376
query86	362	242	233	233
query87	2020	1941	1821	1821
query88	3776	2817	2742	2742
query89	332	277	243	243
query90	1821	160	168	160
query91	167	161	127	127
query92	97	88	89	88
query93	1007	994	580	580
query94	554	301	268	268
query95	577	381	305	305
query96	680	489	242	242
query97	2432	2425	2312	2312
query98	160	162	159	159
query99	759	754	660	660
Total cold run time: 234630 ms
Total hot run time: 151655 ms

@hello-stephen

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

query1	0.01	0.01	0.00
query2	0.10	0.06	0.04
query3	0.26	0.08	0.09
query4	1.61	0.10	0.11
query5	0.24	0.24	0.25
query6	1.17	0.70	0.69
query7	0.04	0.01	0.01
query8	0.05	0.04	0.04
query9	0.40	0.35	0.35
query10	0.61	0.57	0.61
query11	0.20	0.15	0.14
query12	0.19	0.15	0.15
query13	0.49	0.49	0.50
query14	1.02	1.05	1.01
query15	0.63	0.61	0.60
query16	0.36	0.37	0.36
query17	1.16	1.16	1.16
query18	0.22	0.19	0.19
query19	2.14	1.93	2.04
query20	0.02	0.01	0.01
query21	15.49	0.18	0.14
query22	4.99	0.06	0.06
query23	16.27	0.34	0.12
query24	2.86	0.66	0.38
query25	0.13	0.06	0.06
query26	1.65	0.22	0.17
query27	0.04	0.05	0.03
query28	3.61	0.76	0.35
query29	12.49	4.20	3.33
query30	0.28	0.13	0.10
query31	2.78	0.58	0.32
query32	3.22	0.60	0.51
query33	3.26	3.21	3.16
query34	15.33	4.05	3.39
query35	3.34	3.35	3.36
query36	0.54	0.42	0.42
query37	0.09	0.07	0.06
query38	0.05	0.03	0.04
query39	0.04	0.03	0.03
query40	0.18	0.15	0.15
query41	0.08	0.03	0.02
query42	0.04	0.02	0.03
query43	0.04	0.04	0.03
Total cold run time: 97.72 s
Total hot run time: 24.39 s

@hello-stephen

Copy link
Copy Markdown
Contributor

BE Regression && UT Coverage Report

Increment line coverage 100% (0/0) 🎉

Increment coverage report
Complete coverage report

Category Coverage
Function Coverage 77.49% (36196/46709)
Line Coverage 62.72% (409071/652268)
Region Coverage 59.18% (346242/585084)
Branch Coverage 60.11% (160297/266691)

@morningman
morningman merged commit 49b0bee into apache:master Oct 29, 2026
38 checks passed
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]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants