wgtmac commented on code in PR #45375:
URL: https://github.com/apache/arrow/pull/45375#discussion_r1959890857
##########
cpp/src/parquet/types.cc:
##########
@@ -1619,6 +1629,22 @@ class LogicalType::Impl::Float16 final : public
LogicalType::Impl::Incompatible,
GENERATE_MAKE(Float16)
+class LogicalType::Impl::Variant final : public
LogicalType::Impl::Incompatible,
+ public
LogicalType::Impl::SimpleApplicable {
+ public:
+ friend class VariantLogicalType;
+
+ OVERRIDE_TOSTRING(Variant)
+ OVERRIDE_TOTHRIFT(VariantType, VARIANT)
+
+ private:
+ Variant()
+ : LogicalType::Impl(LogicalType::Type::VARIANT, SortOrder::UNKNOWN),
+ LogicalType::Impl::SimpleApplicable(parquet::Type::BYTE_ARRAY) {}
Review Comment:
IMO, `VariantLogicalType` should be similar to `MapLogicalType` and
`ListLogicalType` which annotate group type. Though it (the unshredded form) is
composed of two separate byte arrays, these two types are under the same group
type as below. Can we model it as a `struct<binary,binary>`?
```
optional group variant_name (VARIANT) {
required binary metadata;
required binary value;
}
```
--
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]
