real-mj-song opened a new pull request, #18632: URL: https://github.com/apache/pinot/pull/18632
**TL;DR:** Precursor refactor extracting Arrow → Pinot type-conversion logic from `ArrowRecordExtractor` into a new public utility `ArrowToPinotTypeConverter`. Behavior-preserving on the row-major path; enables a column-major `ColumnReader` consumer to reuse the same conversion in a follow-on PR. Tracks #18629. This is the **precursor** PR; a follow-on PR will add `ArrowColumnReaderFactory` / `ArrowColumnReader` on top of this branch and consume the extracted utility. ### What changed - **New:** `ArrowToPinotTypeConverter` — public static utility holding the schema-driven dispatch on `ArrowType.ArrowTypeID` previously inlined in `ArrowRecordExtractor`. Returns canonical JDK types (`String` from `Utf8` / `LargeUtf8` unwrapped from Arrow's `Text`, `Object[]` for `List` variants, `LocalDate` / `LocalTime` / `Timestamp` for temporal types, `BigDecimal` for `Decimal`, etc.). - `ArrowRecordExtractor.extract` now delegates to `ArrowToPinotTypeConverter.toPinotValue(field, value, extractRawTimeValues)`. The inline `convert()` and its helpers (`convertTimestamp`, `convertDate`, `convertTime`, `convertList`, `convertMap`, `convertStruct`, `convertByRuntimeType`, `toEpochInUnit`) are removed from the extractor. - No behavior change on the row-major path. ### Testing All 63 existing row-major tests pass unchanged: - `ArrowRecordReaderTest` (2 / 2) - `ArrowRecordExtractorTest` (54 / 54) - `ArrowMessageDecoderTest` (7 / 7) `ArrowRecordExtractorTest` exhaustively exercises every converter branch (every Arrow logical type, all temporal units, the `extractRawTimeValues` bypass, list / struct / map recursion, dictionary encoding, nulls). The refactor green-bar on those tests proves behavior preservation. ### Compatibility Purely additive on the SPI side. Public API surface change: `ArrowToPinotTypeConverter` is a new public class; nothing existing is renamed or removed. ### WIP notes (before marking ready) - Add a focused `ArrowToPinotTypeConverterTest` calling the new public API directly to pin the contract for follow-on consumers. ### References - Tracking issue: #18629 - Related row-major refactor: #18434 -- 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]
