refactor: Remove unused and misleading Tree superclass

Summary: The `Tree` superclass is misleading (`size()` only counts direct children and iterator only iterates direct children; usually for such interface we would expect to iterate the whole tree) and not really used anywhere.  Remove it to make the code base cleaner.

Differential Revision: D67241802

velox/dwio/common/TypeWithId.cpp

@@ -64,8 +64,8 @@ namespace {
 
 int countNodes(const TypePtr& type) {
   int count = 1;
-  for (auto& child : *type) {
-    count += countNodes(child);
+  for (uint32_t end = type->size(), i = 0; i < end; ++i) {
+    count += countNodes(type->childAt(i));
   }
   return count;
 }
@@ -89,15 +89,6 @@ std::unique_ptr<TypeWithId> TypeWithId::create(
       type, std::move(children), 0, next - 1, 0);
 }
 
-uint32_t TypeWithId::size() const {
-  return children_.size();
-}
-
-const std::shared_ptr<const TypeWithId>& TypeWithId::childAt(
-    uint32_t idx) const {
-  return children_.at(idx);
-}
-
 std::unique_ptr<TypeWithId> TypeWithId::create(
     const std::shared_ptr<const Type>& type,
     uint32_t& next,
@@ -107,9 +98,9 @@ std::unique_ptr<TypeWithId> TypeWithId::create(
   std::vector<std::unique_ptr<TypeWithId>> children;
   children.reserve(type->size());
   auto offset = 0;
-  for (const auto& child : *type) {
+  for (uint32_t end = type->size(), i = 0; i < end; ++i) {
     children.emplace_back(create(
-        child,
+        type->childAt(i),
         next,
         (myId == 0 && type->kind() == TypeKind::ROW) ? offset++ : column));
   }

velox/dwio/common/TypeWithId.h

@@ -23,7 +23,7 @@
 
 namespace facebook::velox::dwio::common {
 
-class TypeWithId : public velox::Tree<std::shared_ptr<const TypeWithId>> {
+class TypeWithId {
  public:
   /// NOTE: This constructor will re-parent the children.
   TypeWithId(
@@ -33,6 +33,8 @@ class TypeWithId : public velox::Tree<std::shared_ptr<const TypeWithId>> {
       uint32_t maxId,
       uint32_t column);
 
+  virtual ~TypeWithId() = default;
+
   TypeWithId(const TypeWithId&) = delete;
   TypeWithId& operator=(const TypeWithId&) = delete;
 
@@ -47,7 +49,9 @@ class TypeWithId : public velox::Tree<std::shared_ptr<const TypeWithId>> {
       const RowTypePtr& type,
       const velox::common::ScanSpec& spec);
 
-  uint32_t size() const override;
+  uint32_t size() const {
+    return children_.size();
+  }
 
   const std::shared_ptr<const velox::Type>& type() const {
     return type_;
@@ -69,7 +73,9 @@ class TypeWithId : public velox::Tree<std::shared_ptr<const TypeWithId>> {
     return column_;
   }
 
-  const std::shared_ptr<const TypeWithId>& childAt(uint32_t idx) const override;
+  const std::shared_ptr<const TypeWithId>& childAt(uint32_t idx) const {
+    return children_.at(idx);
+  }
 
   const std::shared_ptr<const TypeWithId>& childByName(
       const std::string& name) const {

velox/type/Type.cpp

@@ -530,8 +530,8 @@ folly::dynamic RowType::serialize() const {
 
 size_t Type::hashKind() const {
   size_t hash = (int32_t)kind();
-  for (auto& child : *this) {
-    hash = hash * 31 + child->hashKind();
+  for (uint32_t end = size(), i = 0; i < end; ++i) {
+    hash = hash * 31 + childAt(i)->hashKind();
   }
   return hash;
 }

velox/type/Type.h

@@ -37,7 +37,6 @@
 #include "velox/type/HugeInt.h"
 #include "velox/type/StringView.h"
 #include "velox/type/Timestamp.h"
-#include "velox/type/Tree.h"
 
 namespace facebook::velox {
 
@@ -433,7 +432,7 @@ struct TypeParameter {
 ///   IntegerType    ArrayType
 ///                     |
 ///                   BigintType
-class Type : public Tree<const TypePtr>, public velox::ISerializable {
+class Type : public velox::ISerializable {
  public:
   explicit Type(TypeKind kind, bool providesCustomComparison = false)
       : kind_{kind}, providesCustomComparison_(providesCustomComparison) {}
@@ -480,6 +479,12 @@ class Type : public Tree<const TypePtr>, public velox::ISerializable {
   /// Returns a possibly empty list of type parameters.
   virtual const std::vector<TypeParameter>& parameters() const = 0;
 
+  /// Returns the number of children of this type.
+  virtual uint32_t size() const = 0;
+
+  /// Returns the child at position index.  Index must be smaller than size().
+  virtual const TypePtr& childAt(uint32_t index) const = 0;
+
   /// Returns physical type name. Multiple logical types may share the same
   /// physical type backing and therefore return the same physical type name.
   /// The logical type name returned by 'name()' must be unique though.
@@ -490,7 +495,7 @@ class Type : public Tree<const TypePtr>, public velox::ISerializable {
   /// Options to control the output of toSummaryString().
   struct TypeSummaryOptions {
     /// Maximum number of child types to include in the summary.
-    size_type maxChildren{0};
+    uint32_t maxChildren{0};
   };
 
   /// Returns human-readable summary of the type. Useful when full output of

velox/type/tests/TypeTest.cpp

@@ -85,7 +85,6 @@ TEST(TypeTest, integer) {
   EXPECT_THROW(int0->childAt(0), std::invalid_argument);
   EXPECT_EQ(int0->kind(), TypeKind::INTEGER);
   EXPECT_STREQ(int0->kindName(), "INTEGER");
-  EXPECT_EQ(int0->begin(), int0->end());
 
   testTypeSerde(int0);
 }
@@ -101,7 +100,6 @@ TEST(TypeTest, timestamp) {
   EXPECT_THROW(t0->childAt(0), std::invalid_argument);
   EXPECT_EQ(t0->kind(), TypeKind::TIMESTAMP);
   EXPECT_STREQ(t0->kindName(), "TIMESTAMP");
-  EXPECT_EQ(t0->begin(), t0->end());
 
   testTypeSerde(t0);
 }
@@ -158,7 +156,6 @@ TEST(TypeTest, date) {
   EXPECT_THROW(date->childAt(0), std::invalid_argument);
   EXPECT_EQ(date->kind(), TypeKind::INTEGER);
   EXPECT_STREQ(date->kindName(), "INTEGER");
-  EXPECT_EQ(date->begin(), date->end());
 
   EXPECT_TRUE(date->kindEquals(INTEGER()));
   EXPECT_NE(*date, *INTEGER());
@@ -175,7 +172,6 @@ TEST(TypeTest, intervalDayTime) {
   EXPECT_THROW(interval->childAt(0), std::invalid_argument);
   EXPECT_EQ(interval->kind(), TypeKind::BIGINT);
   EXPECT_STREQ(interval->kindName(), "BIGINT");
-  EXPECT_EQ(interval->begin(), interval->end());
 
   EXPECT_TRUE(interval->kindEquals(BIGINT()));
   EXPECT_NE(*interval, *BIGINT());
@@ -196,7 +192,6 @@ TEST(TypeTest, intervalYearMonth) {
   EXPECT_THROW(interval->childAt(0), std::invalid_argument);
   EXPECT_EQ(interval->kind(), TypeKind::INTEGER);
   EXPECT_STREQ(interval->kindName(), "INTEGER");
-  EXPECT_EQ(interval->begin(), interval->end());
 
   EXPECT_TRUE(interval->kindEquals(INTEGER()));
   EXPECT_NE(*interval, *INTEGER());
@@ -224,7 +219,6 @@ TEST(TypeTest, unknown) {
   EXPECT_THROW(type->childAt(0), std::invalid_argument);
   EXPECT_EQ(type->kind(), TypeKind::UNKNOWN);
   EXPECT_STREQ(type->kindName(), "UNKNOWN");
-  EXPECT_EQ(type->begin(), type->end());
   EXPECT_TRUE(type->isComparable());
   EXPECT_TRUE(type->isOrderable());
 
@@ -237,7 +231,6 @@ TEST(TypeTest, shortDecimal) {
   EXPECT_EQ(shortDecimal->size(), 0);
   EXPECT_THROW(shortDecimal->childAt(0), std::invalid_argument);
   EXPECT_EQ(shortDecimal->kind(), TypeKind::BIGINT);
-  EXPECT_EQ(shortDecimal->begin(), shortDecimal->end());
 
   EXPECT_EQ(*DECIMAL(10, 5), *shortDecimal);
   EXPECT_NE(*DECIMAL(9, 5), *shortDecimal);
@@ -273,7 +266,6 @@ TEST(TypeTest, longDecimal) {
   EXPECT_EQ(longDecimal->size(), 0);
   EXPECT_THROW(longDecimal->childAt(0), std::invalid_argument);
   EXPECT_EQ(longDecimal->kind(), TypeKind::HUGEINT);
-  EXPECT_EQ(longDecimal->begin(), longDecimal->end());
   EXPECT_EQ(*DECIMAL(30, 5), *longDecimal);
   EXPECT_NE(*DECIMAL(9, 5), *longDecimal);
   EXPECT_NE(*DECIMAL(30, 3), *longDecimal);
@@ -370,18 +362,6 @@ TEST(TypeTest, map) {
   EXPECT_THROW(mapType->childAt(2), VeloxUserError);
   EXPECT_EQ(mapType->kind(), TypeKind::MAP);
   EXPECT_STREQ(mapType->kindName(), "MAP");
-  int32_t num = 0;
-  for (auto& i : *mapType) {
-    if (num == 0) {
-      EXPECT_EQ(i->toString(), "INTEGER");
-    } else if (num == 1) {
-      EXPECT_EQ(i->toString(), "ARRAY<BIGINT>");
-    } else {
-      FAIL();
-    }
-    ++num;
-  }
-  CHECK_EQ(num, 2);
 
   EXPECT_STREQ(mapType->name(), "MAP");
   EXPECT_EQ(mapType->parameters().size(), 2);
@@ -424,22 +404,6 @@ TEST(TypeTest, row) {
   EXPECT_TRUE(row0->containsChild("a"));
   EXPECT_TRUE(row0->containsChild("b"));
   EXPECT_FALSE(row0->containsChild("c"));
-  int32_t seen = 0;
-  for (auto& i : *row0) {
-    if (seen == 0) {
-      EXPECT_STREQ("INTEGER", i->kindName());
-    } else if (seen == 1) {
-      EXPECT_EQ("ROW<a:BIGINT>", i->toString());
-      int32_t seen2 = 0;
-      for (auto& j : *i) {
-        EXPECT_EQ(j->toString(), "BIGINT");
-        seen2++;
-      }
-      EXPECT_EQ(seen2, 1);
-    }
-    seen++;
-  }
-  CHECK_EQ(seen, 2);
 
   EXPECT_STREQ(row0->name(), "ROW");
   EXPECT_EQ(row0->parameters().size(), 2);