Repository navigation
[feature](be) Read Lance datasets with FileScannerV2 - #69581
morningman wants to merge 14 commits into
Conversation
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
|
run buildall |
|
/review |
![github-actions[bot]](https://avatars.githubusercontent.com/in/15368?s=60&v=4){kind=small}
There was a problem hiding this comment.
Request changes. Static review of #69581 at 6d43ec5 found five verified defects: a Decimal256 conversion crash, two search-request compatibility failures, a floating runtime-filter false negative, and silent DATE32 corruption. Every finding is inline; no existing inline threads needed carrying forward.
Critical checkpoint conclusions:
- Goal and proof: the BE Lance read/search/schema-fetch/lazy-fetch integration is present, but the five defects prevent a correctness-complete port. Added unit tests provide source coverage; no tests were executed in this static-only review.
- Scope and clarity: reader helpers, dispatch, protocol, core NULL handling and row-layout changes are related to the port; no unrelated production edit identified.
- Concurrency: Scanner serializes reader operations; phase-two workers retain task tracking and completion/semaphore cleanup; call_once publishes the session; entity locking serializes metric hooks and teardown. No additional verified lock-order defect.
- Lifecycle and static initialization: partial-creation guards and Arrow import ownership were checked; scanners close before dataset/FTS state, metrics unregister before session release, and the manager uses a function-local static. No additional verified ownership or initialization defect.
- Configuration: six session/cache settings require restart; three mutable scan settings are validated and sampled when each native scanner is created.
- Compatibility: version-zero row locations retain their 24-byte layout; uint64 varint widening preserves old ordinals, and unknown versions/sizes fail. Existing 4.x search-filter timing and distance bounds are lost. Linux glibc compatibility source/link handling was inspected; no final ELF artifact exists to audit.
- Thrift/protobuf: complete touched definitions were compared with live-matching branch-3.1/4.0/4.1/4.2 refs. No ID reuse, changed enum meaning or new required field found; the two omitted request options still break query semantics across upgrades.
- Parallel paths: normal/vector/FTS/schema/take_rows and internal/Parquet/ORC row fetching were inspected. Existing FE producer guarantees pin search snapshots, normalize split offsets, and exclude nested lazy slots. The decimal and temporal conversion defects affect the shared new Lance boundary.
- Conditions and errors: new structural/version/count checks have concrete failure paths. The reported decimal crash and ignored DATE32 conversion failure need explicit handling.
- Test coverage: substantial BE tests cover types, extensions, nulls, searches, fragments, uint64 IDs and malformed requests. Missing cases include low-precision Decimal256, out-of-range DATE32, negative-NaN probe rows, and old-FE search-option serialization. FE connector and regression suites are separate PR work.
- Expected results: source assertions and all 109 fixture files (99 binary files and 10 JSON hints) were inspected; no new textual regression outputs were added. Fixture generation and executable results were not independently validated.
- Observability: reader/search/fetch timing and cache counters plus process-wide cache metrics are present. Native/cache statistics are finalized at stream/dataset close; live execution attribution was not measured.
- Transactions, persistence and failover: no table-write, EditLog, transaction or MoW visibility path changes; supported search fetching uses fixed dataset snapshots.
- Writes and crash behavior: persistent writes are disposable cache data, not table commits. The Decimal256 finding can crash a BE while reading valid data.
- FE/BE variables: request producers/consumers and row-version transport were traced; prefilter and distance-bound inputs are dropped despite accepting schema_version=1.
- Performance and memory: schema plans and dataset-grouped takes reuse work; Arrow normalization uses its supplied pool. Native Rust/cache accounting, throughput, cancellation stress and final Linux ABI remain execution-validation limits; no build/runtime claim is made.
- Other concerns and user focus: no additional user focus was supplied. Remaining suspected mechanisms were independently resolved or excluded using producer/native evidence; all assigned static coverage and candidate dispositions are recorded in the review ledger.
Review converged after 3 round(s).
| const PrimitiveType doris_decimal_type = precision <= 9 ? TYPE_DECIMAL32 | ||
| : precision <= 18 ? TYPE_DECIMAL64 | ||
| : precision <= 38 ? TYPE_DECIMAL128I | ||
| : TYPE_DECIMAL256; |
There was a problem hiding this comment.
[P1] Handle Decimal256 carriers before selecting a narrower SerDe
A valid Lance Decimal256(18,2) is mapped to Doris DECIMAL64 here, but normalization leaves its Decimal256Array unchanged. DataTypeDecimalSerDe<TYPE_DECIMAL64>::read_column_from_arrow casts it to arrow::DecimalArray (Decimal128Array) and then calls Value on the null result, so reading a nonempty column crashes the BE. The pinned Lance datatype encoding preserves the physical 256-bit width even when precision is <=38; current fixtures only use Decimal256 at precision 39/76. Normalize and checked-narrow that carrier, or support both physical widths in the SerDe, including nested leaves.
There was a problem hiding this comment.
Confirmed. Lance keeps the 256-bit carrier of decimal256(18, 2), while this mapping and the FE type the column DECIMAL64 by precision, so DataTypeDecimalSerDe<TYPE_DECIMAL64>::read_column_from_arrow dereferenced the null result of dynamic_cast<const arrow::DecimalArray*>; the same held for DECIMAL32/128I targets and nested leaves. Fixed in 05437fd: LanceArrowArrayNormalizer narrows a Decimal256 field with precision <= 38 to decimal128(p, s) through a safe Arrow cast, so a value outside the declared precision fails with an error naming the field, and list/struct/map parents are rebuilt around the narrowed leaf. Unit tests cover top-level, sliced nested and overflow cases and a decimal256(18, 2) Lance round trip.
| } | ||
| // Fragment-scoped nearest queries require prefiltering before installing the query. The same | ||
| // path applies the TVF search filter, when present. | ||
| if (lance_scanner_set_prefilter(scanner, true) != 0) { |
There was a problem hiding this comment.
[P1] Preserve the 4.x search-filter timing flag
The branch-4.1/4.2 FE serializes TExternalSearchRequest.prefilter=false by default, but this head omits wire field 5 and unconditionally enables prefiltering here. With top_k=1, use_index=false, and filter='keep = true', a nearest row with keep=false followed by a farther keep=true row should yield an empty postfilter result; this BE returns the farther row. The FE keeps that TVF filter only in the request, so a Doris residual cannot restore the requested behavior during an FE-4.x/BE-master upgrade. Retain field 5 with its established default and honor it when configuring the scanner.
There was a problem hiding this comment.
Agreed, fixed in 60aebf8: this PR now ports the BE side of #68813. TExternalSearchRequest.5: optional bool prefilter has the 4.x id and comment, and both vector and full-text search pass __isset.prefilter && prefilter to lance_scanner_set_prefilter(), so unset or false means postfilter as on branch-4.1/4.2. SearchFilterDefaultsToPostfilter (from #68813) and a new full-text case cover both settings.
| 2: optional TSearchVector query_vector | ||
| 3: optional i64 top_k | ||
| 4: optional i64 offset | ||
| 5: optional TVectorMetric metric |
There was a problem hiding this comment.
[P1] Retain the existing vector distance-bound fields
Branch-4.1/4.2 already define fields 6/7 as distance_lower_bound/distance_upper_bound. Their FE writes these from vector_search properties without synthesizing a Doris residual. This definition discards both fields during Thrift deserialization, and the new reader never applies a native distance range while still accepting schema_version=1. During an FE-4.x/BE-master upgrade, an exact top-1 query whose nearest squared L2 distance is 1 and distance_upper_bound=0.5 therefore returns that row instead of an empty result. Preserve the IDs and enforce the range, or explicitly reject unsupported bounded requests.
There was a problem hiding this comment.
Agreed, fixed in 7edd9cc: TVectorSearchParams 6/7 (distance_lower_bound / distance_upper_bound) have the 4.x ids and types, the request is validated as in #68815 (finite FLOAT bounds, lower < upper at FLOAT precision, no multi-vector), and lance_scanner_set_distance_range() is installed after nearest(), using master's pinned lance-c. #68815's fallback for quantized indexes relies on the BE searching a split without index segments flat, which came from #68707, so a66c2a6 ports that rule (use_index follows the split's segments; a split with segments under use_index=false is rejected). #68815's four tests and fixtures come along unchanged.
| return std::nullopt; | ||
| } | ||
| const auto* sql_operator = predicate.op() == TExprOpcode::GE ? ">=" : "<="; | ||
| return "(" + quote_sql_identifier(slot->column_name()) + " " + sql_operator + " " + |
There was a problem hiding this comment.
[P2] Preserve negative NaNs when pushing floating range filters
For a nested-loop join L.x > B.y with finite B.y=1, FE can generate the MIN runtime filter L.x >= 1. A Lance FLOAT containing negative quiet-NaN bits 0xffc00000 satisfies both predicates in Doris, whose Compare treats every NaN as greatest. The pinned Arrow floating comparison uses total ordering, placing negative NaNs below finite values, and DataFusion delegates GE directly to that kernel; this generated SQL therefore drops a matching probe row. Rejecting nonfinite bound literals does not protect against NaN input values. Keep floating range filters residual-only or widen them to preserve every Doris survivor, with differential negative-NaN coverage.
There was a problem hiding this comment.
Confirmed. Doris keeps every NaN for x >= c (GreaterThanEqualsFloat treats NaN as greatest regardless of sign), while the pinned Arrow/DataFusion kernels use totalOrder, so a MIN-only filter could drop a negative-NaN row that Doris returns. <= and IN are safe: Lance only returns extra negative NaNs for <=, which the residual removes, and IN sets with NaN members are not pushed. Fixed in bf5dcbd: FLOAT/DOUBLE >= runtime filters stay residual-only, the policy PaimonRustPredicateConverter already applies, with a helper test and a reader test on 0xffc00000 / 0xfff8000000000000 rows. branch-4.1 and branch-4.2 carry the same helper.
| RETURN_IF_ERROR( | ||
| _array_normalizer_for_arrow_column[static_cast<size_t>(arrow_index)] | ||
| .normalize_for_doris(arrow_column, _memory_pool, &normalized_column)); | ||
| RETURN_IF_ERROR(output.serde->read_column_from_arrow( |
There was a problem hiding this comment.
[P2] Check DATE32 conversion failures at the Lance boundary
The new adapter advertises Arrow DATE32 as supported DATEV2, including valid external dates above Doris's maximum year 9999. For example, day count 2932897 is 10000-01-01. The shared DATE32 Arrow SerDe ignores get_date_from_daynr(false) and appends its zero-initialized value, while the nullable wrapper copies the source's non-null validity bit. This call therefore reports success with a non-null 0000-00-00 value. DATE64 and native Parquet conversion already reject or explicitly null failed conversions. Add equivalent checked range handling to this path, including nested date leaves, and cover the first date above 9999-12-31.
There was a problem hiding this comment.
Confirmed, though the defect is in the shared DataTypeDateV2SerDe::read_column_from_arrow DATE32 branch: master has ignored the get_date_from_daynr() result there since #48944, so ADBC, Arrow stream load and paimon-rust return the same non-null 0000-00-00. branch-4.1/4.2 reject these values in the SerDe (#68649), and the 4.2 Lance code relies on that. Fixed in 6714c7f with the DATE32 range part of #68649: null slots are skipped, the day number is computed in 64 bits, and an out-of-range day returns InvalidArgument like the DATE64 branch next to it, which also covers ARRAY/STRUCT/MAP leaves.
Cloud UT Coverage ReportIncrement line coverage Increment coverage report
|
…er.h ### What problem does this PR solve? Issue Number: None Related PR: apache#69581 Problem Summary: BE configure fails the build-hygiene gate on this PR (COMPILE and BE UT): util/threadpool.h now reaches 1088 TUs, one over its limit of 1087 (baseline 988 +10%). Master reaches 1083. Five of the new Lance TUs reach the header through exprs/vexpr_context.h -> storage/segment/column_reader.h -> io/cache/cached_remote_file_reader.h -> io/cache/block_file_cache.h, an existing chain this PR does not touch. One of them did not need to. lance_record_batch_converter.h included the whole format_v2/table_reader.h for two structs, ColumnDefinition and GlobalRowIdContext, which are defined in format_v2/column_data.h, and that header reaches nothing on the chain. The converter header now includes column_data.h instead, so lance_record_batch_converter.cpp no longer reaches util/threadpool.h and the reach is 1087, within the limit. No gate table changes. The other four TUs (lance_reader.cpp, lance_runtime_filter_helper.cpp and their unit tests) evaluate conjuncts and runtime filters, so they include vexpr_context.h on purpose; lance_reader.h still gets table_reader.h directly. ### Release note None ### Check List (For Author) - Test: Manual test - build-support/check-build-hygiene.sh passes (threadpool.h reach 1087 of 1087). - With the BE UT compile flags, clang++ -fsyntax-only passes on lance_record_batch_converter.cpp, on a TU that includes only its header, and on the unity batch that holds it. - Behavior changed: No - Does this need documentation: No Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
### What problem does this PR solve? Issue Number: None Related PR: apache#69581, apache#68649 Problem Summary: Reproduction: read an Arrow Date32 array holding a day outside 0000-01-01..9999-12-31 into a Doris DATE column, for example 2932897 (10000-01-01) written by PyArrow into a Lance dataset, or the same value returned by an ADBC driver, an Arrow stream load or the paimon-rust reader. The row comes back as a non-null `0000-00-00`, also inside ARRAY, STRUCT and MAP values. Root cause: the DATE32 branch of the shared `DataTypeDateV2SerDe::read_column_from_arrow` ignores the bool returned by `get_date_from_daynr()` (dropped since apache#48944) and appends the default-constructed value. It also adds the epoch offset in 32 bits, so INT32_MAX overflows, and it decodes the payload of NULL slots. The DATE64 branch next to it already rejects out-of-range days. Fix: port the DATE32 range part of the branch-4.1/4.2 fix apache#68649. NULL slots append the default value without being decoded, the day number is computed in 64 bits, and a day number <= 0, above DATE_MAX_DAYNR or rejected by `get_date_from_daynr()` returns `InvalidArgument("Arrow Date32 value is outside the Doris DATE range: row=..., days=...")` with the 4.x wording. ARRAY/STRUCT/MAP leaves get the same check through their element SerDes. `write_column_to_arrow` and the year-zero calendar mapping (the other part of apache#68649, and master draft apache#67449) are unchanged; the new tests avoid the days whose meaning that mapping changes. Before: out-of-range days read as `0000-00-00`. After: the read fails with the error above; in-range values and NULLs read as before. The Lance reader ported in apache#69581 (from the branch-4.2 snapshot, which already contains apache#68649) relies on this SerDe check. ### Release note Reading Arrow DATE32 values outside 0000-01-01..9999-12-31 (ADBC, Arrow stream load, paimon-rust, Lance) now fails instead of returning 0000-00-00. ### Check List (For Author) - Test: Unit Test - New: `DataTypeDateTimeV2SerDeTest.RejectsInvalidDate32Values`, `LanceTableReaderTypeTest.RejectsDate32OutsideDorisDateRange`, `LanceTableReaderTypeTest.ReadsLastDorisDateAndIgnoresNullDate32Payload`. - All pass on macOS arm64 (ASAN), together with `DataTypeDateTimeV2SerDeTest.*`, `DataTypeSerDeArrowTest.*`, `DataTypeSerDeArrowValidationTest.*`, `ArrowColumnToDorisColumnTest.*`, `ParquetExprTest.*`, `IcebergV2ReaderTest.*` and the Lance suites. With master's DATE32 branch restored, the first two fail. - `DataTypeDateTimeV2SerDeTest` aborts in its setup under macOS ASAN on master: `load_data_from_csv()` in `be/test/testutil/test_util.cpp` dereferences `std::set::end()`. It ran here with that assertion removed locally; that fix goes in a separate PR. - Behavior changed: Yes. An Arrow DATE32 day outside 0000-01-01..9999-12-31 used to be returned as a non-null 0000-00-00; the scan now fails with an InvalidArgument error, as Arrow DATE64 values already did. - Does this need documentation: No Co-authored-by: Gabriel <liwenqiang@selectdb.com> Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…decoding ### What problem does this PR solve? Issue Number: None Related PR: apache#69581 Problem Summary: Reproduction: write a Lance dataset with a `pa.decimal256(18, 2)` column and scan it with the Lance reader added in apache#69581. Lance keeps the 256-bit width on disk, so the scanner returns an Arrow `Decimal256Array`, and the BE crashes on the first non-empty batch. The same happens for any `decimal256(p, s)` with p <= 38, also as a list, large_list, fixed_size_list, struct or map leaf. The existing fixture only has decimal256 columns with precision 39 and 76, which is why the tests did not catch it. Root cause: `arrow_field_to_doris_type()` and the FE pick the Doris type from the precision alone (DECIMAL32 up to 9, DECIMAL64 up to 18, DECIMAL128I up to 38), while `LanceArrowArrayNormalizer` passes the `Decimal256Array` through unchanged. `DataTypeDecimalSerDe<TYPE_DECIMAL32/64/128I>::read_column_from_arrow` casts the array with `dynamic_cast<const arrow::DecimalArray*>`, gets null for a `Decimal256Array`, and dereferences it in `Value()`. Fix: at schema binding, `LanceArrowArrayNormalizer::create()` records `decimal128(p, s)` for a Decimal256 field with p <= 38 and marks it as needing special handling, which parents inherit through the existing child loop. `normalize_for_doris()` then narrows the array with `arrow::compute::Cast(..., CastOptions::Safe())` on the query's Arrow memory pool, the cast master's ADBC normalizer already uses (`format/arrow/arrow_array_normalizer.cpp`). Nested parents rebuild their type around the narrowed child through `set_lance_nested_type()`, as for BFloat16 leaves, before compaction. A value outside the declared precision, which is invalid Arrow data (for example unscaled 10^18 in decimal(18, 2)), fails with `narrow Lance Decimal256 field '<name>' to decimal128(18, 2) failed: Decimal value does not fit in precision 18` instead of being truncated. Precision 39..76 keeps Decimal256 and DECIMAL256 as before. Before: the BE dereferences a null pointer and crashes. After: the values are read unchanged, and an out-of-precision value fails with an error that names the field. branch-4.1 and branch-4.2 have the same normalizer, so the same change applies there. ### Release note None ### Check List (For Author) - Test: Unit Test - New: `LanceTableReaderTypeTest.NarrowsDecimal256WithDecimal128PrecisionDuringNormalization`, `LanceTableReaderTypeTest.KeepsWideDecimal256UnchangedDuringNormalization`, `LanceTableReaderTypeTest.ConvertsDecimal256WithDecimal128PrecisionToDorisDecimals`, `LanceTableReaderTypeTest.NarrowsDecimal256LeavesInSlicedNestedArrays`, `LanceTableReaderTypeTest.RejectsDecimal256ValuesOutsideDeclaredPrecision`, `LanceTableReaderTypeTest.ReadsDecimal256WithDecimal128PrecisionFromLanceDataset` (writes a decimal256(18, 2) dataset, checks that Lance stores Decimal256 and that the reader returns DECIMAL64 values). - All pass on macOS arm64 (ASAN) with the Lance suites. With the narrowing disabled, the normalization cases fail and the sliced nested case then crashes in `DataTypeDecimalSerDe::read_column_from_arrow`, as reported. - Behavior changed: No (fixes a crash in the reader this PR adds) - Does this need documentation: No Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
### What problem does this PR solve? Issue Number: None Related PR: apache#69581 Problem Summary: Reproduction: scan a Lance FLOAT or DOUBLE column that holds a negative NaN (float bits 0xffc00000, double bits 0xfff8000000000000) under a runtime filter `x >= c`, for example a MIN-only filter from a nested-loop join on `x >= y`. Doris treats every NaN as the greatest value regardless of its sign (`GreaterThanEqualsFloat` in `common/compare.h`), so the row satisfies the filter, but it is missing from the result. Root cause: `build_range_filter_sql()` translates the filter into the Lance SQL `(x >= c)`. Lance evaluates it with Arrow/DataFusion comparison kernels that order floating values by IEEE 754 totalOrder, where a negative NaN sorts below every finite value, so Lance drops the row before the scanner evaluates the Doris residual. The residual can only remove rows, not bring them back. Fix: `build_range_filter_sql()` returns `std::nullopt` for a `>=` filter whose slot is FLOAT or DOUBLE, so the filter stays residual-only and is reported under `LanceRuntimeFilterSkippedIds`, the policy `PaimonRustPredicateConverter` already applies to floating predicates. `<=` filters are still pushed: Lance then only returns extra negative-NaN rows, which the residual removes. IN filters are still pushed: a NaN row never matches a finite member, and sets with NaN members were already rejected. A MIN_MAX filter now pushes only its `<=` half. Before: rows holding a negative NaN were dropped under a floating `>=` runtime filter. After: Lance returns them and the Doris residual decides, so the result matches a scan without the pushed filter. branch-4.1 and branch-4.2 carry the identical helper, so the same change applies there. ### Release note None ### Check List (For Author) - Test: Unit Test - New: `LanceRuntimeFilterHelperTest.KeepsFloatingMinFiltersResidualOnly` (`>=` skipped, `<=` and IN still pushed, SQL of a MIN_MAX pair), `LanceTableReaderFilterTest.PreservesNegativeNaNUnderFloatingMinRuntimeFilter` (FLOAT and DOUBLE rows 0.5, 1, 2, +NaN, -NaN under `>= 1`; every row Doris keeps must come back from Lance). - Both pass on macOS arm64 (ASAN) with the Lance suites; with `build_range_filter_sql()` as before this change, both fail. - Behavior changed: No (the Lance reader is added by this PR; floating `>=` runtime filters are no longer pushed into Lance, so such scans may read more rows) - Does this need documentation: No Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…file as in branch-4.x ### What problem does this PR solve? Issue Number: None Related PR: apache#69581, apache#68813 Problem Summary: apache#69581 ports the Lance reader from the branch-4.2 snapshot cabcf45. After that snapshot, apache#68813 changed two things in the 4.x BE, both ported here. 1. Search filter timing. apache#68813 added `TExternalSearchRequest.prefilter` (field 5): unset or false means the TVF search filter refines the top-k candidates (postfilter), true means it constrains them before top-k (prefilter). The 4.x FE sends it for `vector_search()` and `full_text_search()`. Without the field, a master BE always prefiltered vector search and never prefiltered full-text search, so a 4.x FE paired with it got different rows: on the all_types.lance fixture, top_k 1 with search filter `row_id >= 3` returns row 4 with prefilter and no row with postfilter. - `gensrc/thrift/PlanNodes.thrift`: `5: optional bool prefilter` with the 4.x comment; the stale `TSearchFilter` comment is replaced. - `_configure_vector_search()` and `_configure_full_text_search()` pass `request.__isset.prefilter && request.prefilter` to `lance_scanner_set_prefilter()`, replacing the comment that fragment-scoped nearest queries need prefiltering (master's lance-c pin 4fafcd64525 no longer does). - Compatibility (gensrc/AGENTS.md): field 5 is `optional bool prefilter` on the branch-4.2 (a7ae6f4) and branch-4.1 (3f911d0) heads; branch-4.0 and branch-3.1 have no `TExternalSearchRequest`. No id is reused. 2. Profile layout. The snapshot hangs about 45 counters directly under `LanceReader`, looks Lance's scan metrics up in two maps, and creates two scalar-index timers from Lance's statistics callback the first time they are reported, that is from Lance's execution threads while the scan runs. As in apache#68813: - The counters are grouped under `LanceTiming`, `LanceIO`, `LanceIndex`, `LanceVectorIndex`, `LanceScalarIndex`, `LanceExecutionPlan`, `LanceScan`, `LanceSearchPlan` and `LanceDataCache` (label counters, `TUnit::NONE`, like `RuntimeFilterInfo`). - One immutable table, `LANCE_SCAN_METRICS` in `lance_reader_helper.h`, drives both registration and the callback lookup. Every metric is registered in `_init_scanner_profile()` before the callback is installed, so the callback only updates existing counters and every split has the same counter tree. - The callback is `noexcept` and swallows exceptions, so nothing unwinds through the lance-c ABI. - `LanceIndexObjectLoads` (`indices_loaded`), `LanceIndexCacheHits` and `LanceIndexCacheMisses` are new; the info strings `LanceTopKPlusOffset` and `LanceSubstraitFilterBytes` are dropped. - Renames: `LanceScannerReadTime` -> `LanceScannerNextTime`, `LanceRuntimeFilterSqlTime` -> `LanceRuntimeFilterSqlTranslationTime`, `LanceExecutionIOOps` / `IORequests` / `IOBytesRead` -> `LanceIOReadOps` / `LanceIOReadRequests` / `LanceIOReadBytes`, `LanceIndexPartitionCacheMissLoads` -> `LanceIndexComponentsLoaded`, `LancePrefilter*` -> `RowIdPrefilter*` (`LancePrefilterRowIds` -> `RowIdPrefilterIds`), `LanceIVFPartitionsRanked` -> `LanceIVFPartitionsConsidered`, `LanceIVFPartitionsSearched` -> `LanceIndexPartitionsSearched`, `LanceVectorIndexSegmentsSearched` -> `LanceVectorIndexDeltasSearched`, `LanceTaskWaitTime` -> `LanceScanTaskWaitTime`, `LanceIVFPartitionRankingTime` -> `LanceIVFPartitionSelectionTime`, `LanceScalarIndexSegmentPrepareTime` -> `LanceScalarIndexCandidateEvaluationTime`. - Master differs from 4.x in one line: the callback asserts with `DCHECK_LT` that a found metric has a registered counter, where 4.x checks it in the `if`; both come from the same array. - Not ported: apache#68813's FE change that makes `AggCounter` print nothing for `TUnit.NONE`. It belongs to the FE port; until then master's merged profile prints the group labels like the existing `RuntimeFilterInfo`. ### Release note None ### Check List (For Author) - Test: Unit Test - New: `LanceTableReaderVectorSearchTest.SearchFilterDefaultsToPostfilter` (from apache#68813) and `LanceTableReaderFullTextSearchTest.SearchFilterHonorsPrefilterFlag` (top_k 1 with filter `row_id <= 2`: no row with postfilter, row 2 with prefilter; 4.x has no BE test for the full-text path). - Changed as in apache#68813: `AppliesSearchFilterBeforeTopK` becomes `AppliesSearchFilterBeforeTopKWhenPrefilterEnabled`; `MultiVectorScoresFiltersOffsetsAndIndexedCoverage` and `MultiVectorSegmentTopKMatchesIndependentGlobalOracle` set `prefilter` for their filtered cases, and the former checks the `RowIdPrefilter*` counters and that a flat split exposes `LanceIndexPartitionLoadTime`. - All pass on macOS arm64 (ASAN) against master's pinned lance-c, with the Lance suites. With vector search always prefiltering and full-text search never prefiltering, the two new cases fail. - Behavior changed: Yes, on the Lance path added by apache#69581. A search filter without `prefilter` now postfilters vector search, and full-text search honors `prefilter = true`, as on branch-4.1/4.2. Lance profile counters are grouped and renamed. Master has no FE that plans Lance scans yet. - Does this need documentation: No Co-authored-by: zhangstar333 <zhangsida@selectdb.com> Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
… planned ### What problem does this PR solve? Issue Number: None Related PR: apache#69581, apache#68707, apache#68815 Problem Summary: apache#69581 ports the Lance reader from the branch-4.2 snapshot cabcf45, where `_configure_vector_search()` passes the request's `use_index` to `lance_scanner_set_use_index()`. FE decides per split whether to use an index: a split it plans with index segments carries their UUIDs, and a split without them is meant to be searched flat. With `use_index = true` and no segments, Lance instead searches the split with the first index on the column in manifest order. That index need not be the one FE chose: when FE finds no index whose metric matches the query, it plans every split flat, and the snapshot BE still searched them with the mismatched index. On branch-4.2, apache#68815 also plans distance-range searches over quantized indexes (IVF_SQ) as flat fallback splits, because quantized distances can drop rows inside the range; the snapshot BE searches those with the index and returns too few rows. Fix: port the split-level rule of apache#68707. - `lance_scanner_set_use_index(scanner, segment_count > 0)`: a split with segments searches exactly those, a split without segments searches flat, and `LancePlannedFlatSearchFragmentCount` now matches what Lance does. - A split that carries index segments while the request sets `use_index = false` is rejected with InvalidArgument. FE plans segments only when `use_index` allows them, on branch-4.1 (4.1.4 onward) and branch-4.2 alike, so a 4.x FE never sends this combination. - Not ported: the rest of apache#68707 (searching historical versions, tags and branches; rejecting fragment ids missing from the snapshot; naming the snapshot in errors and the profile). That comes with the next 4.x catch-up. ### Release note None ### Check List (For Author) - Test: Unit Test - New, from apache#68707 (only master's `RuntimeState(TQueryOptions(), globals)` constructor differs): `LanceTableReaderVectorSearchTest.SplitWithoutIndexSegmentsSearchesFlat`, `LanceTableReaderVectorSearchTest.RejectsIndexSegmentsWhenUseIndexIsFalse`. - Changed as in apache#68707: `MultiVectorScoresFiltersOffsetsAndIndexedCoverage`, `MultiVectorTopOnePreservesPrecisionAndBatchIndependence` and `MultiVectorCosineMasksUndefinedRows` pass the index segments for their indexed cases (new helper `make_indexed_lance_range`) and check `LanceIndexPartitionsSearched`; `MultiVectorDefaultMetricIsConsistentAcrossFragments` checks that every split is searched flat. - All pass on macOS arm64 (ASAN) against master's pinned lance-c, with the Lance suites. With `use_index` taken from the request and the rejection removed, the two new cases fail. - Behavior changed: Yes, on the Lance path added by apache#69581: a vector split without index segments is searched flat even when `use_index` is true, and a split with segments under `use_index = false` is rejected. Master has no FE that plans Lance scans yet. - Does this need documentation: No Co-authored-by: zy-kkk <zhongyk10@gmail.com> Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
### What problem does this PR solve? Issue Number: None Related PR: apache#69581, apache#68815 Problem Summary: apache#69581 ports the Lance reader from the branch-4.2 snapshot cabcf45. After that snapshot, apache#68815 added `TVectorSearchParams.distance_lower_bound` (6) and `distance_upper_bound` (7) to branch-4.1 and branch-4.2: FLOAT bounds, inclusive lower and exclusive upper, which the 4.x FE fills for a single-vector `vector_search()` with distance bounds. apache#68815 kept the external-search `schema_version` at 1, so a BE cannot tell a bounded request from an unbounded one by version. A master BE without the fields ignored them and returned the unbounded top-k rows, for example rows with `_distance` below the lower bound. Fix: port the BE side of apache#68815. - `gensrc/thrift/PlanNodes.thrift`: add `6: optional double distance_lower_bound` and `7: optional double distance_upper_bound` to `TVectorSearchParams` with the 4.x comment. - `LanceTableReader::_validate_external_search_request()` rejects bounds on a multi-vector query (NotSupported), a bound that is not a finite FLOAT value (NaN, infinity, or beyond FLT_MAX), and a lower bound that is not below the upper bound at FLOAT precision (InvalidArgument), before the dataset is opened. - `_configure_vector_search()` narrows the bounds to FLOAT and installs them with `lance_scanner_set_distance_range()` after `lance_scanner_nearest()`, which clears any earlier range. A missing bound is passed as NULL (unbounded on that side). Requests without bounds are unchanged. Before: distance bounds sent by a 4.x FE were silently ignored. After: the reader returns only rows with lower <= _distance < upper, still capped by top_k and offset, and rejects invalid bounds. Compatibility (gensrc/AGENTS.md): fields 6 and 7 of `TVectorSearchParams` are `optional double distance_lower_bound` and `optional double distance_upper_bound` on the branch-4.2 (a7ae6f4) and branch-4.1 (3f911d0) heads, the same ids and types as here. branch-4.0 and branch-3.1 have no `TVectorSearchParams`, and on master the struct is new in apache#69581 with fields 1-5. No id is reused, and both fields are optional. `lance_scanner_set_distance_range()` is in master's lance-c pin (4fafcd64525), so only the BE changes. ### Release note None ### Check List (For Author) - Test: Unit Test - New (copied unchanged from apache#68815 except for master's `RuntimeState(TQueryOptions(), globals)` constructor): `LanceTableReaderVectorSearchTest.ValidatesDistanceRangeBeforeDatasetAccess`, `LanceTableReaderVectorSearchTest.AppliesDistanceBoundsAndOffset`, `LanceTableReaderVectorSearchTest.DistanceRangeFallbackPreservesExactScores`, `LanceTableReaderVectorSearchTest.DistanceRangeCoversIndexedAndAppendedFragments`. - Fixtures copied from d306323 (identical on the branch-4.2 head): `be/test/format_v2/table/lance/data/distance_range.lance` (11 files), `distance_range_dot.lance` (8 files), `distance_range_sq.lance` (8 files) and their description `be/test/format_v2/table/lance/distance_range_fixture.md`. - All pass on macOS arm64 (ASAN) against master's pinned lance-c, with the Lance suites. `DistanceRangeFallbackPreservesExactScores` needs the previous commit: without it the IVF_SQ fallback split is searched with the quantized index and loses its only row. With the range not installed, the three cases that read rows fail. - Behavior changed: Yes, on the Lance external-search path added by apache#69581: distance bounds are applied instead of ignored, and invalid bounds are rejected. Master has no FE that sends Lance search requests yet. - Does this need documentation: No Co-authored-by: Gabriel <liwenqiang@selectdb.com> Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
…Lance reader ### What problem does this PR solve? Issue Number: None Related PR: apache#69581 Problem Summary: BE configure runs `build-support/check-build-hygiene.sh`. Its reverse reach budget caps how many TUs may transitively include `util/threadpool.h` at the baseline 988 (master 9a48f81, 2026-08-18) plus 10%, that is 1087. Master 76fb880 is at 1084, and the Lance reader adds four TUs that reach it (`format_v2/table/lance_reader.cpp`, `format_v2/lance/lance_runtime_filter_helper.cpp` and their unit tests), so configure on master plus this PR fails with "util/threadpool.h now reaches 1088 TUs (baseline 988 +10% = 1087)". The four TUs add no new edge. They need `VExprContext`, directly or through `format_v2/table_reader.h`, and `exprs/vexpr_context.h` holds `segment_v2::ColumnIteratorOptions` by value, so it includes `storage/segment/column_reader.h` -> `io/cache/cached_remote_file_reader.h` -> `io/cache/block_file_cache.h` -> `util/threadpool.h`, the path the other format_v2 readers already take. Narrowing includes inside the Lance files cannot remove it; the record batch converter header was already narrowed to `format_v2/column_data.h` in this PR. As the gate's message prescribes, this rebaselines `util/threadpool.h` to the measured 1088 (limit 1197) and records the reason next to the entry. Cutting the `vexpr_context.h` -> `column_reader.h` edge, by moving `ColumnIteratorOptions` into a light header, would bring the reach down to about 846; that is a separate build cleanup. ### Release note None ### Check List (For Author) - Test: No need to test: build configuration only. `bash build-support/check-build-hygiene.sh` passes on this branch and on master 76fb880 plus this PR. - Behavior changed: No - Does this need documentation: No Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
|
run buildall |
|
/review |
There was a problem hiding this comment.
Two new P2 findings remain: out-of-range Lance timestamps can silently wrap their year, and persistent native session caches have no Doris tracking/reclamation bridge. Both are reported inline.
The five existing findings were independently rechecked at a1c3636: Decimal256 narrowing (4369909216), search prefilter timing (4369909218), vector distance bounds (4369909220), negative-NaN GE filters (4369909223), and DATE32 range checking (4369909226) are fixed. No existing P0/P1 finding remains applicable, so none is reposted or carried as blocking. The new timestamp finding follows a separate TIMESTAMP/DATETIMEV2 conversion path from the repaired DATE32 issue. There was no additional user-provided focus.
Critical checkpoint conclusions:
- Purpose and proof: the native Lance BE scan/schema/search/fetch port is connected through FileScannerV2. Extensive unit-test code is present; timestamp boundaries and persistent-cache reclamation still need coverage for the inline defects. Tests were inspected, not executed.
- Scope and reuse: the reader, conversion, filter and session helpers form a coherent BE port; shared Null/DATE32 SerDes and row-ID width changes have corresponding callers and tests.
- Concurrency and locks: parallel scanners share the call_once-initialized session; callbacks update pre-registered counters. Phase-two tasks retain per-task outputs and completion ownership. No additional concrete race or lock inversion was found.
- Lifecycle/static initialization: scanner cleanup precedes FTS context/dataset cleanup; partial-open guards and schema resets preserve ownership. The function-local session and synchronized metric-hook teardown avoid an established static-lifetime defect; cache retention is covered inline.
- Configuration: six cache settings are process-scoped; the three mutable buffer/readahead settings are sampled for newly created scanners. Existing scanners are not promised immediate updates.
- Compatibility: the 24-byte version-0 layout and original uint32 row-ID values are preserved; version 1 carries native uint64 Lance IDs, and current receivers reject unknown versions. Old receivers cannot decode version 1: the explicitly deferred FE rollout gate is still needed before enabling mixed-version Lance lazy materialization.
- Thrift/protobuf: every touched definition was compared with live-verified branch-3.1/4.0/4.1/4.2 snapshots. Existing IDs, enum meanings and optional search fields match 4.1/4.2; older branches lack the new search definitions. Legacy protobuf row IDs retain their wire meaning and range.
- Parallel paths: normal/vector/FTS, schema-only, nested types, row-ID take/scatter and legacy Parquet/ORC paths were checked. FE split requests reset search offset after increasing candidate top_k, explaining the different split-level offset handling.
- Conditions and errors: FFI failures, malformed versions/counts and unsupported filter/type cases return errors or retain residual evaluation. Lance's pinned planner repairs signed-zero comparisons; raw DataFusion behavior alone is not a defect here.
- Test coverage: positive/negative tests cover types, NULLs, slicing, indexes, search bounds, prefilter and row-ID validation. Full scanner/metadata-count/RPC integration, cancellation, broader writer interoperability and live resource behavior were not established by execution.
- Expected results: no regression .out files changed. Binary fixture inventories and their test expectations were inspected; their runtime output was not independently reproduced.
- Observability: scan, index, cache, search and fetch metrics/timers are present; metric gauges do not provide the missing cache reclamation hook.
- Transactions/persistence: no Doris EditLog or transaction-state mutation is introduced; snapshot identity is carried into phase-two fetch.
- Writes/atomicity: the production change is a read path. No new Doris write/commit protocol or committed-row visibility defect was identified.
- FE/BE passing: search fields and split contracts match current 4.x senders; the master FE connector/rollout policy is outside this BE-only port.
- Performance/memory: session/schema reuse, zero-copy common arrays and deduplicated snapshot-level take_rows avoid redundant work. Persistent cache tracking/reclamation remains an inline issue. No benchmark or memory-pressure experiment was run.
- Further sweep: adjacent timestamp conversion, complete upstream floating-filter handling, real Rust Arrow export layouts, RF cache scope, recursive fragment rebuilds, native take ordering and cleanup were rechecked. No other verified finding remains.
Static review only: no builds, tests, source modifications or submodule initialization were performed. Existing glibc compatibility adapters/link ordering were inspected, but no completed Linux ELF artifacts were available to verify the GLIBC_2.17 release baseline. These execution/artifact and deferred-FE limits are not claimed as validated behavior. All code-review candidates and risk focuses have a recorded disposition; no unresolved candidate is being submitted as a finding.
Review converged after 2 round(s).
| RETURN_IF_ERROR( | ||
| _array_normalizer_for_arrow_column[static_cast<size_t>(arrow_index)] | ||
| .normalize_for_doris(arrow_column, _memory_pool, &normalized_column)); | ||
| RETURN_IF_ERROR(output.serde->read_column_from_arrow( |
There was a problem hiding this comment.
[P2] Reject out-of-range timestamps before narrowing the year
A valid Lance timestamp(us) value 2005980768000000000 represents 65537-01-01 00:00:00, but this call returns non-null 0001-01-01. The schema mapper selects DATETIMEV2(6), and the shared timestamp SerDe calls from_unixtime(), which passes the civil year to unchecked_set_time(uint16_t year, ...), narrowing 65537 to 1. transform_date_scale(6, 6) only copies, and NullableSerDe retains the source's non-null bit. The repaired DATE32 check is on another conversion path and cannot catch this, including nested timestamp leaves. Validate the calendar range before narrowing and add first-out-of-range and year-wrap timestamp cases.
There was a problem hiding this comment.
Confirmed, and it is not specific to Lance: DataTypeDateTimeV2SerDe::read_column_from_arrow packs the civil year from from_unixtime() into unchecked_set_time(uint16_t year, ...) without a range check, so ADBC, Arrow stream load, paimon-rust and remote Doris reads return the same 0001-01-01 for 2005980768000000000 us, and years 10000..65535 come back as non-null values outside the DATETIME range. The TIMESTAMPTZ Arrow path and the UTC-adjusted Parquet paths call is_valid_date() only after packing, which a wrapped year passes. branch-4.1 and branch-4.2 have the same code. Unlike the DATE32 check in this PR, which ports a fix branch-4.1 / 4.2 already have (#68649), this one is missing on every branch, and the change belongs in the shared SerDe used by all of those readers. It goes in a separate PR that checks the civil year before narrowing, as the ORC path does since #68516, for the Arrow and Parquet timestamp paths, with first-out-of-range, year-wrap, pre-0000 and nested cases. This PR does not touch that SerDe.
| .read_block_size_bytes = | ||
| static_cast<uint64_t>(_config.lance_data_cache_read_block_size_bytes), | ||
| }; | ||
| _session = lance_session_new_with_data_cache( |
There was a problem hiding this comment.
[P2] Make persistent session caches trackable and reclaimable
This singleton gives the native index and metadata caches 10 GiB and 1 GiB by default, but neither cache is charged to a Doris MemTracker or registered with CacheManager. The pinned native session retains these caches after dataset/query teardown; publishing size gauges does not add a reclaim path. Once indexed reads populate them, Doris's memory-pressure capacity adjustment and /api/clear_cache/all only visit registered CachePolicy objects, leaving these idle entries live even while other queries encounter process memory pressure. Disabling the Foyer data cache does not disable these two caches. Add process-cache accounting and a CacheManager eviction/capacity bridge, and cover reclamation after the owning queries finish.
There was a problem hiding this comment.
Confirmed; this is the branch-4.1 / 4.2 design, ported unchanged. The index and metadata caches belong to the lance-c session, which a BE creates on its first Lance scan with the configured capacities (lance_index_cache_size_bytes, lance_metadata_cache_size_bytes, applied at restart); enable_lance_data_cache = false does not affect them, and the lance_session_* metrics report their usage. The pinned lance-c exposes only lance_session_get_cache_stats(). It has no call to clear, evict or resize either cache, so a CachePolicy could neither prune them nor follow the memory-pressure capacity adjustment. Making them reclaimable needs that API in lance-c first and should land on all branches together, so it is tracked as a follow-up. The PR description now says that these caches are not charged to a MemTracker or managed by CacheManager, and how to size them.
Cloud UT Coverage ReportIncrement line coverage Increment coverage report
|
TPC-H: Total hot run time: 28563 ms |
TPC-DS: Total hot run time: 150868 ms |
FE UT Coverage ReportIncrement line coverage `` 🎉 |
ClickBench: Total hot run time: 25.15 s |
FE Regression Coverage ReportIncrement line coverage |
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
a1c3636 to
a19d360
Compare
…er.h ### What problem does this PR solve? Issue Number: None Related PR: apache#69581 Problem Summary: BE configure fails the build-hygiene gate on this PR (COMPILE and BE UT): util/threadpool.h now reaches 1088 TUs, one over its limit of 1087 (baseline 988 +10%). Master reaches 1083. Five of the new Lance TUs reach the header through exprs/vexpr_context.h -> storage/segment/column_reader.h -> io/cache/cached_remote_file_reader.h -> io/cache/block_file_cache.h, an existing chain this PR does not touch. One of them did not need to. lance_record_batch_converter.h included the whole format_v2/table_reader.h for two structs, ColumnDefinition and GlobalRowIdContext, which are defined in format_v2/column_data.h, and that header reaches nothing on the chain. The converter header now includes column_data.h instead, so lance_record_batch_converter.cpp no longer reaches util/threadpool.h and the reach is 1087, within the limit. No gate table changes. The other four TUs (lance_reader.cpp, lance_runtime_filter_helper.cpp and their unit tests) evaluate conjuncts and runtime filters, so they include vexpr_context.h on purpose; lance_reader.h still gets table_reader.h directly. ### Release note None ### Check List (For Author) - Test: Manual test - build-support/check-build-hygiene.sh passes (threadpool.h reach 1087 of 1087). - With the BE UT compile flags, clang++ -fsyntax-only passes on lance_record_batch_converter.cpp, on a TU that includes only its header, and on the unity batch that holds it. - Behavior changed: No - Does this need documentation: No Co-Authored-By: Claude Opus 5.5 (1M context) <noreply@anthropic.com>
TPC-DS: Total hot run time: 151354 ms |
ClickBench: Total hot run time: 25.22 s |
|
Addressed the fourth review round in
|
|
run buildall |
|
/review |
There was a problem hiding this comment.
Request changes. One new P2 finding is inline: DATE32 conversion rejects a child value hidden beneath a NULL fixed-size-list parent, making a supported valid Lance scan fail. This is a different input and failure from the repaired visible-DATE32 range finding.
The existing P1 shared data-cache identity finding still applies to f5af56058e79a80e163627bcc9b58642db3e602a. I independently traced the default-enabled singleton session into the Foyer patch: keys include the Lance store prefix and object path, but omit the physical endpoint. Two origins with the same bucket/path and different registered immutable data-file bytes can return the other origin's cached rows. Random writer filenames reduce this trigger; they do not exclude it. Documentation and a deferred fix do not remove the defect. Retain comment 4373822200; no duplicate inline is posted.
One new independent finding was accepted. Existing timestamp year-wrap P2 4370665038 and native cache accounting/reclamation P2 4370665039 also remain applicable and were deduplicated. The previous Decimal256-carrier, search-prefilter, distance-bound, negative-NaN, nested-offset and zero-field-struct findings are repaired on this head; their focused test assertions were inspected.
Critical checkpoint conclusions:
| Checkpoint | Conclusion |
|---|---|
| Goal and proof | The BE scan, schema-fetch, ordinary/vector/FTS and lazy row-fetch paths are connected. Focused BE tests cover their main contracts. The FE connector and regression suites are separate work, so full master FE-to-BE integration is not established here. |
| Scope and clarity | The substantial port is organized into reader, schema/normalization, conversion, runtime-filter and session helpers. Shared Arrow and row-location changes were reviewed beyond Lance. |
| Concurrency and locks | Session initialization uses call_once; metric hooks/deregistration share entity locking. Runtime-filter SQL snapshots are immutable and native callback counters are registered before use. No additional concrete lock inversion or race was identified by static inspection. |
| Lifetime and static initialization | Scanner/stream callbacks retire before dataset/FTS context teardown; native and Arrow handles have cleanup paths. No new cross-TU initializer dependency was identified. Native cache lifetime/accounting remains the known P2. |
| Configuration | Six session/cache settings are restart scoped; I/O buffer and two readahead settings are mutable, positive-validated and sampled when a new scanner opens. |
| ABI and rolling upgrades | Version-zero row locations retain their 24-byte layout and uint32 domain. Version-one IDs retain uint64 through materialization/RPC/take_rows; legacy readers retain their adapters. Enabling Lance lazy materialization with old BEs still requires the separately scoped FE rollout guard. |
| Thrift/protobuf IDs | Touched definitions were compared with current branch-3.1/4.0/4.1/4.2 refs. Search IDs, optional fields and enum values match the releases that define them; uint32-to-uint64 row_id retains varint encoding and existing values. No new collision or required field was found. |
| Parallel paths | Ordinary/scalar-index/uncovered-fragment, flat/indexed vector, multi-vector, FTS MATCH/PHRASE, pre/postfilter, schema discovery and phase-two paths were checked, including legacy Parquet/ORC/internal row IDs. |
| Special conditions | Concrete cases support the unsupported-type, NaN, slice, precision, vector-shape, version and count guards. Signed-zero pushdown is corrected by the pinned Lance optimizer. No additional verified predicate-equivalence defect was found. |
| Test coverage | Inspected negative/boundary, null/nested, split/batch, EOF, search-oracle, type/carrier, invalid-request and uint64/order/duplicate tests. NULL fixed-size-list DATE32 payloads lack a regression test for the new finding. Direct Lance count/cancellation integration and local signed-zero/subnormal differential cases also remain coverage gaps. |
| Expected results and fixtures | Assertions and fixture references/provenance were inspected; no .out result changes. Binary fixture decoding and the author's reported passes were not independently executed. |
| Observability | Reader/search/native counters, pushed/skipped RF IDs, cache metrics and phase-two timings are present. Native execution summaries are EOF-based; live per-batch native metrics were not established. |
| Transactions and persistence | No Doris transaction, EditLog, MoW or persistence-write changes. Dataset snapshot identity is preserved in the examined fixed-version read/fetch paths. |
| Data writes and failure atomicity | Production changes are read paths; native handle cleanup and RPC error propagation were inspected. No new Doris write/commit atomicity path applies. |
| FE-to-BE values | Search kind, vector byte layout/options, timing flag, distance bounds, split segments, snapshot and row-location version were traced. Full FE rollout behavior remains outside this BE-only port. |
| Performance and build | Schema binding, SQL caching and ordinary-array zero-copy paths avoid repeated setup. Cache resource risks are already reported. The IOKit addition and documented reach-budget rebaseline were inspected. No benchmark or completed ELF GLIBC_2.17 audit was performed. |
| Other issues and user focus | No additional user focus was supplied. All assigned candidates and risk questions have explicit ledger dispositions; the NULL fixed-size-list DATE32 error is the only accepted new finding. |
Validation was static only: no builds, tests, benchmarks, source edits or submodule initialization. Runtime races, remote-I/O cancellation latency, complete binary-fixture decoding and final Linux artifact ABI are not validated by this review. Review convergence concerns coverage and candidate disposition; the existing P1 still requires changes.
Review converged after 3 round(s).
Existing P0/P1 findings confirmed for this head: #69581 (comment)
| const int64_t daynr = static_cast<int64_t>(date_value) + date_threshold; | ||
| DateV2Value<DateV2ValueType> v; | ||
| v.get_date_from_daynr(date_value + date_threshold); | ||
| if (daynr <= 0 || daynr > DATE_MAX_DAYNR || |
There was a problem hiding this comment.
[P2] Ignore DATE32 payloads hidden by null fixed-size lists
A valid Lance v2.1 FIXED_SIZE_LIST<DATE32,1> using lance-encoding:structural-encoding=miniblock can have parent validity [true,false,true] and non-null child days [0,2932897,1]. Its visible result is [1970-01-01], NULL, [1970-01-02]. The pinned writer retains the child data, and the decoder restores only outer validity. This compact array passes normalization unchanged; NullableSerDe records the parent NULL, but ArraySerDe still decodes all three children. The middle child itself is non-null, so this new check rejects its hidden 10000-01-01 payload and fails the scan. Propagate parent nulls into child conversion or skip hidden spans, while retaining rejection for visible invalid dates; add a native mixed-null fixed-list regression. This is distinct from resolved comment 4369909226, which concerned visible out-of-range dates.
There was a problem hiding this comment.
Confirmed, and fixed in a5e1ead1de1. Arrow leaves the child values under a null list, fixed-size-list, map or struct row unspecified, and the array SerDe passed the whole child range to the element SerDe, which only skipped the child's own nulls. The DATE32 range check that this PR adds therefore rejected a hidden day such as 2932897 and failed a batch whose visible values are all valid. The array, map and struct SerDes now hand their children a validity bitmap that also clears the slots hidden by a null parent (mask_arrow_children_hidden_by_null_parents()), so the element SerDe neither validates nor decodes them; the parent's null map hides them as before. This covers DATE32, DATE64 and any other element check; only the validity buffer is replaced, and nothing is allocated when no parent row in the range is null. LanceTableReaderTypeTest.IgnoresDate32PayloadHiddenByNullParents reads your FIXED_SIZE_LIST<DATE32, 1> case, a LIST with a non-empty null segment, a STRUCT, a MAP and a LIST, and fails without the change; ArrowParentNullsTest covers the helper. branch-4.1 and branch-4.2 have the same check and SerDes.
TPC-H: Total hot run time: 28364 ms |
Cloud UT Coverage ReportIncrement line coverage Increment coverage report
|
TPC-DS: Total hot run time: 150842 ms |
ClickBench: Total hot run time: 24.26 s |
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
FE Regression Coverage ReportIncrement line coverage |
… rows ### What problem does this PR solve? Issue Number: None Related PR: apache#69581 Problem Summary: Arrow leaves the child values under a NULL list, fixed-size-list, map or struct row unspecified. A FIXED_SIZE_LIST<DATE32, 1> with validity [1, 0, 1] may hold the days [0, 2932897, 1]; its rows are [1970-01-01], NULL, [1970-01-02]. Lance's v2.1 miniblock encoding keeps such child data, and other Arrow producers (ADBC drivers, Arrow stream load, paimon-rust) may as well. Root cause: the array, map and struct SerDes pass the whole child range to the element SerDe, which skips only the child's own NULLs. The DATE32 range check added by this PR therefore rejected 2932897 (10000-01-01) and failed the batch, although no visible value is out of range. Fix: before reading a child, the nested SerDes hand it a validity bitmap that also clears the slots hidden by a NULL parent row (`mask_arrow_children_hidden_by_null_parents()`), so the element SerDe neither validates nor decodes them; the parent's null map hides them as before. Only the validity buffer is replaced, and nothing is allocated when no parent row in the range is NULL. This covers DATE32, DATE64 and any other check an element SerDe makes. Before: such a batch failed with "Arrow Date32 value is outside the Doris DATE range". After: it reads as [1970-01-01], NULL, [1970-01-02]. branch-4.1 and branch-4.2 have the same DATE32 check and nested SerDes. ### Release note None ### Check List (For Author) - Test: Unit Test - New: `ArrowParentNullsTest` (without a NULL parent the child itself is returned; only the slots of NULL parents are cleared and the child keeps its own NULLs; the field of a sliced struct is addressed with its offset) and `LanceTableReaderTypeTest.IgnoresDate32PayloadHiddenByNullParents` (FIXED_SIZE_LIST, LIST with a non-empty NULL segment, STRUCT, MAP and LIST<STRUCT> over a hidden 10000-01-01). - `ArrowParentNullsTest.*`, `DataTypeSerDeArrowTest.*`, `DataTypeSerDeArrowValidationTest.*`, `ArrowColumnToDorisColumnTest.*`, the Lance and ADBC suites and `IcebergV2ReaderTest.*` on macOS arm64 (ASAN): 297 pass, 7 ADBC SQLite cases skip (no SQLite driver here). With the three SerDe changes reverted, the Lance test fails with "Arrow Date32 value is outside the Doris DATE range: row=1, days=2932897". - Behavior changed: No (visible values are unchanged; the check that hidden values tripped is added by this PR) - Does this need documentation: No Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Addressed the fifth review round in
|
|
run buildall |
|
/review |
There was a problem hiding this comment.
Request changes. Two new P2 findings concern NULL-parent Arrow conversion and query memory accounting. Existing P1 4373822200 still applies: the default shared data-cache key omits physical endpoint identity, so independent origins with different bytes at the same bucket/object path can share data. It is retained without a duplicate inline comment.
Applicable critical checkpoints:
- Goal and correctness: the BE port connects FORMAT_LANCE to FileScannerV2, schema discovery, normal/vector/FTS scans and phase-two row fetch. Hidden Decimal256 children can still fail before the new NULL-parent mask; the shared validity-copy allocation also bypasses query accounting.
- Scope and clarity: the changes form a coherent BE reader/protocol port plus shared Arrow fixes. FE integration is explicitly deferred.
- Concurrency and locking: reviewed parallel readers, shared session initialization, metrics hooks, native statistics callbacks and fetch-task completion. Hook deregistration shares the metrics mutex; scanner counters are registered before callback installation. No additional locking defect substantiated.
- Lifecycle and static initialization: readers close scanner/FTS context/dataset in order; shared initialization is lazy and call_once retains initialization failures. Persistent native index/metadata caches remain outside Doris reclamation, as already reported in P2 4370665039.
- Configuration: six session/data-cache settings apply at restart; the three mutable I/O/readahead settings are read when new scanners are configured. Their distinction is documented in code.
- ABI and rolling upgrades: version-zero row locations retain their 24-byte layout, version/size dispatch rejects unknown encodings, and uint32 file-local IDs remain representable in the widened varint field. The FE rolling-upgrade guard is outside this BE-only PR; complete rolling-upgrade behavior was not executed.
- Thrift/protobuf compatibility: compared touched definitions against branch-3.1, 4.0, 4.1 and 4.2. New search fields are optional; IDs, enum meanings and types agree where present. No new collision found.
- Parallel paths: traced shared ARRAY/MAP/STRUCT/DATE/nullable SerDes and their other Arrow consumers, normal versus search scans, vector versus FTS, and existing internal/Parquet/ORC row locations. Both new findings have effects beyond their local helper.
- Conditions and error handling: malformed requests and unsupported types fail explicitly. The hidden-child Decimal256 check runs before ancestor visibility is propagated; it needs the same visibility discipline as downstream SerDes.
- Functional and negative tests: inspected broad unit coverage for types, slices, NULLs, unsupported fields, malformed requests, native search options and row IDs. The two new cases need focused regressions. Full FE integration/regression execution is deferred by the PR and was not validated here.
- Expected test results: inspected fixture documentation, JSON version hints, assertions and expected conversions/search outcomes; binary fixtures were not decoded or executed in this static review.
- Observability: scan/search/cache counters and process session metrics are present; phase-two counters preserve units. Metrics do not provide native-cache reclamation or account for the new Arrow mask allocation.
- Transactions and persistence: no Doris transaction/EditLog/write protocol changes; snapshot selection and fixed-version row fetch were traced. Persistent cache identity remains the existing blocker.
- Writes, crashes and cleanup: reviewed query cleanup, cancellation, errors and fetch-task completion; no new database-write atomicity change. Native cache files are process resources with the known cache limits above.
- FE/BE propagation: checked release FE producers for search fields, fixed snapshots, offset ownership and lazy-fetch eligibility. Current master FE Lance support is not part of this PR.
- Performance and memory: ordinary compact Arrow arrays retain their no-copy path, and per-scanner I/O/readahead is bounded by configuration. Large child-validity copies must use tracked allocation; native persistent-cache accounting remains an existing issue. No benchmarks were run.
- Other concerns and user focus: no additional user focus was supplied. Signed-zero filtering and native JSON representation suspicions were dismissed through pinned native planner/conversion guarantees. Distributed provider postfiltering matches the published per-split execution contract and is not reported.
Static-only review: no builds, tests, benchmarks, source modifications or submodule initialization. Final Linux ELF/glibc compatibility, binary fixture execution and the deferred FE integration were not validated. Existing Decimal256 carrier, prefilter flag, distance-bound, negative-NaN, DATE32, descendant-offset and empty-struct fixes were rechecked and are not reposted.
Review converged after 2 round(s).
Existing P0/P1 findings confirmed for this head: #69581 (comment)
| auto child_array = arrow::MakeArray(child_data[child_idx]); | ||
| std::shared_ptr<arrow::Array> normalized_child; | ||
| RETURN_IF_ERROR( | ||
| child_normalizer.normalize_for_doris(child_array, memory_pool, &normalized_child)); |
There was a problem hiding this comment.
[P2] Mask NULL ancestors before narrowing Decimal256 children
A Lance v2.1 miniblock FIXED_SIZE_LIST<decimal256(18,2),1> can retain non-null child payloads [1, 10^18, 2] under parent validity [true,false,true]; its visible rows are [0.01], NULL, [0.02]. The pinned native encoder preserves those child bytes and restores only outer validity. This recursion passes the raw Decimal256 child to the safe Decimal128 cast before ArraySerDe applies the new parent mask, so the hidden middle payload fails with "Decimal value does not fit in precision 18" and aborts a scan whose visible values are valid. Propagate ancestor visibility before validating special-child conversions, and add a native mixed-null miniblock regression. This is distinct from the repaired DATE32 thread: the failure occurs in the Lance normalizer before that SerDe fix runs.
There was a problem hiding this comment.
Confirmed, and fixed in 1c74dbab92c. The Lance normalizer narrows a nested Decimal256 leaf by casting the whole raw child array before any SerDe runs, so the safe cast checked the values under a NULL list, fixed-size-list, map or struct row and, for a sliced parent, the values of rows outside the slice; the SerDe-level masking from the previous round only applies after normalization. Before narrowing, the normalizer now marks NULL every child slot that no non-NULL row of the parent references, level by level down to the leaf, so only values a reader can see are checked, and a visible value outside the declared precision is still rejected with the field name. A map is masked through its values: Arrow allows no NULL in a map's entries struct or keys (MapArray::SetData() aborts on one), so a Decimal256 key under a NULL map row is still checked. BFloat16 widening and extension unwrapping never inspect a value, so only the subtrees that narrow a decimal are masked. Covered by LanceTableReaderTypeTest.NarrowsDecimal256PayloadsHiddenByNullParents (NULL fixed-size-list, list, struct, map and list-of-struct rows, and a sliced list) and by a nested visible value added to LanceTableReaderTypeTest.RejectsDecimal256ValuesOutsideDeclaredPrecision; the masking helper has its own cases in ArrowParentNullsTest (MasksTheSlotsOfNullListRows, MasksTheSlotsOfNullFixedSizeListAndStructRows, MasksTheItemsOfNullMapRows, MasksTheChildSlotsOutsideASlicedParent, ReturnsTheChildUnchangedWhenEverySlotIsVisible, RejectsSlotsOutsideTheChildAndUnknownParents). Building such a payload needs hand-made buffers (pyarrow's converters reject a value outside the declared precision), so the test builds the batch shape directly through the converter, like the DATE32 test of the previous round; the Lance writer's pass-through of fixed-size-list child data was checked for that thread.
| continue; | ||
| } | ||
| if (validity == nullptr) { | ||
| auto allocated = arrow::AllocateBitmap(bit_length); |
There was a problem hiding this comment.
[P2] Charge copied validity bitmaps to the query memory tracker
arrow::AllocateBitmap(bit_length) uses Arrow's default pool, whereas Doris's tracked ArrowMemoryPool is a separate pool selected explicitly by the Lance converter. This helper therefore allocates its owned validity copy outside query/workload memory accounting and allocation-limit checks. Child cardinality is independent of the parent batch size: a NULL list retaining 100 million byte children needs a 12.5 MB bitmap here, multiplied by nested columns and parallel readers. Route these copies through a Doris-tracked pool or buffer and cover accounting/allocation failure for this SerDe stage; the normalizer's memory-pool test does not reach it. This affects the shared ARRAY/MAP/STRUCT Arrow consumers as well as Lance.
There was a problem hiding this comment.
Fixed in 1c74dbab92c: the masked validity bitmap is now allocated from ExecEnv::arrow_memory_pool(), the tracked doris::ArrowMemoryPool that the Lance converter and the ADBC reader already use, so it is charged to the query and passes its limit check; unit tests that never initialize ExecEnv fall back to Arrow's default pool, as LanceRecordBatchConverter does. ArrowParentNullsTest.AllocatesTheMaskFromTheDorisArrowPool puts a proxy pool into ExecEnv and checks that the bitmap is charged to it and released with the masked array, and ArrowParentNullsTest.ReportsMaskAllocationFailure checks that a pool refusing the bitmap fails the read with a Status. The scale is smaller than described, though. The bitmap is one bit per child slot and lives only until the child SerDe returns, while the same call appends at least two tracked bytes per child slot to the Doris column (value plus null map; the segment of a NULL list row is read as NULL elements), so the 100-million-child example already charges about 200 MB to the query in that call, against 12.5 MB here. Columns are converted one after another and each mask is released once its child is read, so at most one bitmap per nesting level (two for a map) is alive at a time, not one per nested column.
Cloud UT Coverage ReportIncrement line coverage Increment coverage report
|
TPC-H: Total hot run time: 28317 ms |
TPC-DS: Total hot run time: 151298 ms |
ClickBench: Total hot run time: 25.11 s |
BE UT Coverage ReportIncrement line coverage Increment coverage report
|
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
…sks to the query ### What problem does this PR solve? Issue Number: None Related PR: apache#69581 Problem Summary: 1. The Lance normalizer narrows a Decimal256 leaf of precision <= 38 to decimal128 with Arrow's safe cast, and for a nested leaf it cast the whole raw child array. The values under a NULL list, fixed-size-list, map or struct row, which Arrow leaves unspecified, and for a sliced parent the values of the rows outside the slice were precision-checked too. A FIXED_SIZE_LIST<decimal256(18,2),1> with validity [1, 0, 1] may hold the unscaled children [1, 10^18, 2]; its rows are [0.01], NULL, [0.02], yet the hidden 10^18 failed the batch with "Decimal value does not fit in precision 18". The SerDe-level masking added for the hidden DATE32 values runs only after normalization. Fix: before a child goes to a child normalizer that narrows a decimal, every child slot that no visible non-NULL parent row references is marked NULL (`mask_arrow_child_slots_not_visible()`). Each level masks its own children, so the descendants of a hidden slot are hidden too. A visible value outside the declared precision is still rejected. A map key cannot be NULL, so a Decimal256 map key under a NULL map row is still checked. Children that narrow nothing skip the mask. 2. The validity bitmaps of these masks came from Arrow's default pool, which the query's memory tracker does not see. They now come from ExecEnv's `doris::ArrowMemoryPool`, the pool the Lance converter already uses; unit tests that never initialize ExecEnv fall back to the default pool, as the converter does. Before: the batch above failed. After: it reads as [0.01], NULL, [0.02]. branch-4.1 and branch-4.2 do not narrow Lance Decimal256 values and have no such masks. ### Release note None ### Check List (For Author) - Test: Unit Test - New: `LanceTableReaderTypeTest.NarrowsDecimal256PayloadsHiddenByNullParents` (FIXED_SIZE_LIST, LIST, STRUCT, MAP and LIST<STRUCT> over a hidden 10^18, and a sliced LIST); `RejectsDecimal256ValuesOutsideDeclaredPrecision` gains a nested visible out-of-range value; `ArrowParentNullsTest` gains 8 cases (each parent type, a sliced parent, no allocation when every slot is visible, offsets outside the child, the mask allocated from the Doris Arrow pool, allocation failure). - `ArrowParentNullsTest.*`, `DataTypeSerDeArrowTest.*`, `DataTypeSerDeArrowValidationTest.*`, `ArrowColumnToDorisColumnTest.*`, the Lance and ADBC suites and `IcebergV2ReaderTest.*` on macOS arm64 (ASAN): 306 pass, 7 ADBC SQLite cases skip (no SQLite driver here). Mutations: without the normalizer mask the Lance test fails; with the mask allocated from the default pool the two pool tests fail; without masking the slots outside a sliced parent the slice test and the Lance test fail. - clang-tidy (llvm 22) reports only `readability-inconsistent-ifelse-braces` on the three `DORIS_CHECK` lines of the new function, the macro's expansion that existing `DORIS_CHECK`s report too. - Behavior changed: No (visible values are unchanged; the narrowing that hidden values tripped is added by this PR) - Does this need documentation: No Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
### What problem does this PR solve? Issue Number: None Related PR: apache#69581 Problem Summary: A Lance nested array whose descendants start at an offset is copied into an offset-zero array before decoding, and the copy runs `ValidateFull()` first, for the offset and length checks the builder needs. `ValidateFull()` also checks every non-NULL value of every descendant over its whole length: UTF-8 for strings, the precision of decimals, whole days for Date64 and the day range for Time32 / Time64, also in slots that no visible row reaches. The slice [1, 4) of a FIXED_SIZE_LIST<STRING, 1> with rows ["z"], ["a"], NULL, ["c"] may hold the child strings ["z", "a", "\xff", "c"]; its rows are ["a"], NULL, ["c"], yet the hidden "\xff" failed the batch with "Invalid UTF8 sequence". The previous fixes mask such slots only for the element SerDes and for the Decimal256 leaves the normalizer narrows, so a hidden value in any other subtree of a compacted array still failed: a string, a decimal256 of precision above 38, a Date64 or a time. Fix: `compact_lance_array()` masks the array before validating it: `mask_arrow_descendants_not_visible()` marks NULL, at every nesting level, every slot that no visible row reaches, through the per-level `mask_arrow_child_slots_not_visible()`. `ValidateFull()` and the copy then run on the masked array, so the offset and length checks stay and an invalid visible value still fails. The builders copy no child slot of a NULL list, map or fixed-size-list row, and the masked fields of a NULL struct row are read as NULL anyway, so every visible value is unchanged. Map keys cannot be NULL and stay unmasked. A slot that is NULL already needs no hiding, so the subtrees the normalizer masked before narrowing get no second bitmap, and an array with nothing hidden allocates nothing. Before: the slice above failed to read. After: it reads as ["a"], NULL, ["c"]. branch-4.1 and branch-4.2 do not compact Lance arrays this way. ### Release note None ### Check List (For Author) - Test: Unit Test - New: `LanceTableReaderTypeTest.CompactsArraysWithInvalidPayloadsHiddenByNullParents` (FIXED_SIZE_LIST<STRING> with invalid UTF-8, LIST<DECIMAL256(40,2)> out of precision and LIST<STRUCT<s, d>> under NULL rows and outside the slice), `RejectsVisibleInvalidPayloadsInCompactedArrays` (the same values visible), `ArrowParentNullsTest.MasksHiddenSlotsAtEveryNestingLevel` and `LeavesVisibleAndAlreadyMaskedArraysUnchanged`. - `ArrowParentNullsTest.*`, `DataTypeSerDeArrowTest.*`, `DataTypeSerDeArrowValidationTest.*`, `ArrowColumnToDorisColumnTest.*`, the Lance and ADBC suites and `IcebergV2ReaderTest.*` on macOS arm64 (ASAN): 310 pass, 7 ADBC SQLite cases skip. Mutations: without the mask before validation the compaction test fails; without recursion the nesting test and the compaction test fail; masking already-NULL slots again fails the no-second-bitmap test. - clang-tidy (llvm 22) reports only `readability-inconsistent-ifelse-braces` on the three `DORIS_CHECK` lines of the new function. - Behavior changed: No (visible values are unchanged) - Does this need documentation: No Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
|
Addressed the sixth review round in
BE unit tests on macOS arm64 (ASAN): |
|
run buildall |
|
/review |
|
Codex automated review failed and did not complete. Error: workspace routing discovery unauthorized (401) Please inspect the workflow logs and rerun the review after the underlying issue is resolved. |
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 filterprefilterflag and profile layout (#68813), distance bounds forvector_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 linksliblance_cbut has no Lance reader, so everyFORMAT_LANCEscan 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()withformat=lance, and thevector_search/full_text_searchTVFs. 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, includingTExternalSearchRequest.5(prefilter) andTVectorSearchParams.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 problem, and what it cost.
FORMAT_LANCEscanfile_scan_operatorkeeps it off FileScannerV2 and V1 has no Lance path, so the query failsLanceTableReaderin FileScannerV2fetch_table_schemafor a local Lance TVFNULL_TYPEGlobalRowLoacationV2only has a 32-bit row id, but a Lance row id is a 64-bit id within the snapshottake_rowsHow this PR fixes it.
Reader:
be/src/format_v2/lance/*andbe/src/format_v2/table/lance_reader.*, as on branch-4.2 except one local renamed for master's-Wshadow. The unit tests constructRuntimeState(TQueryOptions(), globals), because master removed the globals-only constructor.FileScannerV2:
is_supported, the reader dispatch and_to_file_formathandleFORMAT_LANCE/"lance";FileFormat::LANCEis added to its two exhaustive switches. Lance queries use FileScannerV2 whateverenable_file_scanner_v2says, as ADBC and fluss do: there is no V1 Lance reader, and the variable is fuzzy in the regression harness. On branch-4.2 a Lance query fails with NotSupported when the variable is off. Loads are unchanged.Row locations:
ROW_VERSION {FILE_LOCAL_ROW_ID = 0, LANCE_DATASET_ROW_ID = 1}and a union layout forGlobalRowLoacationV2. Version 0 keeps its byte layout, which static asserts pin.PRequestBlockDesc.row_idbecomesuint64(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 stampsrow_location_version.RowIdStorageReadervalidates the counts and the version, range-checks internal row ids againstuint32, and groups Lance rows by plan node, snapshot version and dataset URI, so each dataset is read with onetake_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,TVectorSearchParams1-7,TFtsCoverageMode,TFtsQueryType,TFtsMatchOperator,TFullTextSearchParams,TSearchFilterFormat,TSearchFilter,TVectorSearchOptions,TExternalSearchQuery),TExternalSearchRequest2-5 andTLanceFileDesc4-6, all with the branch-4.1 / 4.2 ids. The enums spell out their 4.x values. No id is reused.After the snapshot (one commit each):
prefilterdecides 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 underLanceTiming,LanceIO,LanceIndex,LanceVectorIndex,LanceScalarIndex,LanceExecutionPlan,LanceScan,LanceSearchPlanandLanceDataCache, 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. [opt](lance) add prefilter option for filter execution control #68813's FE tweak that hidesTUnit.NONElabels inAggCounterbelongs to the FE port.use_indexsays; a split with segments underuse_index = falseis rejected. 4.x FEs plan segments only whenuse_indexallows them.distance_lower_bound(inclusive) anddistance_upper_bound(exclusive) are validated (finite FLOAT values, lower < upper at FLOAT precision, single-vector only) and installed withlance_scanner_set_distance_range(), which master's pinned lance-c provides.Review fixes:
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 thatColumnStructcannot hold (its row count comes from its first child).>=runtime filters stay residual-only. Lance compares floats by totalOrder, so a pushedx >= cdropped negative-NaN rows that Doris keeps.DataTypeDateV2SerDe::read_column_from_arrow) rejects days outside 0000-01-01..9999-12-31 instead of returning a non-null0000-00-00, skips null slots and adds the epoch in 64 bits: the DATE32 range part of branch-4.1 / 4.2 [fix](arrow) Correct year-zero DATE conversion on branch-4.1 #68649. It also changes ADBC, Arrow stream load and paimon-rust reads.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 itsValidateFull(), 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.Core:
to_protobufkeepsNULL_TYPEfor 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:
lance_index_cache_size_byteslance_metadata_cache_size_bytesenable_lance_data_cachelance_data_cache_path${DORIS_HOME}/lance_data_cachelance_data_cache_disk_capacity_byteslance_data_cache_read_block_size_byteslance_io_buffer_size_bytes(mutable)lance_batch_readahead(mutable)lance_fragment_readahead(mutable)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. 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. Sizelance_data_cache_disk_capacity_bytesandlance_data_cache_read_block_size_bytesagainst the BE's open-file limit, or setenable_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/alldo not shrink them, andenable_lance_data_cache = falsedoes 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$bucketfor 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 setenable_lance_data_cache = false. Both are as on branch-4.1 / 4.2 and tracked as follow-ups.Build: macOS links
-framework IOKit. This is the first time objects ofliblance_c.aare pulled intodoris_be, so the binary and the link time grow. The build-hygiene reverse reach budget ofutil/rle_encoding.his rebaselined from 825 to 911: master5caaeb3e3d4alone is at 907 (limit 908), and the reader's four TUs reach it through the existingexprs/vexpr_context.h->storage/segment/column_reader.hpath, which no include narrowing in the Lance files can avoid.util/threadpool.hreaches 1096 TUs with this PR, within master's baseline of 1200.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). TheInitReaderAvgTime/GetBlockAvgTimeprofile fix that #67005 carried goes in a separate PR.Results.
FORMAT_LANCEqueryLanceTableReaderin FileScannerV2take_rows0000-00-00InvalidArgumentBE unit tests (ASAN, macOS arm64, against master's pinned lance-c): 428 cases, all pass. They cover the Lance suites (101:
LanceTableReader*79,LanceNestedNullTest10,LanceRuntimeFilterHelperTest9,LanceSessionManagerTest3); the row-location protocol (MaterializationSharedStateTest,RowIdStorageReaderTest, and the newRowIdFetchRequestValidationTest: count mismatch, unknown version, a Lance version on an internal mapping, an internal row id of 2^32);FileScannerV2Testwith the newLanceQueriesAlwaysUseScannerV2,FileScanOperatorAdbcTestandFileScanOperatorFlussTest; the ORC / Parquet global row id tests that now read.file_local; and the Arrow / protobuf / DATE serde suites with the newArrowParentNullsTest, 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 (ArrowParentNullsTest10,LanceTableReaderTypeTest3) and extends one; with them the Arrow SerDe, Lance, ADBC andIcebergV2ReaderTestsuites run 317 cases: 310 pass and the 7 ADBC SQLite cases skip without a local SQLite driver.Release note
Add nine
lance_*BE configs andlance_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)
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 master5caaeb3e3d4([feature](external) Support Elasticsearch HTTP scans in FileScannerV2 #69614 and [refactor](external) Remove Hive ACID, Avro and LakeSoul readers on master #69601 conflicted in the scanner selection, theFileFormatenum, its test and the reach budget table): the 45 TUs that this PR changes or that master changed next to it were syntax-checked against the rebased tree with the UT build flags, and CI runs the suites again. Reverting any of the review fixes, the [opt](lance) add prefilter option for filter execution control #68813 flag, the [feature](lance) Search historical versions, tags and branches with vector_search and full_text_search #68707 split rule or the [feature](lance) Support distance bounds in vector_search #68815 bounds makes its new tests fail. On macOS,DataTypeDateTimeV2SerDeTestneeds a test-utility fix that goes in a separate PR (its setup dereferencesstd::set::end()); it ran with that fix applied locally.build-support/check-build-hygiene.sh(on this branch rebased onto master5caaeb3e3d4), clang-format 16 andbuild-support/run-clang-tidy.shon the changed files: all pass; on the changed lines clang-tidy (llvm 22) reports onlyreadability-inconsistent-ifelse-braceson theDORIS_CHECKlines of the masking functions, the macro's expansion that master's existingDORIS_CHECKs report too.PRequestBlockDesc.row_idisuint64; phase two of TopN lazy materialization rejects row locations of an unknown version or size;FORMAT_LANCEscans run on FileScannerV2; an Arrow DATE32 day outside the Doris DATE range fails the read instead of becoming0000-00-00.Check List (For Reviewer who merge this PR)