Skip to content

[fix](arrow) Reject invalid UTF-8 and out-of-range timestamps in Flight results - #69771

Merged
morningman merged 3 commits into
apache:masterfrom
morningman:b2m-t45-flight-result-validation
Oct 29, 2026
Merged

morningman merged 3 commits into
apache:masterfrom
morningman:b2m-t45-flight-result-validation

Conversation

@morningman

@morningman morningman commented Oct 28, 2026 •

Copy link
Copy Markdown
Contributor

What problem does this PR solve?

Issue Number: None

Related PR: #68596, #68604

Problem Summary:

In short

This ports branch-4.x's #68596 and #68604 to master. An Arrow Flight result can carry values that Arrow clients cannot decode: a STRING value that is not valid UTF-8 (for example unhex('84')), and a DATETIME outside 0001-01-01..9999-12-31 (Doris's own range starts at 0000-01-01 00:00:00). The Arrow writers accept both, so the BE publishes the batch and the failure surfaces later in the client: PyArrow rejects the string, and raises OverflowError when it converts the timestamp. This PR checks every batch in the Flight convertor before it is handed out and fails the query with a server-side error that names the column, the nested path and the row. It also makes a failed _query_plan scan of the Spark and Flink connectors report its error: the BE used to end such a scan as a complete result, so a rejected batch would have become silently missing rows.

Background

  • ArrowFlightArrowBlockConvertor (be/src/format/arrow/arrow_block_convertor.h) builds the Arrow batches of an Arrow Flight result: ArrowFlightBatchReader uses it for Flight SQL and ADBC clients, which also covers a remote Doris catalog with use_arrow_flight = true, and the memory scratch sink uses it for the _query_plan scan API of the Spark and Flink Doris connectors.
  • The STRING serde appends the bytes of each value to an Arrow utf8 (or large_utf8) builder without looking at them. Arrow requires these values to be valid UTF-8, but only Array::ValidateFull() checks it; building and publishing the array does not.
  • The memory scratch sink queues the batches that get_next of the BE Thrift scanner hands to those connectors, and ends the queue with a nullptr sentinel. Its close() put the sentinel whatever the execution status, and nothing has recorded a status in the queue since the non-pipeline engine was removed ([refactor](pipeline) Delete non-pipeline exec nodes #35803), so a fragment that failed (a scan error, a cancellation) ended the scan as if it were complete.
  • DATETIME(p) is published as timestamp[s], timestamp[ms] or timestamp[us] by its scale. Arrow accepts any int64 there, so a year-0 value such as 0000-12-31 00:00:00 (-62135683200000000 us) is published as it is, while Python's datetime, which PyArrow converts to, only covers years 1..9999.

The problem, and what it cost

Result value Before this PR
unhex('84'), unhex('C0AF'), a surrogate or a code point above U+10FFFF, also inside an ARRAY / STRUCT / MAP key or value Published as is; the client fails decoding the batch, after earlier batches were already delivered
Two rows unhex('C2') and unhex('A2') Published; each row is invalid UTF-8 on its own, though the concatenated buffer is valid
CAST('0000-12-31 00:00:00' AS DATETIME(6)), or a zero date read from a table or an INT96 Parquet file Published as -62135683200000000; PyArrow raises OverflowError converting it
A _query_plan scan (Spark / Flink connector) whose fragment fails get_next reports the end of the scan after the rows fetched so far; with the validation below, a table with one invalid UTF-8 row would read zero rows and no error

The query looks successful on the server, so neither the error nor the offending column shows up in Doris; the Flight client gets a failure that does not say which value caused it.

How this PR fixes it

  • ArrowBlockConvertor::convert_to_arrow becomes virtual, and ArrowFlightArrowBlockConvertor overrides it: after the base conversion, it runs ValidateFull() on every column and returns InvalidArgument("Invalid Arrow Flight result in column <n> ('<name>'): ...") on failure. ValidateFull() checks each value, nested children included, so the row boundaries of the C2 / A2 case are respected. Binary columns keep their bytes; NULL values are skipped through the validity bitmap.
  • A new FlightTimestampValidator walks each column recursively (list values, map keys and values, struct fields), reads the actual timestamp unit, and checks each value against 0001-01-01..9999-12-31, also against the local calendar when the Arrow type has a time zone. Values under a NULL list / map / struct parent are not observable and are not checked. A value outside the range returns InvalidArgument("Arrow Flight timestamp in column '<path>' at row <n> is outside the supported 0001-9999 range (type: ...)"), with the row counted in the block.
  • For a timestamp with a time zone, the local bounds are computed with the exact offsets (cctz, Python's zoneinfo) and with the minute-rounded offsets of pytz, which PyArrow uses for zone names when it is installed ((offset + 30) // 60 * 60), and the stricter bound wins. pytz turns America/Chicago's local mean time -05:50:36 into -05:51, so 0001-01-01 05:50:36Z, local 00:00:00 for cctz, would be 0000-12-31 23:59:36 in PyArrow; the lower bound there is 05:51:00Z. In such zones up to 30 seconds at the start of year 1 are rejected.
  • MemoryScratchSinkLocalState::close records a failed execution status in the result queue before it puts the sentinel, and ResultQueueMgr::fetch_result checks the queue status again when it takes the sentinel, because a fetch already waiting for data passed the first check before the failure. get_next then returns the error to the connector.
  • Of the Arrow convertors, only the Flight convertor changes. The ordinary Doris Arrow convertor (Python UDFs) and the table-format convertors (Parquet, Iceberg, Paimon writers) keep their current behavior; DorisArrowBlockConvertor still publishes the same values in the new tests.
  • The code is branch-4.2's final form of the two PRs; master's Flight convertor already overrides init() and write_column(), and the new override sits next to them.

Results

Query over Arrow Flight Before After
SELECT unhex(encoded) AS payload with an invalid UTF-8 row Batch published, client fails decoding Query fails: Invalid Arrow Flight result in column 1 ('payload'): Invalid: Invalid UTF8 sequence at string index 0
SELECT array(unhex(encoded)) AS nested ... Batch published ... ('nested'): Invalid: List child array invalid: Invalid: Invalid UTF8 sequence at string index 0
SELECT CAST(value AS DATETIME(6)) AS event_time over 0000-12-31 00:00:00 Batch published, client fails converting Query fails: Arrow Flight timestamp in column 'event_time' at row 1 is outside the supported 0001-9999 range (type: timestamp[us]); nested values report paths such as nested[], nested.key, nested.child
Remote Doris catalog over Flight reading a 0000-01-01 00:00:00 DATETIME Value returned Arrow Flight timestamp in column 'datetime_0' at row 1 is outside the supported 0001-9999 range (type: timestamp[s])
_query_plan scan through open_scanner / get_next over a table with an invalid UTF-8 row 0 rows, then the end of the scan get_next returns [INVALID_ARGUMENT]Invalid Arrow Flight result in column 2 (''): Invalid: Invalid UTF8 sequence at string index 1
The same scan over a 0000-12-31 00:00:00 DATETIME 0 rows, then the end of the scan Arrow Flight timestamp in column '' at row 2 is outside the supported 0001-9999 range (type: timestamp[us])
Valid Unicode text, NULLs, binary bytes, 0001-01-01, 9999-12-31 23:59:59.999999 Returned Returned unchanged

Behavior change: a Flight query (Flight SQL, ADBC, a remote Doris catalog with use_arrow_flight = true, or a _query_plan read of the Spark / Flink connector) whose result contains a STRING value that is not valid UTF-8, or a DATETIME outside years 0001..9999, now fails with the error above instead of returning data. A remote Doris catalog that used to round-trip the zero date now reports this error; the three remote Doris suites that inserted 0000-01-01 00:00:00 as their minimum value now use 0001-01-01 00:00:00, and test_remote_doris_all_types_select asserts the zero-date error separately. A _query_plan read whose fragment fails for any reason, also a scan error or a cancellation, now gets the error from get_next instead of ending as a complete result.

Tests:

  • BE UT ArrowBlockConvertorTest: six new cases for invalid UTF-8 (four malformed encodings, per-row boundaries across a slice and the next batch, nested array / struct / map key / map value, large strings, bytes under a NULL), and valid text, NULLs and binary.
  • BE UT ArrowFlightTimestampTest (new file): zero date, year 0 and year 10000 at scales 0 / 3 / 6, calendar boundaries and pre-epoch fractions, slices and later batches, nested values and map keys, UTC and zoned bounds, the America/Chicago lower bound with pytz's rounded offset, naive bounds that do not depend on the session time zone, and values under NULL parents; the ordinary Doris convertor still publishes the same values.
  • BE UT ResultQueueMgrTest: a fetch already waiting for data reports a failure recorded before the sentinel; an OK status keeps a normal end.
  • Regression arrow_flight_sql_p0/test_query_plan_scan_validation reads _query_plan scans the way the connectors do (FE _query_plan, then BE open_scanner / get_next over Thrift): a valid scan reads all rows, and the invalid UTF-8 and year-0 scans get the errors above.
  • Regression arrow_flight_sql_p0/test_flight_utf8_validation, test_flight_timestamp_range (both red before this PR on master, green with it), test_flight_int96_timestamp_range (reads the INT96 fixtures of group4 over HDFS; needs the Hive docker environment), and the three remote Doris suites.

Release note

Arrow Flight results now fail with a server-side error when a STRING value is not valid UTF-8 or a DATETIME value is outside 0001-01-01..9999-12-31, instead of returning data that clients such as PyArrow cannot decode. This also applies to remote Doris catalogs that read over Arrow Flight and to the _query_plan reads of the Spark and Flink connectors, which now also get the error of any failed scan instead of a result that ends early.

Check List (For Author)

  • Test: Regression test / Unit Test
    • BE UT (ASAN, macOS): ArrowBlockConvertorTest, ArrowFlightTimestampTest, DataTypeSerDeArrowTest, ArrowFlightVariantTest, ArrowSchemaConvertorTest, ArrowRowBatchMetadataTest: 75 tests pass; after the review fixes ResultQueueMgrTest, ArrowFlightTimestampTest, ArrowBlockConvertorTest, ExternalScanContextMgrTest: 37 tests pass. Mutations: dropping the second queue-status check turns the waiting-fetch test red; bounding with the exact offset turns the America/Chicago test red. Mutations: dropping the ValidateFull() result turns the four invalid-UTF-8 tests red; dropping the timestamp check turns the four out-of-range tests red. IgnoresTimestampsMaskedByNullParents guards against false positives and stays green either way, because master's serdes write NULL parents with AppendNull().
    • Regression on a local cluster: test_flight_utf8_validation and test_flight_timestamp_range fail without this change and pass with it; test_query_plan_scan_validation fails before the queue-status fix (the invalid UTF-8 scan reads 0 rows and no error) and passes with it; test_remote_doris_all_types_select, test_remote_doris_statistics (both .out files regenerated with the scripts) and test_remote_doris_table_stats pass
    • Not run locally: test_flight_int96_timestamp_range (needs enableHiveTest and the Hive docker HDFS fixtures)
    • clang-format 16 and clang-tidy pass on the changed files; build-support/check-build-hygiene.sh passes
  • Behavior changed: Yes. See above: Flight results with invalid UTF-8 strings or out-of-range DATETIME values fail on the server, and a failed _query_plan scan returns its error.
  • Does this need documentation: No

Original author: @Gabriel39

…ht results

### What problem does this PR solve?

Issue Number: None

Related PR: apache#68596, apache#68604

Problem Summary: Port branch-4.x's apache#68596 and apache#68604. An Arrow Flight
result could carry values that Arrow clients cannot decode, and the BE
published them anyway:

1. A STRING value that is not valid UTF-8, for example unhex('84'), also
   inside an ARRAY, a STRUCT or a MAP key or value. The STRING serde
   appends the bytes to an Arrow utf8 builder unchecked, and Arrow only
   verifies UTF-8 in Array::ValidateFull(), so PyArrow failed decoding the
   batch on the client.
2. A DATETIME outside 0001-01-01..9999-12-31, which Doris can hold: its
   range starts at 0000-01-01 00:00:00, and 0000-12-31 00:00:00 is
   published as -62135683200000000 us. Arrow accepts any int64 timestamp,
   while Python's datetime only covers years 1..9999, so PyArrow raised
   OverflowError converting it.

The server saw a successful query, and the client got an error that did
not say which value caused it.

ArrowBlockConvertor::convert_to_arrow is now virtual, and
ArrowFlightArrowBlockConvertor overrides it: after the base conversion it
runs ValidateFull() on every column, and a new FlightTimestampValidator
checks every timestamp, nested ones and map keys included, against the
supported calendar range in the actual Arrow unit (and against the local
calendar for zoned values), skipping values under a NULL list, map or
struct parent. Either failure returns InvalidArgument naming the column,
the nested path and the row, before the batch is handed out. The code is
branch-4.2's final form of the two PRs, placed next to the init() and
write_column() overrides master's Flight convertor already has. Other
Arrow convertors are unchanged.

### Release note

Arrow Flight results now fail with a server-side error when a STRING value
is not valid UTF-8 or a DATETIME value is outside 0001-01-01..9999-12-31,
instead of returning data that clients such as PyArrow cannot decode. This
also applies to remote Doris catalogs reading over Arrow Flight and to the
_query_plan reads of the Spark and Flink connectors.

### Check List (For Author)

- Test: Regression test / Unit Test
    - BE UT: ArrowBlockConvertorTest gains 6 tests, new
      ArrowFlightTimestampTest has 7; 75 tests in 6 Arrow suites pass
    - Regression: test_flight_utf8_validation and
      test_flight_timestamp_range fail without this change and pass with
      it; the three remote Doris suites use year 0001 as their minimum
      DATETIME (two .out files regenerated) and pass;
      test_flight_int96_timestamp_range needs the Hive docker environment
      and was not run locally
- Behavior changed: Yes. A Flight result with an invalid UTF-8 STRING or
  an out-of-range DATETIME fails on the server.
- Does this need documentation: No

Co-authored-by: Gabriel <[email protected]>
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.

Request changes: the new rejection can silently truncate Spark/Flink external scans (P1), and a named-zone boundary still reaches a PyArrow/pytz calendar overflow (P2). Both findings are inline and independently traced against head 5e729b1. There were no existing inline threads or P0/P1 findings to carry forward.

Reviewed all 12 changed files and their producers/consumers. The relevant flow is: output Block -> base Arrow conversion/materialized columns -> Flight validation -> local/remote Flight reader or memory scratch sink -> client. The scratch-sink error/EOF branch is the blocking gap.

Critical checkpoint conclusions:

  • Goal and proof: UTF-8 and timestamp rejection works along the ordinary BE Flight path by inspection, but the two reported cases keep the stated end-to-end goal incomplete. Added tests cover most intended cases; the two findings lack their relevant end-to-end cases.
  • Scope and clarity: implementation and fixture changes are focused on Flight output validation.
  • Concurrency and locking: validation adds no shared mutable state or locks. Existing asynchronous scanner consumption matters to P1, including fetches already blocked on the queue.
  • Lifecycle and memory safety: validator state and batch ownership are local and released on error; the output is assigned only after validation. No new global/static initialization dependency or ownership cycle was found.
  • Configuration: no configuration item is added or changed.
  • Compatibility and symbols/formats: output rejection intentionally changes client behavior, including remote Doris and connector scans. Ordinary/Python/table writers retain their converter paths. The new virtual method does not change a persisted or wire layout; no new libc API is introduced.
  • Thrift/protobuf: no definitions, field IDs or enum values change.
  • Parallel paths: local/remote BE Flight readers and scratch-sink scans were traced. Readers propagate errors; scratch scans have P1. FE-materialized results use UTF-8 vectors, not these typed timestamp arrays.
  • Special conditions and nulls: range units, exclusive upper bounds, slices, const materialization, null strings and null container masking match the reachable producers; named-zone client interpretation has P2.
  • Test coverage: BE tests and Flight/INT96/remote-Doris regressions cover malformed UTF-8, binary, nested/key values, nulls, calendar bounds, fractions, slices and later batches. Scanner error publication and pytz historical-zone boundaries need coverage.
  • Changed expected results: the datetime replacements and statistics minimum match the changed input values and existing fixture/golden correspondence. Generation provenance was not independently established.
  • Observability and errors: ordinary Flight errors carry column/path/row context and readers add reader context; P1 discards the error before the external client sees it.
  • Transactions and persistence: no transaction, EditLog, rowset or persistence logic changes.
  • Data writes and crash behavior: this is result conversion, with no stored-data mutation or visibility/version/delete-bitmap change; successful scan completeness is compromised by P1.
  • FE/BE variables: no new variable or execution-version negotiation is needed or added.
  • Performance: UTF-8/full validation and timestamp traversal add linear work in emitted data; subtrees without timestamps skip row traversal. No benchmark was run or throughput claim made.
  • Other BE checkpoints: no new hub-header dependency, PCH edit, gate-table exception, explicit instantiation family or test inclusion of a source .cpp was found.

User focus: no additional focus was supplied; the whole PR was reviewed. Static-only limits: no builds, tests, benchmark, live connector/client reproduction, fixture download, or final ELF compatibility audit was performed. Findings are established from the code paths and, for P2, the linked primary decoder/timezone sources. There are no unresolved candidates or unreviewed changed paths.

Review converged after 3 round(s).

FlightTimestampValidator validator;
RETURN_IF_ERROR(
validator.init(batch->column(i), batch->schema()->field(i)->name(), _timezone));
RETURN_IF_ERROR(validator.validate(0, batch->num_rows(), start_row));

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.

[P1] Propagate validation failures to external scan readers

MemoryScratchSinkOperatorX::sink_impl() also calls this converter for Spark/Flink _query_plan scans. With a stored year-zero DATETIMEV2 (or invalid UTF-8 STRING), the new validation error returns before that batch is queued. The pipeline then closes the sink, but MemoryScratchSinkLocalState::close() ignores exec_status and queues nullptr without updating the queue's status. ResultQueueMgr::fetch_result() interprets that sentinel as successful EOF, and BaseBackendService::get_next() has no separate query-status check; external fragments also skip FE status reporting. Consequently these scans now succeed with zero or partial rows, silently losing the rejected batch and remaining data. Propagate the failure through the external scanner's queue/get_next path, including a fetch already waiting for data, and add a scanner-level regression before enabling this rejection for that consumer.

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 3b3f2a6. The mechanism is right, and it is older than this PR: nothing has called ResultQueueMgr::update_queue_status since the non-pipeline engine was removed (#35803), so any failure on this path (a scan error, an expression error, a cancellation such as a memory-limit kill) already ended a _query_plan read as a complete result; the rejected batch is one more such failure. MemoryScratchSinkLocalState::close now records a non-OK exec_status in the queue before it puts the sentinel, and ResultQueueMgr::fetch_result checks the queue status again when it takes the sentinel, which covers a fetch already blocked in blocking_get.

Tests: BE UT ResultQueueMgrTest.fetch_result_reports_failure_recorded_before_sentinel (a fetch waiting for data) and fetch_result_end_after_ok_status; regression arrow_flight_sql_p0/test_query_plan_scan_validation reads _query_plan scans through open_scanner / get_next. On the previous head its invalid UTF-8 scan read 0 rows and ended with eos; now get_next returns [INVALID_ARGUMENT]Invalid Arrow Flight result in column 2 (''): Invalid: Invalid UTF8 sequence at string index 1, the year-0 scan returns the timestamp error, and a valid scan still reads all rows. branch-4.1 / 4.2 have the same close() and already ship this validation; that is passed on to their maintainers.

const auto local_end = cctz::convert(cctz::civil_second(10000, 1, 1), timezone);
// Python first constructs UTC and then applies the Arrow timezone. Both calendar
// representations must fit, including offsets at the first and last supported day.
min_seconds =

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] Account for PyArrow's historical timezone offsets at the lower bound

For time_zone='America/Chicago', a stored TIMESTAMPTZ(6) value 0001-01-01 05:50:36.000000 +00:00 produces -62135575764000000 us. The TZDB offset is -05:50:36, so this equals cctz's local_start and passes validation. However, PyArrow chooses pytz when installed, and pytz rounds historical offsets to minutes, giving -05:51 here. The inferred Python local datetime is then 24 seconds before year 1: TimestampScalar.as_py() converts UTC using astimezone and still raises OverflowError after this batch is published. The fixed-offset tests do not cover this named-zone case. Account for the supported decoder's timezone rules in the safe bound and add a named-zone boundary case; the zoneinfo path without pytz preserves seconds and is unaffected.

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 3489284. Agreed: pytz rounds every offset with (offset + 30) // 60 * 60, and PyArrow uses it for zone names when it is installed. The zoned bounds are now computed with both the exact and the pytz-rounded offsets at 0001-01-01 and 10000-01-01, and the stricter one wins; for America/Chicago the lower bound moves from 05:50:36Z to 05:51:00Z. Whole-minute offsets (UTC, fixed offsets, the year-9999 offsets) keep their bounds. For a zone whose local mean time rounds to a more negative offset, up to 30 seconds at the start of year 1 are now also rejected for zoneinfo clients; one bound that holds for both decoders is the trade-off we take. New UT ArrowFlightTimestampTest.ZonedLowerBoundHoldsWithPytzRoundedOffsets: the exact-offset start is rejected and 05:51:00Z is accepted; it fails when the bound uses the exact offset.

@hello-stephen

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

------ Round 1 ----------------------------------
============================================
q1	17634	5165	5039	5039
q2	2163	307	245	245
q3	10228	1363	826	826
q4	4684	463	339	339
q5	7481	800	535	535
q6	185	182	152	152
q7	758	803	589	589
q8	9264	1359	1085	1085
q9	5520	4183	4169	4169
q10	6836	1289	1032	1032
q11	420	247	225	225
q12	633	403	289	289
q13	18086	3595	3058	3058
q14	268	270	245	245
q15	q16	714	714	673	673
q17	1204	772	604	604
q18	6924	6123	6077	6077
q19	1103	944	596	596
q20	391	329	230	230
q21	4885	2173	2304	2173
q22	349	307	250	250
Total cold run time: 99730 ms
Total hot run time: 28431 ms

----- Round 2, with runtime_filter_mode=off -----
============================================
q1	5616	5560	5575	5560
q2	674	576	543	543
q3	5232	5369	4889	4889
q4	1988	2151	1563	1563
q5	5231	5155	5489	5155
q6	272	213	162	162
q7	2329	2024	1829	1829
q8	3068	2653	2665	2653
q9	7990	7990	7990	7990
q10	3736	3926	3220	3220
q11	613	454	433	433
q12	672	707	514	514
q13	3489	3914	3395	3395
q14	331	315	306	306
q15	q16	681	700	647	647
q17	8162	7040	6851	6851
q18	13020	12178	12750	12178
q19	911	803	790	790
q20	2125	2150	1925	1925
q21	5589	4614	4658	4614
q22	485	448	426	426
Total cold run time: 72214 ms
Total hot run time: 65643 ms

@hello-stephen

Copy link
Copy Markdown
Contributor
TPC-DS: Total hot run time: 151676 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 5e729b15afa07e4d521b2858a186d183ce540e73, data reload: false

query5	4336	571	446	446
query6	442	201	190	190
query7	4819	445	244	244
query8	336	168	152	152
query9	8802	3229	3268	3229
query10	466	320	258	258
query11	5911	3890	3612	3612
query12	139	89	87	87
query13	1258	455	334	334
query14	6146	3928	3686	3686
query14_1	3458	3381	3466	3381
query15	225	201	192	192
query16	940	423	413	413
query17	871	658	533	533
query18	2423	419	311	311
query19	207	166	135	135
query20	87	86	84	84
query21	212	126	108	108
query22	12982	13043	13031	13031
query23	13831	12809	12359	12359
query23_1	12579	12497	12525	12497
query24	7357	831	472	472
query24_1	493	484	487	484
query25	535	426	365	365
query26	1272	261	147	147
query27	2759	446	271	271
query28	4596	2032	2024	2024
query29	1552	609	465	465
query30	306	227	185	185
query31	1008	934	830	830
query32	143	98	93	93
query33	537	309	248	248
query34	903	866	514	514
query35	733	774	658	658
query36	865	827	715	715
query37	126	99	86	86
query38	1773	1741	1675	1675
query39	806	792	761	761
query39_1	730	731	755	731
query40	225	122	100	100
query41	72	69	68	68
query42	92	87	82	82
query43	343	367	308	308
query44	1341	712	718	712
query45	175	178	168	168
query46	833	973	584	584
query47	3005	2978	2901	2901
query48	289	310	218	218
query49	568	397	289	289
query50	680	288	214	214
query51	10382	10757	10246	10246
query52	83	77	70	70
query53	186	205	151	151
query54	246	188	187	187
query55	77	71	63	63
query56	225	219	223	219
query57	1679	1631	1574	1574
query58	280	261	250	250
query59	2171	2204	1992	1992
query60	283	255	222	222
query61	140	147	148	147
query62	395	331	288	288
query63	187	158	158	158
query64	2723	965	759	759
query65	3342	3290	3268	3268
query66	1762	431	334	334
query67	20275	20170	20035	20035
query68	3045	1020	646	646
query69	384	290	252	252
query70	841	792	785	785
query71	301	231	198	198
query72	2746	2577	2301	2301
query73	518	525	297	297
query74	4683	4535	4366	4366
query75	2322	2347	1926	1926
query76	2327	1016	589	589
query77	351	362	269	269
query78	9833	9711	9145	9145
query79	1015	857	541	541
query80	1237	445	345	345
query81	597	316	273	273
query82	580	131	103	103
query83	300	196	180	180
query84	320	126	112	112
query85	860	410	361	361
query86	397	244	227	227
query87	1986	1982	1821	1821
query88	3807	2777	2795	2777
query89	332	286	239	239
query90	1954	169	167	167
query91	163	154	127	127
query92	99	81	88	81
query93	1020	1007	579	579
query94	633	299	271	271
query95	579	380	302	302
query96	679	499	242	242
query97	2439	2524	2298	2298
query98	158	155	149	149
query99	770	774	640	640
Total cold run time: 236121 ms
Total hot run time: 151676 ms

@hello-stephen

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

query1	0.01	0.01	0.01
query2	0.09	0.06	0.04
query3	0.26	0.08	0.08
query4	1.61	0.11	0.10
query5	0.23	0.24	0.24
query6	1.18	0.68	0.68
query7	0.04	0.01	0.01
query8	0.06	0.04	0.04
query9	0.40	0.35	0.35
query10	0.61	0.60	0.61
query11	0.20	0.14	0.14
query12	0.18	0.15	0.14
query13	0.49	0.49	0.50
query14	1.02	1.02	1.02
query15	0.63	0.60	0.61
query16	0.36	0.36	0.34
query17	1.11	1.14	1.13
query18	0.21	0.19	0.20
query19	2.05	2.01	1.92
query20	0.02	0.01	0.02
query21	15.48	0.18	0.14
query22	4.95	0.05	0.05
query23	16.27	0.32	0.13
query24	3.08	0.47	0.36
query25	0.14	0.04	0.06
query26	1.04	0.21	0.14
query27	0.05	0.05	0.05
query28	3.92	0.81	0.40
query29	12.51	4.22	3.32
query30	0.28	0.13	0.11
query31	2.77	0.56	0.33
query32	3.22	0.60	0.51
query33	3.26	3.20	3.15
query34	15.43	4.03	3.43
query35	3.31	3.33	3.33
query36	0.57	0.47	0.43
query37	0.10	0.07	0.06
query38	0.05	0.04	0.03
query39	0.04	0.03	0.02
query40	0.18	0.15	0.15
query41	0.08	0.03	0.03
query42	0.04	0.03	0.03
query43	0.04	0.03	0.03
Total cold run time: 97.57 s
Total hot run time: 24.4 s

morningman and others added 2 commits October 29, 2026 00:14
### What problem does this PR solve?

Issue Number: None

Related PR: apache#69771

Problem Summary: The Spark and Flink Doris connectors read a table through
FE's _query_plan API and the BE Thrift scanner (open_scanner / get_next),
which drains the queue that MemoryScratchSink fills with Arrow batches.
When the fragment failed, the sink's close() still put the nullptr
sentinel and nothing recorded the failure: no code has called
ResultQueueMgr::update_queue_status since the non-pipeline engine was
removed (apache#35803), so RecordBatchQueue::status() stayed OK and get_next
reported eos. A scan error, a cancellation or, with this PR, a batch the
Arrow Flight convertor rejects (invalid UTF-8, a DATETIME before year 1)
ended the read normally with the rows fetched so far: a table with one
invalid UTF-8 row read zero rows and no error.

MemoryScratchSinkLocalState::close now records a failed exec_status in
the queue before the sentinel (update_status ignores OK), and
fetch_result checks the queue status again when it takes the sentinel,
because a fetch already waiting for data passed the first check before
the failure. get_next then returns the error, for example
"[INVALID_ARGUMENT]Invalid Arrow Flight result in column 2 (''): Invalid:
Invalid UTF8 sequence at string index 1".

### Release note

A failed read through the _query_plan scan API (Spark / Flink Doris
connectors) now returns the error instead of ending as a complete result.

### Check List (For Author)

- Test: Unit Test / Regression test
    - BE UT ResultQueueMgrTest: a fetch waiting for data reports a failure
      recorded before the sentinel; an OK status keeps a normal end.
    - Regression arrow_flight_sql_p0/test_query_plan_scan_validation reads
      _query_plan scans over the BE Thrift scanner: before this change the
      invalid UTF-8 scan read 0 rows and no error; with it, the UTF-8 and
      timestamp scans get the errors and a valid scan reads all rows.
- Behavior changed: Yes. Such scans fail instead of returning partial data.
- Does this need documentation: No

Co-Authored-By: Claude Opus 5.5 <[email protected]>
### What problem does this PR solve?

Issue Number: None

Related PR: apache#69771

Problem Summary: For an Arrow timestamp with a time zone, the Flight
validator bounded values so that the local calendar in the zone, computed
by cctz with the exact offsets, stays in 0001..9999. PyArrow resolves the
zone name with pytz when pytz is installed, and pytz rounds every UTC
offset to whole minutes. With time_zone 'America/Chicago', whose local
mean time is -05:50:36, the lower bound 0001-01-01 05:50:36Z is local
0001-01-01 00:00:00 for cctz and zoneinfo, but 0000-12-31 23:59:36 for
pytz (-05:51), so TimestampScalar.as_py() still raised OverflowError on a
published batch.

The local bounds are now also computed with the offsets rounded the way
pytz does ((offset + 30) // 60 * 60, floor division), and the stricter
bound wins: 05:51:00Z in that example. Bounds whose offsets are whole
minutes (UTC, fixed offsets, the offsets at year 9999) do not change; for
a zone whose local mean time rounds to a larger negative offset, up to 30
seconds at the start of year 1 are now rejected for every client.

### Release note

None

### Check List (For Author)

- Test: Unit Test
    - BE UT ArrowFlightTimestampTest.ZonedLowerBoundHoldsWithPytzRoundedOffsets:
      America/Chicago's exact-offset start is rejected and 05:51:00Z is
      accepted; the test fails when the bound uses the exact offset.
- Behavior changed: Yes. The values above are rejected.
- Does this need documentation: No

Co-Authored-By: Claude Opus 5.5 <[email protected]>
@morningman

Copy link
Copy Markdown
Contributor Author

Addressed the first review round in 3b3f2a6 and 3489284.

Per finding:

  • P1, failed _query_plan scans ended as complete results (r4400505759): fixed in 3b3f2a6. This predates the PR: nothing has written the result queue's status since the non-pipeline engine was removed ([refactor](pipeline) Delete non-pipeline exec nodes #35803), so any failure on that path ended the read at the rows fetched so far. The sink now records a failed execution status before the end-of-scan sentinel, and fetch_result checks the status again when it takes the sentinel. The new regression test_query_plan_scan_validation reads _query_plan scans through open_scanner / get_next: on the previous head the invalid UTF-8 scan read 0 rows without an error; now it gets the error.
  • P2, pytz's minute-rounded offsets at the zoned lower bound (r4400505772): fixed in 3489284. The zoned bounds also hold with pytz's rounded offsets; America/Chicago's lower bound moves from 05:50:36Z to 05:51:00Z, covered by a new BE UT.

The description now covers both changes, including the behavior change for _query_plan readers.

@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 all 16 changed files at 3489284: no new verified findings. Both earlier inline findings are repaired: 4400505759 now propagates external-scan failures, including a waiting fetch; 4400505772's America/Chicago boundary value is rejected. Neither is reposted, and no earlier P0/P1 remains blocking.

Critical checkpoint conclusions:

  • Goal and correctness: completed Flight batches are validated before publication; timestamp units, naive/zoned bindings, NULL parents, nested paths, const materialization and slice offsets agree with the actual schema/SerDes. Added unit and scanner regressions target the failures; they were inspected, not executed.
  • Scope and reuse: validation is confined to the Flight subclass; queue propagation reuses existing first-error-wins status handling. The changes are focused on result validation and error delivery.
  • Concurrency and locking: pipeline workers publish status before EOF; the Thrift fetch worker rechecks it after EOF. The queue map lock is released before blocking, the status mutex protects both reads and writes, and no new nested lock order or heavy locked work was found.
  • Lifecycle and memory: temporary batches/validator trees and shared queue ownership preserve lifetimes through failure. Existing task/reader memory contexts remain active; no new ownership cycle, thread, or cross-TU static initialization dependency was introduced.
  • Configuration: no new configuration or dynamic-update requirement.
  • Compatibility: the added virtual override is internal BE code; no external protocol, stored layout or Arrow schema changes were found. The documented behavior change is rejection of invalid Flight results and propagation of failed connector scans.
  • Thrift/protobuf IDs: no gensrc definitions changed.
  • Parallel paths: both local and remote Flight readers and the external scratch sink use validation. Ordinary Doris/Python and table-format converters retain their existing paths.
  • Conditional checks: comments explain UTC/local bounds, negative offset rounding, masked children and the waiting-fetch race. The actual external-plan whitelist excludes the speculative LIMIT/TopN/exchange termination case.
  • Test coverage: inspected six new UTF8 cases, eight timestamp cases, two queue cases, four new regression suites and three adjusted remote suites. Coverage includes malformed encodings, row boundaries, binary exemption, nested values, NULLs, fractions, slices, calendar boundaries, historical offsets, INT96 and scanner error delivery. Test discovery and scanner API/defaults match their producers.
  • Expected results: both changed .out files match the changed DATETIME input minima; separate zero-year rejection coverage remains. Output-generation provenance was not independently verified.
  • Observability: validation errors retain column/path/type and row details where applicable; existing reader, pipeline and get_next context identifies the failed query/fragment. No new metric requirement was found.
  • Transactions and persistence: outgoing result handling does not change visible-version, delete-bitmap, EditLog or failover logic.
  • Writes and atomicity: no production data-write or commit path changes.
  • FE/BE variables: no new variable or omitted transmission path.
  • Performance: added work is linear validation over returned text/timestamp-bearing branches plus schema-sized temporary metadata; queue transfer adds a small status check. No new batch copy or pathological complexity was found; performance was not measured.
  • Additional risks and user focus: dedicated reviews of timezone decoder behavior, nested representations, queue cancellation and test configuration found no additional verified issue. No additional user focus was supplied. Boundary assumptions were checked against PyArrow conversion and pytz rounding.

Converged in 1 round: all three reviewers returned NO_NEW_VALUABLE_FINDINGS, all changed files/checkpoints were covered, and every candidate and initial risk has a recorded disposition. No material unresolved source-code question remains. This was static review only: no builds, tests or benchmarks ran. The sleep-based waiting-fetch unit test does not deterministically force the interleaving on every execution, and arbitrary client/server timezone-database version combinations were not exhaustively validated.

Review converged after 1 round(s).

@hello-stephen

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

------ Round 1 ----------------------------------
============================================
q1	17603	5168	5053	5053
q2	2101	322	276	276
q3	10214	1365	806	806
q4	4675	455	330	330
q5	7426	787	520	520
q6	182	188	150	150
q7	758	782	613	613
q8	9263	1306	1045	1045
q9	5504	4204	4189	4189
q10	6818	1281	1036	1036
q11	427	240	227	227
q12	640	392	274	274
q13	18136	3559	3016	3016
q14	285	274	242	242
q15	q16	706	711	665	665
q17	1192	742	575	575
q18	6943	6149	6128	6128
q19	1103	953	604	604
q20	379	326	231	231
q21	5130	2369	2475	2369
q22	371	320	286	286
Total cold run time: 99856 ms
Total hot run time: 28635 ms

----- Round 2, with runtime_filter_mode=off -----
============================================
q1	5920	5871	5902	5871
q2	713	621	560	560
q3	5716	5734	5373	5373
q4	2020	2163	1568	1568
q5	5810	5828	5606	5606
q6	242	182	142	142
q7	2284	2029	1761	1761
q8	2818	2454	2467	2454
q9	7481	7525	7477	7477
q10	3620	3579	3152	3152
q11	579	403	394	394
q12	654	695	506	506
q13	3266	3647	3086	3086
q14	287	286	253	253
q15	q16	655	660	618	618
q17	7750	7036	6884	6884
q18	12979	12194	12821	12194
q19	921	795	793	793
q20	2123	2117	1934	1934
q21	5710	4563	4653	4563
q22	500	443	413	413
Total cold run time: 72048 ms
Total hot run time: 65602 ms

@hello-stephen

Copy link
Copy Markdown
Contributor
TPC-DS: Total hot run time: 150873 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 3489284b381c835339ac90f7df399566642a80da, data reload: false

query5	4307	582	438	438
query6	431	205	186	186
query7	4792	430	231	231
query8	317	161	154	154
query9	8773	3183	3185	3183
query10	477	301	254	254
query11	5990	3864	3541	3541
query12	146	92	88	88
query13	1248	450	341	341
query14	6143	3883	3626	3626
query14_1	3384	3384	3339	3339
query15	204	195	182	182
query16	944	439	401	401
query17	850	655	530	530
query18	2414	424	310	310
query19	213	167	133	133
query20	84	88	83	83
query21	212	128	104	104
query22	12997	12911	12889	12889
query23	13697	12787	12434	12434
query23_1	12508	12639	12493	12493
query24	7378	810	467	467
query24_1	488	474	469	469
query25	508	393	334	334
query26	1251	251	130	130
query27	2787	434	264	264
query28	4616	2023	2005	2005
query29	1543	584	446	446
query30	303	223	182	182
query31	974	949	806	806
query32	137	88	93	88
query33	516	290	232	232
query34	889	859	515	515
query35	736	757	652	652
query36	805	804	756	756
query37	124	95	80	80
query38	1746	1745	1670	1670
query39	789	785	746	746
query39_1	735	726	734	726
query40	214	111	91	91
query41	65	60	60	60
query42	86	81	82	81
query43	341	356	307	307
query44	1307	697	686	686
query45	191	178	163	163
query46	822	957	564	564
query47	3008	2865	2910	2865
query48	344	302	214	214
query49	572	415	289	289
query50	684	283	213	213
query51	10547	10449	10583	10449
query52	77	86	71	71
query53	182	204	156	156
query54	222	191	173	173
query55	68	68	61	61
query56	246	233	221	221
query57	1616	1626	1550	1550
query58	283	262	254	254
query59	2186	2192	1978	1978
query60	292	236	222	222
query61	147	152	134	134
query62	382	331	277	277
query63	186	152	165	152
query64	2733	928	755	755
query65	3307	3262	3248	3248
query66	1766	406	308	308
query67	20183	20321	19940	19940
query68	3051	1008	615	615
query69	372	298	253	253
query70	862	800	738	738
query71	299	242	211	211
query72	2679	2581	2294	2294
query73	505	524	295	295
query74	4605	4510	4298	4298
query75	2312	2269	1936	1936
query76	2324	997	592	592
query77	347	372	272	272
query78	9643	9671	9170	9170
query79	904	843	530	530
query80	478	421	347	347
query81	510	325	268	268
query82	205	128	104	104
query83	186	192	179	179
query84	289	118	97	97
query85	755	416	363	363
query86	255	239	234	234
query87	1949	1965	1806	1806
query88	3695	2771	2764	2764
query89	315	288	246	246
query90	2118	174	161	161
query91	160	158	130	130
query92	93	86	84	84
query93	985	965	577	577
query94	448	316	268	268
query95	575	338	368	338
query96	679	488	242	242
query97	2447	2429	2318	2318
query98	161	150	155	150
query99	746	771	661	661
Total cold run time: 233337 ms
Total hot run time: 150873 ms

@hello-stephen

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

query1	0.01	0.01	0.01
query2	0.10	0.05	0.05
query3	0.26	0.08	0.08
query4	1.62	0.10	0.11
query5	0.23	0.24	0.23
query6	1.17	0.67	0.68
query7	0.04	0.01	0.00
query8	0.05	0.04	0.04
query9	0.40	0.33	0.33
query10	0.63	0.57	0.61
query11	0.19	0.14	0.15
query12	0.18	0.15	0.15
query13	0.48	0.48	0.48
query14	1.03	1.01	1.00
query15	0.62	0.60	0.61
query16	0.35	0.30	0.37
query17	1.14	1.11	1.09
query18	0.20	0.20	0.19
query19	2.11	2.00	1.96
query20	0.02	0.01	0.01
query21	15.48	0.19	0.13
query22	4.92	0.05	0.06
query23	16.24	0.31	0.12
query24	2.81	0.49	0.39
query25	0.11	0.05	0.08
query26	1.09	0.21	0.16
query27	0.06	0.05	0.05
query28	3.46	0.74	0.34
query29	12.46	4.22	3.31
query30	0.28	0.15	0.11
query31	2.76	0.54	0.32
query32	3.22	0.60	0.49
query33	3.14	3.30	3.26
query34	15.50	4.03	3.41
query35	3.30	3.30	3.32
query36	0.56	0.44	0.42
query37	0.09	0.06	0.06
query38	0.05	0.04	0.03
query39	0.04	0.03	0.03
query40	0.18	0.15	0.14
query41	0.08	0.03	0.03
query42	0.03	0.03	0.03
query43	0.04	0.04	0.03
Total cold run time: 96.73 s
Total hot run time: 24.26 s

@hello-stephen

Copy link
Copy Markdown
Contributor

BE UT Coverage Report

Increment line coverage 97.79% (133/136) 🎉

Increment coverage report
Complete coverage report

Category Coverage
Function Coverage 67.12% (32439/48331)
Line Coverage 51.98% (343451/660697)
Region Coverage 47.41% (279057/588653)
Branch Coverage 49.11% (131712/268202)

@hello-stephen

Copy link
Copy Markdown
Contributor

BE Regression && UT Coverage Report

Increment line coverage 100.00% (136/136) 🎉

Increment coverage report
Complete coverage report

Category Coverage
Function Coverage 77.49% (36291/46836)
Line Coverage 62.72% (409964/653668)
Region Coverage 59.16% (346984/586525)
Branch Coverage 60.07% (160730/267550)

@morningman
morningman merged commit b340365 into apache:master Oct 29, 2026
46 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants