bkietz commented on a change in pull request #7378: URL: https://github.com/apache/arrow/pull/7378#discussion_r439477647
########## File path: cpp/src/arrow/type.h ########## @@ -1015,25 +1018,12 @@ class ARROW_EXPORT Decimal128Type : public DecimalType { /// \brief Concrete type class for union data class ARROW_EXPORT UnionType : public NestedType { public: - static constexpr Type::type type_id = Type::UNION; static constexpr int8_t kMaxTypeCode = 127; static constexpr int kInvalidChildId = -1; - static constexpr const char* type_name() { return "union"; } - - UnionType(const std::vector<std::shared_ptr<Field>>& fields, Review comment: I'm not sure what you mean; UnionType is an abstract base class now and should never be constructed except as a base object of SparseUnionType or DenseUnionType. If we want to maintain compatibility of code which constructs UnionType directly then we'll need to have a single concrete `UnionType` which corresponds to both `Type::SPARSE_UNION` and `Type::DENSE_UNION`. That's possible but very much not our pattern with `DataType` subclasses. ---------------------------------------------------------------- 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. For queries about this service, please contact Infrastructure at: us...@infra.apache.org