morningman opened a new pull request, #69771:
URL: https://github.com/apache/doris/pull/69771
### 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.
#### 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.
- `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 |
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.
- 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])` |
| 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.
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, 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.
- 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.
### Check List (For Author)
- Test: Regression test / Unit Test
- BE UT (ASAN, macOS): `ArrowBlockConvertorTest`,
`ArrowFlightTimestampTest`, `DataTypeSerDeArrowTest`, `ArrowFlightVariantTest`,
`ArrowSchemaConvertorTest`, `ArrowRowBatchMetadataTest`: 75 tests pass.
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_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.
- Does this need documentation: No
Original author: @Gabriel39
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
