Impl DataType public mapping from MKQL to Arrow #27295
Conversation
|
🟢 |
There was a problem hiding this comment.
Pull Request Overview
This PR prepares infrastructure for implementing a new public mapping between MiniKQL and Arrow types. The changes focus on refactoring test utilities and expanding type conversion coverage to support comprehensive testing of the MiniKQL-to-Arrow type mapping.
Key Changes:
- Moved
IsArrowCompatiblefrom test namespace to public API - Enhanced test data generation with support for additional data types (Decimal, timezone types, string variants, Uuid)
- Added comprehensive unit tests for individual data type conversions
- Modified timezone representation in Arrow from ID (uint16) to IANA name (string)
Reviewed Changes
Copilot reviewed 3 out of 5 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| kqp_result_set_arrow_ut.cpp | Expanded test coverage with new data types, added individual type conversion tests, and updated helper functions for timezone handling |
| kqp_result_set_arrow.h | Moved IsArrowCompatible declaration from NTestUtils namespace to public API |
| kqp_result_set_arrow.cpp | Relocated IsArrowCompatible implementation, changed timezone field from uint16 ID to UTF8 string, and updated singular type handling |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
⚪ ⚪ Ya make output | Test bloat | Test bloat
⚪ Ya make output | Test bloat | 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 |
|
⚪ ⚪ Ya make output | Test bloat | Test bloat
⚪ Ya make output | Test bloat | 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 |
|
⚪ ⚪ 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 |
dahbka-lis
changed the title
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 4 out of 6 changed files in this pull request and generated 2 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
|
⚪
🟢
*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 |
| return holderFactory.CreateVariantHolder(value.Release(), variantIndex); | ||
| } | ||
| default: | ||
| default: { |
There was a problem hiding this comment.
ты где удалял default опцию, может и здесь удалить?
There was a problem hiding this comment.
ты где удалял default опцию, может и здесь удалить?
Я старался пока сильно не трогать код nested типов, в следующем PR посмотрю
2 participants
Changelog entry
...
Changelog category
Description for reviewers
Support primitive and parametric types from here, add tests for each data type.
...