branch-4.1: [fix](arrow) correct the Flight SQL GetTables schema and the TIMESTAMPTZ arrow reader #66344 - #66355
Open
github-actions[bot] wants to merge 1 commit into
Open
branch-4.1: [fix](arrow) correct the Flight SQL GetTables schema and the TIMESTAMPTZ arrow reader #66344#66355github-actions[bot] wants to merge 1 commit into
github-actions[bot] wants to merge 1 commit into
Conversation
…PTZ arrow reader (#66344) ### What problem does this PR solve? Related Issue: #65615 Problem Summary: Two places where Doris says one thing about an Arrow value and does another. Both were found while reading Doris over Arrow Flight SQL, and both reproduce on master on their own. **1. `CommandGetTables` describes types the batches do not carry** `FlightSqlSchemaHelper.getArrowType` is documented as mirroring `convert_to_arrow_type` in the backend -- the schema a client reads from `GetTables` is what it types its columns by. Two of the mappings had drifted from what BE actually writes: - A `DATEV2` column was described as `Date(MILLISECOND)` -- Arrow date64 -- while BE writes `arrow::Date32Type`, a day number. A client that trusts the schema renders and compares the column as a datetime and then fails on the first batch with `not support convert to datetimev2 from arrow type: 16`. - A complex column was described with a placeholder child: an array's element type was the Null type, a map's pair was a bare list, a struct had no fields at all. An Arrow ARRAY/MAP/STRUCT carries its element types in its children and nowhere else, so this said "array of nothing" about every array in the catalog while BE emitted `ListType(item)`, `MapType(key, value)` and `StructType(fields)` in the data. `describeTables` already reports the whole tree -- `Column.createChildrenColumn` even names an array's element `item` and a map's pair `key`/`value`, which is what Arrow calls them -- so the children are now built from it, recursively. A descriptor that reports no children keeps the old placeholders: a source that cannot describe its nested types is no worse off than before. **2. A TIMESTAMPTZ column read from Arrow was decoded by copying its bits** `DataTypeTimeStampTzSerDe` never overrode `read_column_from_arrow`, so the one it inherited from `DataTypeNumberSerDe<TYPE_TIMESTAMPTZ>` ran: its fixed-width path `memcpy`s the array's buffer straight into the column. Arrow hands it int64 EPOCH values; the column stores PACKED date/time values. Both are eight bytes wide, so the width check passes, the scan succeeds, and every row is silently wrong -- a Doris datetime read back over Arrow Flight SQL rendered as `+08:05` with no date in it. This is reachable today through the `remote_doris` catalog. Its schema is the remote table's `Column` objects deserialized verbatim (`RemoteDorisRestClient.parseColumns`), so a remote TIMESTAMPTZ column stays TIMESTAMPTZ locally; `RemoteDorisScanNode` plans the scan as `FORMAT_ARROW`; and `remote_doris_reader` materializes it through this serde. The remote side writes the epochs with the serde's own `write_column_to_arrow`, so both halves of that round trip are Doris code and the mismatch is entirely internal. The other Arrow readers -- `arrow_stream_reader` for the `arrow` file format, `python_udtf_function`, `paimon_cpp_reader` -- share the same serde and are covered by the same change; whether each of them can present a TIMESTAMPTZ column today is not something this PR claims either way. The reader converts by unit into the microseconds the column stores. NANO is divided rather than refused, since no Doris datetime type keeps sub-microsecond digits, and the division floors so a pre-1970 instant does not move forward by one microsecond. The value is read as an instant on the UTC line, the inverse of what `write_column_to_arrow` emits, so the round trip is exact. Two cases are deliberate. A null slot is not converted: the bytes under it are whatever the source left there, and running a garbage epoch through the range check would fail a batch whose rows are all well-formed. An Arrow type this serde cannot decode is an error rather than a fallback -- accepting anything eight bytes wide is how the corruption above stayed invisible.
Contributor
|
Thank you for your contribution to Apache Doris. Please clearly describe your PR:
|
Contributor
|
run buildall |
Contributor
BE Regression && UT Coverage ReportIncrement line coverage Increment coverage report
|
Contributor
FE Regression Coverage ReportIncrement line coverage |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Cherry-picked from #66344