morningman opened a new pull request, #68315:
URL: https://github.com/apache/doris/pull/68315
### What problem does this PR solve?
Issue Number: #67577
Related PR: #66344 (the `GetTables` schema fix whose nested-type tests move
here), #67966 / #68266 (the Flight SQL session work the next interfaces build
on)
Problem Summary:
Stage 2 of #67577, the one item of it that is done now (the rest waits for
the rework of the BE Arrow type layer): the Doris-to-Arrow type mapping FE uses
for the schemas it hands to Flight SQL clients gets a home of its own,
`org.apache.doris.arrow.DorisArrowTypeMapping`, with no value changed.
Today the only Arrow schema FE builds is the `table_schema` column of
`GetTables`, and the mapping behind it lived as private methods of
`FlightSqlSchemaHelper` (`getArrowType`, `columnDescToArrowType`, `buildField`,
`arrowChildren`, `createFlightSqlColumnMetadata`). The Flight SQL interfaces
that follow — the metadata commands, the result and parameter schemas of
prepared statements, `ExecuteSchema` — need the same mapping, and a client that
types its columns from one of those schemas must read the batches BE emits as
the type another promised. A second caller could only have copied the switch.
- `DorisArrowTypeMapping.toArrowType(PrimitiveType, precision, scale)`,
`toArrowType(TColumnDesc)` and `toField(dbName, tableName, TColumnDesc)` are
the moved methods, byte for byte: the switch, the nested-type rules for ARRAY /
MAP / STRUCT, the Flight SQL column metadata. `FlightSqlSchemaHelper` keeps the
`GetTables` plumbing and the schema serialization, and calls `toField`.
- The mapping is recorded as it stands in `DorisArrowTypeMappingTest`: one
row per `PrimitiveType`, plus one per precision / scale band where the band
picks the unit, and a check that every `PrimitiveType` has a row, so a new type
gets a deliberate mapping instead of the `default` branch. The first commit
adds the table against the old private method, the second moves the code; the
rows are identical in both, which is the proof that no value changed.
- The cells known to disagree with BE's `convert_to_arrow_type` are carried
over as they are and pinned by the test, on purpose: `GetTables` says
`timestamp[.., tz=UTC]` for TIMESTAMPTZ where BE stamps the session time zone,
and `Null` for TIMEV2 / VARBINARY / AGG_STATE where BE emits float64 / binary /
binary. As recorded in #67577 (Stage 2, deferred), they are corrected in one
step against a golden shared with BE once the BE Arrow type layer has been
reworked — in this one class, not one call site at a time. Please do not fix
them here.
- `FlightSqlSchemaHelperArrowTypeTest` splits along the same line: the type
and nesting cases move into `DorisArrowTypeMappingTest`, the two
serialized-schema round trips stay next to `getSerializedSchema` as
`FlightSqlSchemaHelperSerializedSchemaTest`.
- A new regression suite, `arrow_flight_sql_p0/test_get_tables_schema`,
records what reaches a client: a raw Flight SQL client asks
`GetTables(include_schema)` for three tables that together declare a column of
every type an internal table can hold, decodes each `table_schema` the way a
client does and compares it field by field, down to the leaves, with the Arrow
type, the nullability and the Flight SQL column metadata as served today — the
whole path from `describeTables`' descriptors through `DorisArrowTypeMapping`
to the serialization. The two kept-as-is cells reachable from an internal table
(the `UTC` zone, `Null` for AGG_STATE) are pinned there too and say so.
No behaviour change: `GetTables` returns the same bytes.
### Release note
None
### Check List (For Author)
- Test
- [x] Regression test
- [x] Unit Test
- [x] Manual test (add detailed scripts or steps below)
**Unit tests.** `DorisArrowTypeMappingTest`: 47 table rows (every
`PrimitiveType`; DATETIMEV2 at scales 0 / 1 / 3 / 4 / 6, TIMESTAMPTZ at 0 / 3 /
6, DECIMALV2 with a declared precision it ignores), the every-type-has-a-row
check, and the seven nested-type cases moved from
`FlightSqlSchemaHelperArrowTypeTest`;
`FlightSqlSchemaHelperSerializedSchemaTest`: the two round trips. 57 cases, the
same 57 the first commit runs against the old code.
**Regression.** `arrow_flight_sql_p0/test_get_tables_schema` (new): the
scalars, DATETIME and TIMESTAMPTZ at three scales each, TIMESTAMP_NS,
DECIMAL(9,2) / (18,4) / (38,10) / (76,20), JSON, VARIANT, IPV4 / IPV6, BITMAP /
HLL / QUANTILE_STATE / AGG_STATE, ARRAY / MAP / STRUCT and a three-level
nesting, one expected line per field; it fails on a single changed cell
(checked by breaking one on purpose). Whole `arrow_flight_sql_p0` green locally
against an FE built from this branch (14 suites, 0 failed); `test_select` also
reads `DatabaseMetaData.getColumns`, which is `GetTables(include_schema)`
through the Flight SQL JDBC driver.
**Manual.** The same `GetTables(include_schema = true)` dump (type,
nullability, metadata, children of every field) taken over raw Flight with
pyarrow from an FE built before this PR and from one built at its head:
identical, 52 lines each. The suite's expected blocks are that dump.
- Behavior changed:
- [x] No.
- Does this need documentation?
- [x] No.
### 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 -->
--
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]
