feat: Support UUID values across row and file IO#790
Conversation
huan233usc
left a comment
huan233usc
left a comment
There was a problem hiding this comment.
A few minor, non-blocking nits — otherwise LGTM.
| return std::nullopt; | ||
| } | ||
|
|
||
| constexpr std::array<uint8_t, Uuid::kLength> kUuidBytes1 = { |
There was a problem hiding this comment.
kUuidBytes1/2 + MakeUuidArray are duplicated with avro_test.cc — consider a shared test util.
There was a problem hiding this comment.
kUuidBytes1/2 are just random UUID values, adding a shared test util file might be overkill.
| field->nullable(), field->metadata()); | ||
| } | ||
|
|
||
| // Arrow cannot construct builders for arrow.uuid extension arrays yet, so build |
There was a problem hiding this comment.
This storage-type rewrite is a general workaround; if a second caller shows up, consider promoting it to a shared internal helper.
There was a problem hiding this comment.
I think only AvroReader needs this workaround, so I think it's fine to keep it here for now?
| SchemaField::MakeRequired(3, MapType::kValueName, iceberg::uuid())))}); | ||
|
|
||
| auto map_offsets = MakeInt32Array({0, 2, 3}); | ||
| auto map_keys = MakeStringArray({"first", "second", "only"}); |
There was a problem hiding this comment.
Map key is a string here — the UUID-key path (StorageTypeForBuilder(key_type)) is untested; add a UUID-key case if Iceberg allows it.
There was a problem hiding this comment.
Add a case, thanks.
2 participants
closes #314