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


Reply via email to