Skip to content

[fix](be) Reject Arrow timestamps outside the DATETIMEV2 and TIMESTAMPTZ ranges - #69697

Merged
morningman merged 2 commits into
apache:masterfrom
morningman:b2m-t37f-arrow-timestamp-year-range
Nov 3, 2026
Merged

morningman merged 2 commits into
apache:masterfrom
morningman:b2m-t37f-arrow-timestamp-year-range

Conversation

@morningman

@morningman morningman commented Oct 27, 2026 •

Copy link
Copy Markdown
Contributor

What problem does this PR solve?

Issue Number: None

Related PR: #69581 #68516

Problem Summary:

In short. An Arrow timestamp whose civil year is outside 0000..9999 is read into DATETIMEV2 or TIMESTAMPTZ as a different, valid-looking value: timestamp(us) 2005980768000000000, which is 65537-01-01, reads as a non-null 0001-01-01. This PR checks the year before the value is narrowed into Doris' 16-bit year field, as the ORC path already does, and fails the read otherwise. The review of #69581 found it on the Lance path; the code is shared by every Arrow reader.

Background. DataTypeDateTimeV2SerDe::read_column_from_arrow and DataTypeTimeStampTzSerDe::read_column_from_arrow decode Arrow timestamps for ADBC, Arrow stream load, paimon-rust, remote Doris and, once #69581 lands, Lance, nested leaves included. Both convert epoch seconds with DateV2Value::from_unixtime(), which passes cctz's 64-bit year to unchecked_set_time(uint16_t year, ...) ("there's no overflow check since it's hot path"). #68516 added a check before this narrowing to the ORC path (orc_timestamp_to_datetime()).

The problem, and what it cost.

Arrow timestamp(us) DATETIMEV2 before TIMESTAMPTZ before
253402300800000000 (10000-01-01) stored as year 10000, outside the type's range error
2005980768000000000 (65537-01-01) 0001-01-01 00:00:00 0001-01-01 00:00:00+00:00
a year before 0000 wraps to 655xx or back into 0000..9999 error, or a wrapped valid year

In the wrapped cases the read succeeds with a wrong value, and nothing in the result tells the user.

How this PR fixes it.

  • DATETIMEV2: convert to civil time in the value's time zone (UTC for a timezone-naive Arrow timestamp, the session time zone for a zoned one) and reject a year outside 0000..9999 before packing, like orc_timestamp_to_datetime(). After packing, is_valid_date() is checked too: cctz counts year 0 as a leap year and Doris does not (is_leap(0) is false), so the civil date 0000-02-29 passes the year check, and before this PR it was stored as an invalid date.
  • TIMESTAMPTZ is stored in UTC, so append_timestamptz_from_utc_epoch_micros checks the instant against 0000-01-01 00:00:00 UTC .. 9999-12-31 23:59:59.999999 UTC before converting. Its runtime is_valid_date() check after packing stays, for the same leap day: timestamp(us) -62162121600000000 (0000-02-29 UTC) is inside the range but packs into an invalid value. The range message now says 0000-9999, the range is_valid_date() accepts.
  • Both fail the read with DataQualityError, as the other range errors of these readers do. NULL slots are still skipped before conversion.
  • Not changed: the Parquet INT64 / INT96 timestamp paths validate the UTC range before packing (parquet_timestamp_micros(), parquet_int96_timestamp_micros()), so they could not wrap. Arrow reads into the deprecated DATE / DATETIME (v1) types go through VecDateTimeValue::from_unixtime() with the same unchecked narrowing; they are left as they are. orc_timestamp_to_datetime() checks only the year, so an ORC timestamp on 0000-02-29 is still stored as an invalid date; it is not changed here.

Merge order: after #69581. A NULL STRUCT, LIST or MAP row may hide any child value, an out-of-range timestamp included, because Arrow leaves the payload under a NULL parent unspecified. #69581 masks those child slots in the shared nested SerDes (mask_arrow_children_hidden_by_null_parents, df95b41) before any leaf SerDe reads them. Without it, the checks here fail a batch whose visible values are valid: a STRUCT with rows [NULL, {ts: 0}] whose NULL row carries 2005980768000000000 reads with DataQualityError on this PR alone, and reads both rows on a tree with both PRs.

Results.

Arrow timestamp(us) DATETIMEV2 after TIMESTAMPTZ after
253402300799999999 (9999-12-31 23:59:59.999999) read read
-62167219200000000 (0000-01-01) read read
-62162121600000000 (0000-02-29, not a Doris date) DataQualityError DataQualityError
10000-01-01, 65537-01-01, -62167219200000001 (year -1) DataQualityError DataQualityError
zoned, at +08:00: 253402271999999999 / 253402272000000000 9999-12-31 23:59:59.999999 / DataQualityError

Release note

Reading an Arrow timestamp whose year is outside 0000..9999 into DATETIMEV2 or TIMESTAMPTZ (ADBC, Arrow stream load, paimon-rust, remote Doris) now fails instead of returning a different date. So does a DATETIMEV2 read of 0000-02-29, which is not a valid Doris date.

Check List (For Author)

  • Test
    • Regression test
    • Unit Test
      • New DataTypeDateTimeV2SerDeTest.RejectsArrowTimestampsOutsideDateTimeV2Range, RejectsNestedArrowTimestampOutsideDateTimeV2Range (an ARRAY leaf) and DataTypeTimeStampTzSerDeTest.RejectsArrowTimestampsOutsideTimestampTzRange. With the previous DATETIMEV2 code both DATETIMEV2 tests fail; with the previous TIMESTAMPTZ code 65537-01-01 reads as a valid value and its test fails. The two range tests also read 0000-02-29 and expect DataQualityError; they fail when the checks after packing skip that value.
      • These suites and the Arrow / Parquet SerDe suites pass on macOS arm64 (ASAN), run on a tree that also carries [feature](be) Read Lance datasets with FileScannerV2 #69581, whose Lance reader suites pass with it (279 tests). After the 0000-02-29 change, the serde, Arrow round-trip, nested serde and Lance reader suites pass on such a tree (168 tests).
    • Manual test (add detailed scripts or steps below)
    • No need to test or manual test. Explain why:
  • Behavior changed:
    • No.
    • Yes.
      Such reads now fail with DataQualityError instead of returning a wrong date.
  • 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

…PTZ ranges

### What problem does this PR solve?

Issue Number: None

Related PR: apache#69581

Problem Summary:

An Arrow timestamp whose civil year is outside 0000..9999 was read as a
different value. `DataTypeDateTimeV2SerDe::read_column_from_arrow` converted
each value with `DateV2Value::from_unixtime()`, which passes the cctz year to
`unchecked_set_time(uint16_t year, ...)` without a check, and nothing checked
the result. So timestamp(us) 2005980768000000000 (65537-01-01 00:00:00) read
as a non-null 0001-01-01 00:00:00, the years 10000..65535 were stored as
DATETIMEV2 values outside its range, and years before 0000 wrapped as well.
The TIMESTAMPTZ Arrow path converts through the same `from_unixtime()` and
called `is_valid_date()` only after packing, which a year wrapped back into
0000..9999 passes. ADBC, Arrow stream load, paimon-rust and remote Doris
reads all decode Arrow timestamps through these SerDes, nested leaves
included. apache#69581's review found it for Lance.

DATETIMEV2 now converts to civil time in the value's time zone and rejects a
year outside 0000..9999 before packing, as the ORC path does since apache#68516
(`orc_timestamp_to_datetime()`); a zoned timestamp is checked after the shift
to the session time zone. TIMESTAMPTZ, which is stored in UTC, checks the
instant against 0000-01-01 00:00:00 UTC .. 9999-12-31 23:59:59.999999 UTC
before converting, so its check after packing can no longer fail and becomes
a DCHECK; the message now names the 0000-9999 range it enforces, as
`is_valid_date()` already accepted year 0. Both fail the read with
DataQualityError. NULL slots are still skipped before conversion.

The Parquet INT64 and INT96 timestamp paths validate the UTC range before
packing (`parquet_timestamp_micros()`, `parquet_int96_timestamp_micros()`),
so they could not wrap and behave as before. Arrow reads into DATE and
DATETIME (v1) use `VecDateTimeValue::from_unixtime()`, which has the same
unchecked narrowing; those deprecated types are not changed here.

### Release note

Reading an Arrow timestamp whose year is outside 0000..9999 into DATETIMEV2
or TIMESTAMPTZ (ADBC, Arrow stream load, paimon-rust, remote Doris) now
fails instead of returning a different date.

### Check List (For Author)

- Test: Unit Test
    - New `DataTypeDateTimeV2SerDeTest.RejectsArrowTimestampsOutsideDateTimeV2Range`
      (the last and first instants of the range pass; 10000-01-01,
      65537-01-01 and the last microsecond of year -1 fail; a zoned value is
      checked at +08:00), `RejectsNestedArrowTimestampOutsideDateTimeV2Range`
      (an ARRAY leaf) and
      `DataTypeTimeStampTzSerDeTest.RejectsArrowTimestampsOutsideTimestampTzRange`.
    - With the previous DATETIMEV2 code, both DATETIMEV2 tests fail (10000-01-01
      and 65537-01-01 read without an error); with the previous TIMESTAMPTZ
      code, 65537-01-01 reads as a valid value and the TIMESTAMPTZ test fails.
    - The two SerDe suites and the Arrow and Parquet SerDe suites pass on
      macOS arm64 (ASAN), run on a tree that also carries apache#69581, whose Lance
      reader suites pass with it (279 tests in all).
- Behavior changed: Yes. See the release note.
- 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.

Static review of head 3f336d2294cb9d06ba42e055d08ae2b99cb4d673: two verified P2 correctness regressions are recorded inline. The year checks prevent the intended wraparound, but NULL complex ancestors and Doris' year-zero calendar rule need handling before the change is correct. No existing inline findings were present; there are no existing P0/P1 comments to carry forward. No additional user focus was supplied.

Critical checkpoint conclusions:

  • Goal and proof: the added unit cases exercise year 10000/65537/-1 rejection, supported endpoints, a session-zone upper boundary, and a visible ARRAY timestamp leaf. They miss both reported regressions.
  • Scope and reuse: the two production changes are small and focused and follow the ORC pre-narrowing pattern; no unrelated code or interfaces were added.
  • Concurrency and locking: conversion runs synchronously on caller-owned mutable columns; no new shared mutable state, threads, locks, or lock ordering.
  • Lifecycle and initialization: added values are local or constexpr; the existing function-local UTC object has no new cross-TU dependency, ownership cycle, or resource lifetime.
  • Configuration: no settings added or changed; existing Arrow input validation does not combine ancestor validity.
  • Compatibility: packed column and storage layouts, function interfaces, and rolling-upgrade protocols are unchanged. External-input validation changes as intended, with the two inline exceptions requiring correction.
  • Thrift/protobuf IDs: no generated definitions changed.
  • Parallel paths: both timestamp types and their nullable/ARRAY/MAP/STRUCT delegates were traced. ORC already checks the civil year; Parquet INT64/INT96 validates its 0001..9999 range before the shared TIMESTAMPTZ helper. Legacy DATE/DATETIME narrowing and unrelated precision behavior predate this PR.
  • Conditional checks: the new branches document concrete wraparound examples. The removed calendar-validation branch cannot safely become DCHECK because year bounds do not prove Doris calendar validity.
  • Test coverage: existing tests add ordinary Arrow units, negative fractions, scaling, direct leaf NULL payloads, and timezone-naive behavior. Add year-zero February 29 and NULL-ancestor cases; no end-to-end test for these cases is present.
  • Test results: no generated result files changed; new expected scalar values/error messages match the inspected logic. Builds, tests, sanitizers, benchmarks, formatting checks, and runtime validation were not run under this static-only contract.
  • Observability: the new DataQualityError messages carry epoch/type context; no new metrics or per-row logging is needed for these local checks.
  • Transactions and persistence: no transaction, visible-version, delete-bitmap, EditLog, replay, or master-failover code changed.
  • Data mutation and atomicity: only transient destination columns are appended. Reader Status propagation and scanner publication prevent partially converted failed batches from becoming output; no separate persistence or cleanup defect was found.
  • FE/BE propagation: no new variables or metadata cross the boundary.
  • Performance: constant work per value; DATETIMEV2 retains one cctz conversion, with no new large allocations, copies, repeated scans, or row-wise COW detachment. No additional static performance issue was found.
  • Other BE obligations: allocation-aware column storage and mutable ownership remain unchanged. Test-only header additions introduce no hub-header, template-instantiation, unity .cpp inclusion, or new libc API concern; final ELF compatibility was not executed.

All four changed files, candidate dispositions, and both initial risk areas were covered. The calendar invariant failure and NULL-ancestor failure are separate findings; repeated instances were deduplicated. There are no unresolved static review questions. Validation remains limited to source inspection.

Review converged after 3 round(s).

"Decoded TIMESTAMPTZ is outside the Doris 0001-9999 range: micros={}",
timestamp_micros);
}
DCHECK(timestamp_tz.is_valid_date());

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.

[P2] Keep the runtime calendar-validity check for TIMESTAMPTZ

The new epoch interval does not imply is_valid_date(). Arrow timestamp(MICRO) value -62162121600000000 is UTC 0000-02-29 00:00:00 and passes these bounds. cctz's Gregorian leap-year rule accepts year-zero February 29, whereas Doris is_leap(0) in util/time_lut.h explicitly returns false, so the resulting value fails DateV2Value::is_valid_date(). Previously this input returned DataQualityError; now it aborts a DCHECK-enabled BE or appends an invalid TIMESTAMPTZ when DCHECK is disabled (UTC formatting can then emit only +00:00). Keep the new pre-narrowing bounds check, retain a runtime validity failure after conversion, and add this leap-day input to the unit test.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Fixed in 6c1fd8f. The range check before packing stays, and append_timestamptz_from_utc_epoch_micros checks is_valid_date() at run time again after packing instead of the DCHECK, so -62162121600000000 (0000-02-29 UTC) fails with Decoded TIMESTAMPTZ is not a valid Doris date, as it did on master. The DATETIMEV2 Arrow path checked only the civil year and stored that date as an invalid DATETIMEV2 (it did so before this PR too); it now checks is_valid_date() after packing as well. RejectsArrowTimestampsOutsideTimestampTzRange and RejectsArrowTimestampsOutsideDateTimeV2Range read the leap day and expect DataQualityError; both fail when the new checks skip that value. orc_timestamp_to_datetime() has the same year-only check and is left to a change of its own.

// as orc_timestamp_to_datetime() does for ORC.
const auto civil = cctz::convert(
cctz::time_point<cctz::seconds>(cctz::seconds(seconds)), real_ctz);
if (civil.year() < 0 || civil.year() > 9999) {

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.

[P2] Mask NULL ancestors before validating nested timestamp leaves

A valid nullable Arrow STRUCT can have parent validity [false, true] and a non-null timestamp(MICRO) child containing [2005980768000000000, 0]; its logical rows are [NULL, {ts:1970-01-01}]. Arrow specifies independent parent/child validity. DataTypeNullableSerDe copies the parent bitmap, then DataTypeStructSerDe passes field(i) unchanged, so the leaf's IsNull(0) is false and this new check rejects the hidden year-65537 payload. The previous conversion's wrapped year was hidden by the NULL parent, so this batch succeeded. The same regression affects the new TIMESTAMPTZ check and NULL LIST parents with arbitrary nonempty child spans. Normalization leaves ordinary struct/list timestamp arrays unchanged, and stream-load/ADBC callers propagate the error, failing a read whose visible timestamps are valid. Carry ancestor validity or skip/default children of NULL parents before applying either check, and add mixed NULL/non-NULL parent coverage.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Agreed on the mechanism. The fix is the NULL-parent masking in #69581 rather than a second copy here: df95b41 (mask_arrow_children_hidden_by_null_parents) masks the child slots of NULL list, map and struct rows in the shared nested SerDes before any leaf SerDe reads them, so the leaf checks of this PR never see a hidden payload, for every Arrow caller. ArrowParentNullsTest there covers mixed NULL / non-NULL parents and child offsets.

I checked your example on a tree with both PRs: a STRUCT with rows [NULL, {ts: 0}] whose NULL row carries 2005980768000000000, and a LIST [NULL, [0]] whose NULL row spans that value, both read two rows with row 0 NULL. With the masking taken out, the STRUCT case fails with Arrow timestamp 2005980768000000000 of type timestamp[us] is outside the DATETIMEV2 range. This PR will be merged after #69581, so master does not carry the regression in between; the description now says so. When this PR is rebased onto #69581, I will add your STRUCT and LIST cases here as a test.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Follow-up on the STRUCT and LIST cases promised above. This PR was merged on 2026-11-03, before #69581, so the cases went into #69581 instead, which was merged on 2026-11-06 as 0d1ae9e: DataTypeDateTimeV2SerDeTest.IgnoresArrowTimestampsHiddenByNullParents in be/test/core/data_type_serde/data_type_serde_datetime_v2_test.cpp. For both DATETIMEV2 and TIMESTAMPTZ, it hides 65537-01-01 (2005980768000000000) under the NULL row of a LIST, a FIXED_SIZE_LIST, a MAP value and a STRUCT. The NULL row reads as NULL and the visible rows keep their values, while the same value in a visible row still fails with the range error. Returning the child unmasked from mask_arrow_children_hidden_by_null_parents fails the test, and dropping only the STRUCT mask fails it at the STRUCT case.

A correction to my reply above: master did carry the regression between the two merges, from 2026-11-03 to 2026-11-06. In that window, a batch whose NULL list, map or struct row hid an out-of-range timestamp failed with DataQualityError instead of being read. It raised an error and did not return wrong values.

@hello-stephen

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

------ Round 1 ----------------------------------
============================================
q1	17615	5133	5150	5133
q2	2113	335	247	247
q3	10219	1366	819	819
q4	4678	458	340	340
q5	7433	804	529	529
q6	181	182	152	152
q7	758	803	595	595
q8	9266	1353	1123	1123
q9	5467	4186	4169	4169
q10	6834	1281	1054	1054
q11	434	244	224	224
q12	639	401	291	291
q13	18088	3571	3018	3018
q14	284	268	249	249
q15	q16	711	708	677	677
q17	1188	746	576	576
q18	7047	6162	6116	6116
q19	1095	964	620	620
q20	369	329	229	229
q21	4424	2149	2340	2149
q22	349	293	265	265
Total cold run time: 99192 ms
Total hot run time: 28575 ms

----- Round 2, with runtime_filter_mode=off -----
============================================
q1	5646	5583	5586	5583
q2	667	574	536	536
q3	5196	5363	4876	4876
q4	2010	2118	1532	1532
q5	5243	5123	5417	5123
q6	256	200	154	154
q7	2353	2077	1956	1956
q8	3081	2727	2725	2725
q9	8111	7958	7940	7940
q10	3716	3778	3279	3279
q11	628	470	445	445
q12	680	717	520	520
q13	3527	3890	3340	3340
q14	323	316	319	316
q15	q16	692	714	655	655
q17	8350	6965	6987	6965
q18	13146	12260	12993	12260
q19	909	817	813	813
q20	2156	2150	1912	1912
q21	5686	4670	4738	4670
q22	490	466	412	412
Total cold run time: 72866 ms
Total hot run time: 66012 ms

@hello-stephen

Copy link
Copy Markdown
Contributor
TPC-DS: Total hot run time: 151836 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 3f336d2294cb9d06ba42e055d08ae2b99cb4d673, data reload: false

query5	4311	567	454	454
query6	434	217	190	190
query7	4826	454	235	235
query8	336	159	148	148
query9	8757	3264	3284	3264
query10	461	319	260	260
query11	5998	3834	3548	3548
query12	145	91	88	88
query13	1252	443	324	324
query14	6110	3919	3628	3628
query14_1	3428	3409	3406	3406
query15	203	199	186	186
query16	950	432	434	432
query17	873	643	534	534
query18	2425	438	327	327
query19	200	168	158	158
query20	88	85	85	85
query21	210	130	108	108
query22	12952	12977	12913	12913
query23	13386	12880	12499	12499
query23_1	12593	12542	12411	12411
query24	7344	851	483	483
query24_1	491	488	502	488
query25	534	421	361	361
query26	1270	272	144	144
query27	2750	441	280	280
query28	4633	2081	2119	2081
query29	1571	619	466	466
query30	299	217	189	189
query31	1003	945	828	828
query32	145	99	95	95
query33	559	302	244	244
query34	956	858	524	524
query35	745	743	636	636
query36	860	821	725	725
query37	133	105	86	86
query38	1792	1745	1666	1666
query39	793	789	747	747
query39_1	743	737	735	735
query40	218	120	100	100
query41	73	70	69	69
query42	90	88	86	86
query43	356	361	317	317
query44	1397	715	742	715
query45	180	181	175	175
query46	865	991	588	588
query47	2921	2977	2913	2913
query48	319	313	261	261
query49	584	396	291	291
query50	684	284	216	216
query51	10274	10859	10446	10446
query52	85	78	72	72
query53	194	206	158	158
query54	241	189	189	189
query55	70	76	67	67
query56	239	220	218	218
query57	1633	1592	1560	1560
query58	293	275	267	267
query59	2135	2181	2014	2014
query60	278	239	228	228
query61	151	151	143	143
query62	388	332	287	287
query63	188	164	162	162
query64	2739	965	773	773
query65	3314	3258	3260	3258
query66	1771	417	297	297
query67	20073	20186	20034	20034
query68	2898	1025	642	642
query69	381	281	243	243
query70	845	801	766	766
query71	307	239	218	218
query72	2822	2563	2259	2259
query73	550	541	307	307
query74	4621	4470	4346	4346
query75	2337	2257	1905	1905
query76	2330	1021	618	618
query77	340	376	273	273
query78	9823	9805	9143	9143
query79	994	902	547	547
query80	548	421	351	351
query81	532	313	267	267
query82	575	139	103	103
query83	306	194	182	182
query84	313	126	108	108
query85	822	428	374	374
query86	329	245	227	227
query87	2024	1930	1784	1784
query88	3893	2851	2824	2824
query89	335	283	244	244
query90	1887	173	166	166
query91	161	147	129	129
query92	101	87	85	85
query93	1033	1018	593	593
query94	460	307	266	266
query95	583	394	308	308
query96	697	511	243	243
query97	2466	2418	2295	2295
query98	162	156	147	147
query99	783	756	672	672
Total cold run time: 234335 ms
Total hot run time: 151836 ms

@hello-stephen

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

query1	0.01	0.01	0.01
query2	0.10	0.05	0.04
query3	0.25	0.09	0.08
query4	1.60	0.11	0.10
query5	0.24	0.25	0.24
query6	1.17	0.68	0.67
query7	0.04	0.01	0.00
query8	0.05	0.04	0.03
query9	0.40	0.32	0.35
query10	0.57	0.57	0.57
query11	0.19	0.14	0.14
query12	0.19	0.15	0.14
query13	0.50	0.49	0.49
query14	1.04	1.02	1.02
query15	0.64	0.61	0.61
query16	0.36	0.34	0.39
query17	1.16	1.12	1.15
query18	0.21	0.21	0.19
query19	2.10	1.99	1.98
query20	0.02	0.01	0.02
query21	15.42	0.19	0.14
query22	4.88	0.05	0.06
query23	16.27	0.32	0.12
query24	3.07	0.57	0.45
query25	0.10	0.05	0.05
query26	1.38	0.21	0.15
query27	0.05	0.05	0.05
query28	3.48	0.72	0.36
query29	12.48	4.06	3.28
query30	0.30	0.13	0.10
query31	2.77	0.57	0.32
query32	3.23	0.59	0.50
query33	3.26	3.22	3.29
query34	15.49	3.99	3.39
query35	3.34	3.35	3.33
query36	0.55	0.45	0.44
query37	0.09	0.06	0.07
query38	0.04	0.04	0.03
query39	0.04	0.03	0.03
query40	0.18	0.16	0.15
query41	0.09	0.03	0.03
query42	0.04	0.03	0.03
query43	0.04	0.04	0.03
Total cold run time: 97.43 s
Total hot run time: 24.41 s

@hello-stephen

Copy link
Copy Markdown
Contributor

BE UT Coverage Report

Increment line coverage 🎉

Increment coverage report
Complete coverage report

Category Coverage
Function Coverage 67.00% (32298/48205)
Line Coverage 51.83% (341749/659310)
Region Coverage 47.26% (277547/587236)
Branch Coverage 48.94% (130841/267351)

@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% (36197/46709)
Line Coverage 62.73% (409196/652280)
Region Coverage 59.31% (347028/585107)
Branch Coverage 60.15% (160415/266699)

### What problem does this PR solve?

Issue Number: None

Related PR: apache#69697

Problem Summary: The range check added before packing an Arrow timestamp
does not make every instant inside the range a Doris date. cctz counts
year 0 as a leap year and Doris does not (is_leap(0) in util/time_lut.h is
false), so timestamp(us) -62162121600000000, 0000-02-29 00:00:00 UTC, is
inside 0000-01-01Z..9999-12-31Z but packs into a value that
is_valid_date() rejects. For TIMESTAMPTZ this PR had turned the runtime
is_valid_date() check after packing into a DCHECK, so that input aborted
an ASAN or debug BE and appended an invalid TIMESTAMPTZ in a release
build, where master returns DataQualityError. The DATETIMEV2 Arrow path
checks only the civil year, so the same date at the session time zone was
stored as an invalid DATETIMEV2, as it was before this PR.

Both paths now check is_valid_date() at run time after packing and return
DataQualityError for such a value. The ORC path
(orc_timestamp_to_datetime) has the same year-only check and is not
changed here.

### Release note

None

### Check List (For Author)

- Test: Unit Test
    - BE UT RejectsArrowTimestampsOutsideTimestampTzRange and
      RejectsArrowTimestampsOutsideDateTimeV2Range read
      -62162121600000000 and expect DataQualityError; both fail when the
      new checks skip that value. The serde, Arrow round-trip, nested
      serde and Lance reader suites (168 tests) pass.
- Behavior changed: Yes. An Arrow timestamp on 0000-02-29 is rejected
  instead of being stored as an invalid date.
- Does this need documentation: No

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

Static review of head 6c1fd8f8ab037fba68da24861bb618d5d3cdab9a found no new valuable correctness issue beyond the existing inline context.

I reviewed both production SerDes changes and both added test files against the authoritative diff, then traced the Arrow, nullable/nested, Parquet, decoded-value, and ORC paths. The DATETIMEV2 civil-year check, TIMESTAMPTZ epoch bounds, year-zero calendar validation, negative-time normalization, timezone boundary, scale carry, and caller error/null-on-failure behavior are internally consistent. The added branches document their wraparound cases; no new hub-header, COW, template-instantiation, unity, Thrift/protobuf, locking, lifecycle, or performance concern was found.

Existing inline P2 comment 4390157295 (year-zero calendar validity) is addressed in this head and was not reposted. Existing inline P2 comment 4390157310 (hidden timestamp payloads beneath NULL LIST/STRUCT/MAP ancestors) is already-known context and remains dependent on the separate #69581 masking change; it was not duplicated. Neither is a P0/P1 ID, so existing_blocking_comment_ids is empty. The user focus file contains no additional focus.

Validation was static only under the review contract: no builds, tests, sanitizers, benchmarks, formatting checks, service startup, or runtime validation were executed. git diff --check was clean.

Review delivery is separate from convergence. The final main-agent sweep found no accepted new finding and covered all four changed paths and all ledger dispositions. Convergence is false because the unchanged-byte independent final-review confirmations for rounds 2 and 3 could not be obtained after repeated runner interruptions; the verified review is still delivered at the bounded round limit.

Review did not converge after 3 rounds. This automated review cannot pass the review check.

At the three-round cap, the main static sweep verified all changed paths, existing-thread dispositions, and no new finding, but the required unchanged-byte final reviewer confirmations were lost to repeated runner interruptions/model-capacity exits. The existing P2 NULL-ancestor thread is known context and was not duplicated; no additional unverified candidate remains.

@morningman

Copy link
Copy Markdown
Contributor Author

run buildall

@hello-stephen

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

------ Round 1 ----------------------------------
============================================
q1	17616	5132	5058	5058
q2	2136	341	261	261
q3	10212	1409	828	828
q4	4748	467	338	338
q5	7453	798	535	535
q6	179	185	149	149
q7	761	802	613	613
q8	9253	1370	1084	1084
q9	5624	4225	4188	4188
q10	6826	1307	1009	1009
q11	427	247	226	226
q12	649	408	284	284
q13	18067	3592	3091	3091
q14	272	266	247	247
q15	q16	722	723	691	691
q17	1201	764	587	587
q18	7030	6202	6141	6141
q19	1113	961	621	621
q20	391	342	228	228
q21	4897	2158	2315	2158
q22	348	295	259	259
Total cold run time: 99925 ms
Total hot run time: 28596 ms

----- Round 2, with runtime_filter_mode=off -----
============================================
q1	5633	5619	5619	5619
q2	674	596	543	543
q3	5168	5391	4912	4912
q4	2036	2179	1617	1617
q5	5274	5170	5533	5170
q6	257	223	162	162
q7	2392	2078	1888	1888
q8	3086	2698	2738	2698
q9	8135	8087	7927	7927
q10	3835	3681	3282	3282
q11	631	457	411	411
q12	743	714	536	536
q13	3568	3905	3334	3334
q14	309	324	293	293
q15	q16	744	719	651	651
q17	8485	7129	6871	6871
q18	13049	12183	12964	12183
q19	927	788	782	782
q20	2187	2168	1941	1941
q21	5681	4671	4779	4671
q22	483	446	395	395
Total cold run time: 73297 ms
Total hot run time: 65886 ms

@hello-stephen

Copy link
Copy Markdown
Contributor
TPC-DS: Total hot run time: 152030 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 6c1fd8f8ab037fba68da24861bb618d5d3cdab9a, data reload: false

query5	4310	614	467	467
query6	445	220	209	209
query7	4794	457	225	225
query8	315	170	160	160
query9	8801	3243	3251	3243
query10	491	310	267	267
query11	6043	3885	3609	3609
query12	149	93	92	92
query13	1264	445	366	366
query14	6012	3931	3666	3666
query14_1	3406	3487	3471	3471
query15	206	193	182	182
query16	945	427	394	394
query17	823	653	519	519
query18	2414	414	309	309
query19	191	160	128	128
query20	85	82	83	82
query21	204	127	109	109
query22	12974	13120	12851	12851
query23	13650	12974	12499	12499
query23_1	12576	12518	12550	12518
query24	7395	817	464	464
query24_1	505	490	499	490
query25	542	427	376	376
query26	1270	266	145	145
query27	2753	450	276	276
query28	4602	2075	2058	2058
query29	1589	612	476	476
query30	311	231	188	188
query31	1022	979	824	824
query32	142	94	89	89
query33	539	313	257	257
query34	894	860	508	508
query35	756	774	636	636
query36	813	821	726	726
query37	134	101	92	92
query38	1776	1746	1675	1675
query39	806	800	757	757
query39_1	738	742	750	742
query40	237	130	100	100
query41	74	69	68	68
query42	90	86	86	86
query43	348	368	314	314
query44	1353	698	715	698
query45	188	180	167	167
query46	848	968	569	569
query47	3023	2952	2508	2508
query48	313	318	230	230
query49	592	421	304	304
query50	691	284	218	218
query51	10507	10448	10425	10425
query52	82	82	78	78
query53	189	208	156	156
query54	262	210	193	193
query55	72	72	69	69
query56	249	240	219	219
query57	1671	1643	1594	1594
query58	304	278	275	275
query59	2191	2176	1974	1974
query60	317	248	252	248
query61	174	168	175	168
query62	401	343	311	311
query63	195	164	163	163
query64	2814	968	793	793
query65	3357	3280	3300	3280
query66	1774	439	307	307
query67	20313	20224	20174	20174
query68	3003	1000	623	623
query69	389	279	250	250
query70	860	781	753	753
query71	304	240	215	215
query72	2730	2302	2349	2302
query73	521	577	301	301
query74	4684	4550	4353	4353
query75	2352	2313	1925	1925
query76	2324	1038	599	599
query77	359	379	258	258
query78	9928	9900	9262	9262
query79	1000	877	529	529
query80	809	451	337	337
query81	576	321	273	273
query82	574	132	107	107
query83	298	198	192	192
query84	318	129	109	109
query85	849	448	364	364
query86	362	241	235	235
query87	1987	1975	1836	1836
query88	3831	2802	2788	2788
query89	339	278	234	234
query90	1784	171	169	169
query91	170	154	132	132
query92	105	87	85	85
query93	1000	988	577	577
query94	498	304	262	262
query95	577	401	316	316
query96	696	487	236	236
query97	2452	2458	2323	2323
query98	162	161	151	151
query99	766	756	646	646
Total cold run time: 235702 ms
Total hot run time: 152030 ms

@hello-stephen

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

query1	0.01	0.01	0.00
query2	0.09	0.05	0.06
query3	0.26	0.09	0.09
query4	1.61	0.11	0.11
query5	0.25	0.25	0.25
query6	1.17	0.70	0.68
query7	0.04	0.01	0.01
query8	0.06	0.04	0.04
query9	0.42	0.36	0.37
query10	0.60	0.62	0.63
query11	0.20	0.14	0.14
query12	0.19	0.15	0.15
query13	0.49	0.51	0.51
query14	1.03	1.01	1.01
query15	0.63	0.60	0.62
query16	0.37	0.33	0.35
query17	1.16	1.18	1.14
query18	0.19	0.20	0.22
query19	2.13	1.90	2.06
query20	0.02	0.02	0.01
query21	15.48	0.18	0.13
query22	4.86	0.05	0.05
query23	16.27	0.33	0.12
query24	3.12	0.63	0.39
query25	0.11	0.05	0.07
query26	1.21	0.21	0.16
query27	0.06	0.04	0.04
query28	3.69	0.86	0.39
query29	12.49	4.10	3.25
query30	0.29	0.13	0.09
query31	2.76	0.56	0.33
query32	3.22	0.60	0.50
query33	3.25	3.18	3.29
query34	15.38	4.06	3.40
query35	3.32	3.37	3.36
query36	0.55	0.45	0.44
query37	0.09	0.07	0.06
query38	0.05	0.04	0.03
query39	0.04	0.03	0.02
query40	0.16	0.14	0.14
query41	0.08	0.04	0.03
query42	0.04	0.03	0.03
query43	0.05	0.04	0.03
Total cold run time: 97.49 s
Total hot run time: 24.42 s

@morningman

Copy link
Copy Markdown
Contributor Author

/review

@github-actions

Copy link
Copy Markdown
Contributor

Codex automated review failed and did not complete.

Error: Selected model is at capacity. Please try a different model.
Workflow run: https://github.com/apache/doris/actions/runs/40842387016

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

@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 all four changed files and the related Arrow callers, nested SerDes, date packing/rounding, and shared Parquet consumers at head 6c1fd8f8ab037fba68da24861bb618d5d3cdab9a. No new nonduplicate findings.

The existing NULL-ancestor finding still applies to this exact head: nullable parent bitmaps are copied without masking child arrays, so a hidden out-of-range timestamp can fail an otherwise valid batch. The author explicitly requires #69581 to merge first; it was still open/unmerged when checked. Preserve that prerequisite and revalidate after rebasing, including the promised mixed-NULL STRUCT/LIST cases. I have not verified the author's combined-tree runtime results. This is already reported and is not reposted.

The existing calendar-validity finding is fixed: TIMESTAMPTZ retains a runtime is_valid_date() failure after packing, and both readers have year-zero February 29 negative tests. Both existing findings are P2; there are no independently confirmed existing P0/P1 comments to list as blockers.

Required checkpoint Conclusion
Goal and correctness Range checks precede narrowing and prevent year 65537 from wrapping to year 1. Calendar checks reject 0000-02-29. The known NULL-parent prerequisite above remains applicable.
Scope and clarity Focused changes to two reader implementations and two unit-test files; concrete failure examples explain the checks.
Concurrency and locks No new threads, shared mutation, locks, or lock ordering. Conversion uses caller-owned mutable columns.
Lifecycle and initialization New values/constants are local or constexpr; the existing UTC static is unchanged. No new ownership or cross-TU initialization dependency.
Configuration No configuration changes.
Compatibility No function signature, packed representation, storage/block format or rolling-upgrade protocol changes. No new libc API/dependency; final ELF/GLIBC requirements were not measured.
Thrift/protobuf No definition changes.
Parallel paths All Arrow units use the checks. TIMESTAMPTZ's shared Parquet callers already enforce a narrower 0001..9999 interval, preserving their accepted values. ORC's existing civil-year check and legacy DATE/DATETIME paths were compared; their pre-existing differences are unchanged.
Conditional checks and errors Conditions have reachable out-of-domain examples and explanatory comments. Runtime calendar validity remains essential inside the epoch interval. Callers propagate non-OK Status.
Test coverage New unit cases cover accepted endpoints, adjacent invalid epochs, year-wrap, year-zero leap day, a session-zone boundary and a visible ARRAY leaf. Existing tests cover units, negative fractions, direct NULL leaves and rounding. New tests omit mixed-NULL STRUCT/LIST parents and range-edge cases for every unit; no end-to-end execution was performed.
Test results Expected epoch literals and formatted values were reviewed. No golden results changed. Tests were inspected, not run.
Observability Conversion errors identify the source timestamp and type or microsecond value; existing Status propagation is sufficient for these deterministic failures.
Transactions and persistence No transaction, EditLog, failover or persistence changes.
Data writes and atomicity Changes affect decoding; errors abort the read rather than publish a successful partial batch. Parquet strict-error rollback is preserved. No new crash-sensitive resource ownership.
FE/BE variable propagation No new variables or transmitted metadata.
Performance DATETIMEV2 still performs one civil conversion per visible row; added checks are constant time and add no owning buffers or copies. No benchmark was run.
Other core invariants Column ownership/COW and memory tracking are unchanged; no visibility, version or delete-bitmap changes. No additional independently verified issue.

No additional user-provided focus was supplied. Static source review is complete; no builds, tests, runtime validation, binary audits or source modifications were performed. The combined #69581 tree and future merge ordering remain outside this exact-head validation.

Converged in 1 review round: both complete-review agents and the separate risk-focused agent returned NO_NEW_VALUABLE_FINDINGS; every changed hunk and risk focus was reviewed, and all candidates were verified as fixed, already covered, or dismissed with evidence. No unresolved new review question remains.

Review converged after 1 round(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.56% (36381/46906)
Line Coverage 62.82% (411198/654577)
Region Coverage 59.24% (347914/587276)
Branch Coverage 60.17% (161320/268092)

@morningman

Copy link
Copy Markdown
Contributor Author

run external

@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.56% (36381/46906)
Line Coverage 62.82% (411196/654577)
Region Coverage 59.24% (347898/587276)
Branch Coverage 60.17% (161314/268092)

@morningman
morningman merged commit e4c2dc2 into apache:master Nov 3, 2026
45 checks passed
@github-actions

github-actions Bot commented Nov 3, 2026

Copy link
Copy Markdown
Contributor

PR approved by anyone and no changes requested.

@github-actions github-actions Bot added the approved Indicates a PR has been approved by one committer. label Nov 3, 2026
@github-actions

github-actions Bot commented Nov 3, 2026

Copy link
Copy Markdown
Contributor

PR approved by at least one committer and no changes requested.

morningman added a commit to morningman/doris that referenced this pull request Nov 5, 2026
…output block

### What problem does this PR solve?

Issue Number: None

Related PR: apache#69581

Problem Summary:

Reproduction: load one large Arrow IPC batch through Arrow stream load with
a nested column that has NULL rows. For example, take 2^24 LIST<INT> rows
that alternate between [1] and NULL (2^23 child values), read in blocks of
4096 rows. Every block allocates a 1 MiB validity bitmap, zero-fills it and
copies the child's validity into it, so the batch allocates 4 GiB and writes
8 GiB from the query's Arrow pool. A STRUCT field costs N^2/B bits per batch
the same way. So does a NULL list row whose producer left values in its
span.

Root cause: `mask_arrow_children_hidden_by_null_parents()` was added earlier
in this PR so that element SerDes never decode child values hidden by a NULL
list, fixed-size-list, map or struct row. At the first NULL parent row of the
requested range, it allocated a bitmap covering the whole child and copied
the child's validity, before it looked at that row's child span. A NULL row
that owns no slots, as Arrow's list and map builders write it, or only NULL
slots, hides nothing but paid the same. `ArrowStreamReader` keeps each IPC
batch and hands the SerDes one row range per output block, and the mask is
thrown away after each call, so the bitmap was rebuilt for every block. The
map SerDe builds one for the keys and one for the items, and the struct
SerDe one per field. Since the bitmaps come from ExecEnv's accounted Arrow
pool, these temporary allocations also count against the query's memory
limit.

Fix:

1. `mask_arrow_children_hidden_by_null_parents()` reads a NULL row's child
   span first and skips the row when the span is empty or has no set
   validity bit in [offset + begin, offset + end). A child without a validity
   buffer counts as all valid. The bitmap is allocated at the first row that
   really hides a valid value. The check, `has_valid_slot()`, is now shared
   with `mask_unreferenced_slots()`, the slot mask of the Lance path. A NULL
   row that hides a valid value is still masked, and its bitmap still comes
   from ExecEnv's Arrow pool. Offsets are still read through the SerDes'
   unaligned loads, still after their offset validation, and an Arrow Null
   child is still returned as it is.
2. `ArrowStreamReader::_prepare_arrays()` masks each column of a retained
   batch once, after the existing normalization and cast. It uses
   `mask_arrow_descendants_not_visible()` with the reader's accounted pool.
   The per-block SerDe calls then find nothing left to hide and allocate
   nothing. If this mask fails, the error names the column the way the
   per-block errors do. It rejects only offsets that reach outside the child
   or run backwards at a NULL row. `enable_arrow_input_validation` (on by
   default) rejected those batches as well, in the block that read them.

What remains: a MapArray cannot hold NULL keys, so the per-batch mask leaves
the keys alone. A NULL map row that still owns entries therefore costs each
block a key bitmap. Arrow's map builder writes NULL map rows without
entries. The other Arrow readers (ADBC, remote Doris, Lance, the Arrow block
convertor, Python UDTF) read each batch with one SerDe call, so they pay for
the mask at most once per batch. paimon-rust reads row ranges of batches
about the size of a block.

Before: every block of a large nested batch with a NULL row allocated and
filled a bitmap the size of the whole child. After: the blocks allocate
nothing from the SerDes' pool, and each batch is masked at most once per
nested level. In the example above nothing is allocated at all, because
the NULL rows own no values.

Also covered: the NULL-parent case of the DATETIMEV2 / TIMESTAMPTZ range
checks from apache#69697. That PR merged before this one and its tests use visible
values only.

master, branch-4.1 and branch-4.2 have no parent-null masks (this PR adds
them), so nothing needs porting.

### Release note

None

### Check List (For Author)

- Test: Unit Test
    - New: `ArrowParentNullsTest.AllocatesNothingForNullRowsThatHideNothing`
      covers LIST rows that alternate [1] and NULL, read two rows per call; a
      NULL MAP row without entries (keys and items); and LIST and
      FIXED_SIZE_LIST NULL rows over NULL slots. Each returns the child
      itself and allocates nothing from the Doris Arrow pool. A NULL row over
      valid values next to them is still masked, with one bitmap.
    - New: `LoadArrowPipeTest.Null{List,FixedSizeList,Map,Struct}BatchReadsInBlocksWithoutPerBlockMasks`
      read one IPC batch of 64 rows four rows per block through
      `ArrowStreamReader`, with every fourth row NULL. No block allocates
      from the pool the SerDes mask with, and every row reads as one SerDe
      call over the whole batch reads it.
    - New: `DataTypeDateTimeV2SerDeTest.IgnoresArrowTimestampsHiddenByNullParents`
      hides 65537-01-01 under a NULL LIST, FIXED_SIZE_LIST, MAP value and
      STRUCT row, as DATETIMEV2 and as TIMESTAMPTZ. It reads as a NULL
      element, while the same value in a visible row fails with the range
      error.
    - Done: `run-be-ut.sh --run --filter=...` (ASAN, macOS arm64) over 24 suites:
      `ArrowParentNullsTest`, `LoadArrowPipeTest`, the DATETIMEV2 / TIMESTAMPTZ,
      Arrow, Arrow validation, array, map and struct SerDe suites,
      `ArrowColumnToDorisColumnTest`, `ArrowBlockConvertorTest`, `AdbcReaderTest`,
      `RemoteDorisV2ReaderTest`, `PaimonRustTableReaderTest` and `Lance*`: 328 of
      328 pass. clang-format 16 and `check-build-hygiene.sh` pass.
    - Mutations: allocating at the first NULL row of each call again (B1 off)
      fails the new `ArrowParentNullsTest` case and the four new
      `LoadArrowPipeTest` cases; dropping the per-batch mask's result (B2 off)
      fails the LIST and STRUCT `LoadArrowPipeTest` cases (the reader casts a
      FIXED_SIZE_LIST to a compacted LIST, and a NULL map row owns no entries,
      so B1 covers those two); returning the child unmasked fails the new
      timestamp case, four `ArrowParentNullsTest` cases and
      `LanceTableReaderTypeTest.IgnoresDate32PayloadHiddenByNullParents`.
- Behavior changed: No (the values read are unchanged; only the
  allocations change)
- Does this need documentation: No

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

Labels

approved Indicates a PR has been approved by one committer. backport/4.2-master reviewed

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants