Support nested types for the new public Arrow format #28413
Conversation
There was a problem hiding this comment.
Pull Request Overview
This pull request performs a comprehensive refactoring of the KQP Arrow format utilities by renaming files from kqp_result_set_arrow.* to kqp_formats_arrow.* and extracting test utilities into separate modules. The changes improve code organization without altering functionality.
Key Changes:
- Renamed core Arrow conversion files:
kqp_result_set_arrow.{h,cpp}→kqp_formats_arrow.{h,cpp} - Extracted test helper utilities from test file into
kqp_formats_ut_helpers.{h,cpp} - Split large test file into modular structure with dedicated helper and test files
- Updated all include references across the codebase to use the new filenames
- Enhanced documentation with comprehensive API comments
Reviewed Changes
Copilot reviewed 12 out of 13 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| ydb/core/kqp/common/result_set_format/ya.make | Updated build configuration to reference kqp_formats_arrow.cpp instead of old filename |
| ydb/core/kqp/common/result_set_format/ut/ya.make | Added new test helper source file and renamed test file in build configuration |
| ydb/core/kqp/common/result_set_format/ut/kqp_result_set_arrow_ut.cpp | Removed (deleted) - old test file |
| ydb/core/kqp/common/result_set_format/ut/kqp_formats_ut_helpers.h | New header for test utility functions with proper documentation |
| ydb/core/kqp/common/result_set_format/ut/kqp_formats_ut_helpers.cpp | New implementation of test utilities extracted from old test file |
| ydb/core/kqp/common/result_set_format/ut/kqp_formats_arrow_ut.cpp | New test file with refactored and expanded test cases |
| ydb/core/kqp/common/result_set_format/kqp_result_set_builders.cpp | Updated include path to use new header filename |
| ydb/core/kqp/common/result_set_format/kqp_result_set_arrow.h | Removed (deleted) - old header file |
| ydb/core/kqp/common/result_set_format/kqp_result_set_arrow.cpp | Removed (deleted) - old implementation file |
| ydb/core/kqp/common/result_set_format/kqp_formats_arrow.h | New header with improved documentation and cleaner API surface |
| ydb/core/formats/arrow/arrow_helpers_minikql.cpp | Updated include path to use new header filename |
| ydb/core/formats/arrow/arrow_batch_builder.cpp | Updated include path to use new header filename |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
|
🟢 |
|
⚪ ⚪ Ya make output | Test bloat | Test bloat
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
|
⚪ ⚪ Ya make output | Test bloat | Test bloat
🟢
*please be aware that the difference is based on comparing your commit and the last completed build from the post-commit, check comparation |
3 participants
Changelog entry
...
Changelog category
Description for reviewers
...