refactor: Break up Presto custom types into Declaration and Registrat…

…ion files (#12393)

Summary:
Pull Request resolved: #12393

Today Presto's custom types have a bunch of classes implemented in the same file:
1) The Type itself
2) The CastOperator (if any)
3) The implementation of CustomTypeFactories
In addition to the function registerXXXType()

While implementing an InputGenerator for TimestampWithTimeZone I ran into an issue, I needed
to take a dependency on the TimestampWithTimeZone Type itself (or at least its utility functions).

Since the InputGenerator needs to be referred to by the CustomTypeFactories implementation
this lead to a circular dependency between the so files.

One option is to add the InputGenerator to the TimestampWithTimeZoneType file, but adding yet
another class there would make these files even more monolithic. Another option was to add it
to the velox/functions/prestosql/types directory and the so it produces, but given the
InputGenerators are only used for testing it seems they should be separated from the production
code, e.g. into a separate fuzzers subdirectory like we do for InputGenerators for functions.

Excluding those options, one option I'm left with is to break up these files (which generally seems
like a good thing for clarity's sake). This change breaks the custom types into a XXXType.h file
where they Type itself is defined, and XXXRegistration.h/cpp files which define the
registerXXXType() functions.  In most cases the CastOperator and the CustomTypeFactories
do not need to be exposed, so I've included those in the XXXRegistration.cpp file.  The one
exception is JsonCastOperator which I gave it's own JsonCastOperator.h/cpp files.

Presto includes the registerXXXType() functions in some tests from the XXXType.h files, so this
is the first of 3 steps.  Once this lands, I'll update Presto to include the XXXRegistration.h files
instead, then come back and remove the dependency on XXXRegistration.h from the XXXType.h
files, allowing us to break up the so files.

Once this is done InputGenerators can depend on the Type they're generating without
introducing a circular dependency.

Reviewed By: kagamiori

Differential Revision: D69891219

velox/common/fuzzer/tests/ConstrainedGeneratorsTest.cpp

@@ -18,6 +18,7 @@
 
 #include <gtest/gtest.h>
 
+#include "velox/common/memory/Memory.h"
 #include "velox/functions/prestosql/types/JsonType.h"
 #include "velox/type/Variant.h"
 

velox/duckdb/conversion/tests/DuckParserTest.cpp

@@ -16,8 +16,8 @@
 #include "velox/duckdb/conversion/DuckParser.h"
 #include "velox/common/base/tests/GTestUtils.h"
 #include "velox/core/PlanNode.h"
-#include "velox/functions/prestosql/types/JsonType.h"
-#include "velox/functions/prestosql/types/TimestampWithTimeZoneType.h"
+#include "velox/functions/prestosql/types/JsonRegistration.h"
+#include "velox/functions/prestosql/types/TimestampWithTimeZoneRegistration.h"
 #include "velox/parse/Expressions.h"
 
 using namespace facebook::velox;

velox/expression/tests/SignatureBinderTest.cpp

@@ -18,6 +18,7 @@
 #include <velox/type/HugeInt.h>
 #include <vector>
 #include "velox/common/base/tests/GTestUtils.h"
+#include "velox/functions/prestosql/types/TimestampWithTimeZoneRegistration.h"
 #include "velox/functions/prestosql/types/TimestampWithTimeZoneType.h"
 
 namespace facebook::velox::exec::test {
@@ -787,9 +788,7 @@ TEST(SignatureBinderTest, logicalType) {
 }
 
 TEST(SignatureBinderTest, customType) {
-  registerCustomType(
-      "timestamp with time zone",
-      std::make_unique<const TimestampWithTimeZoneTypeFactories>());
+  registerTimestampWithTimeZoneType();
 
   // Custom type as an argument type.
   {
@@ -846,9 +845,7 @@ TEST(SignatureBinderTest, hugeIntType) {
 }
 
 TEST(SignatureBinderTest, namedRows) {
-  registerCustomType(
-      "timestamp with time zone",
-      std::make_unique<const TimestampWithTimeZoneTypeFactories>());
+  registerTimestampWithTimeZoneType();
 
   // Simple named row field.
   {

velox/functions/prestosql/IPAddressFunctions.h

@@ -17,7 +17,9 @@
 
 #include "velox/functions/Macros.h"
 #include "velox/functions/Registerer.h"
+#include "velox/functions/prestosql/types/IPAddressRegistration.h"
 #include "velox/functions/prestosql/types/IPAddressType.h"
+#include "velox/functions/prestosql/types/IPPrefixRegistration.h"
 #include "velox/functions/prestosql/types/IPPrefixType.h"
 
 namespace facebook::velox::functions {

velox/functions/prestosql/JsonFunctions.cpp

@@ -17,10 +17,10 @@
 
 #include "velox/common/base/SortingNetwork.h"
 #include "velox/expression/VectorFunction.h"
-#include "velox/functions/lib/Utf8Utils.h"
 #include "velox/functions/lib/string/StringImpl.h"
 #include "velox/functions/prestosql/json/JsonStringUtil.h"
 #include "velox/functions/prestosql/json/SIMDJsonUtil.h"
+#include "velox/functions/prestosql/types/JsonCastOperator.h"
 #include "velox/functions/prestosql/types/JsonType.h"
 
 namespace facebook::velox::functions {

velox/functions/prestosql/UuidFunctions.h

@@ -20,7 +20,7 @@
 
 #include "velox/functions/Macros.h"
 #include "velox/functions/Registerer.h"
-#include "velox/functions/prestosql/Comparisons.h"
+#include "velox/functions/prestosql/types/UuidRegistration.h"
 #include "velox/functions/prestosql/types/UuidType.h"
 
 namespace facebook::velox::functions {

velox/functions/prestosql/aggregates/ApproxDistinctAggregate.cpp

@@ -23,7 +23,7 @@
 #include "velox/exec/Aggregate.h"
 #include "velox/expression/FunctionSignature.h"
 #include "velox/functions/prestosql/aggregates/AggregateNames.h"
-#include "velox/functions/prestosql/types/HyperLogLogType.h"
+#include "velox/functions/prestosql/types/HyperLogLogRegistration.h"
 #include "velox/vector/DecodedVector.h"
 #include "velox/vector/FlatVector.h"
 

velox/functions/prestosql/aggregates/PrestoHasher.cpp

@@ -23,6 +23,8 @@
 #include "velox/functions/lib/RowsTranslationUtil.h"
 #include "velox/functions/prestosql/types/IPAddressType.h"
 #include "velox/functions/prestosql/types/TimestampWithTimeZoneType.h"
+#include "velox/type/DecimalUtil.h"
+#include "velox/vector/ComplexVector.h"
 
 namespace facebook::velox::aggregate {
 

velox/functions/prestosql/aggregates/RegisterAggregateFunctions.cpp

@@ -14,8 +14,7 @@
  * limitations under the License.
  */
 #include "velox/functions/prestosql/aggregates/RegisterAggregateFunctions.h"
-#include "velox/exec/Aggregate.h"
-#include "velox/functions/prestosql/types/JsonType.h"
+#include "velox/functions/prestosql/types/JsonRegistration.h"
 
 namespace facebook::velox::aggregate::prestosql {
 

velox/functions/prestosql/benchmarks/JsonExprBenchmark.cpp

@@ -23,6 +23,7 @@
 #include "velox/functions/lib/benchmarks/FunctionBenchmarkBase.h"
 #include "velox/functions/prestosql/JsonFunctions.h"
 #include "velox/functions/prestosql/json/JsonExtractor.h"
+#include "velox/functions/prestosql/types/JsonRegistration.h"
 #include "velox/functions/prestosql/types/JsonType.h"
 
 namespace facebook::velox::functions::prestosql {

velox/functions/prestosql/registration/ArrayFunctionsRegistration.cpp

@@ -24,6 +24,7 @@
 #include "velox/functions/prestosql/ArrayFunctions.h"
 #include "velox/functions/prestosql/ArraySort.h"
 #include "velox/functions/prestosql/WidthBucketArray.h"
+#include "velox/functions/prestosql/types/JsonRegistration.h"
 
 namespace facebook::velox::functions {
 extern void registerArrayConcatFunctions(const std::string& prefix);

velox/functions/prestosql/registration/ComparisonFunctionsRegistration.cpp

@@ -14,12 +14,13 @@
  * limitations under the License.
  */
 #include "velox/functions/Registerer.h"
-#include "velox/functions/lib/RegistrationHelpers.h"
 #include "velox/functions/prestosql/Comparisons.h"
+#include "velox/functions/prestosql/types/IPAddressRegistration.h"
 #include "velox/functions/prestosql/types/IPAddressType.h"
+#include "velox/functions/prestosql/types/IPPrefixRegistration.h"
 #include "velox/functions/prestosql/types/IPPrefixType.h"
+#include "velox/functions/prestosql/types/TimestampWithTimeZoneRegistration.h"
 #include "velox/functions/prestosql/types/TimestampWithTimeZoneType.h"
-#include "velox/type/Type.h"
 
 namespace facebook::velox::functions {
 namespace {

velox/functions/prestosql/registration/DateTimeFunctionsRegistration.cpp

@@ -14,9 +14,9 @@
  * limitations under the License.
  */
 
-#include "velox/expression/VectorFunction.h"
 #include "velox/functions/Registerer.h"
 #include "velox/functions/prestosql/DateTimeFunctions.h"
+#include "velox/functions/prestosql/types/TimestampWithTimeZoneRegistration.h"
 
 namespace facebook::velox::functions {
 namespace {

velox/functions/prestosql/registration/HyperLogFunctionsRegistration.cpp

@@ -15,6 +15,7 @@
  */
 #include "velox/functions/Registerer.h"
 #include "velox/functions/prestosql/HyperLogLogFunctions.h"
+#include "velox/functions/prestosql/types/HyperLogLogRegistration.h"
 
 namespace facebook::velox::functions {
 

velox/functions/prestosql/registration/JsonFunctionsRegistration.cpp

@@ -16,6 +16,7 @@
 
 #include "velox/functions/Registerer.h"
 #include "velox/functions/prestosql/JsonFunctions.h"
+#include "velox/functions/prestosql/types/JsonRegistration.h"
 
 namespace facebook::velox::functions {
 void registerJsonFunctions(const std::string& prefix) {

velox/functions/prestosql/types/CMakeLists.txt

@@ -13,13 +13,14 @@
 # limitations under the License.
 velox_add_library(
   velox_presto_types
-  HyperLogLogType.cpp
-  JsonType.cpp
-  TDigestType.cpp
-  TimestampWithTimeZoneType.cpp
-  UuidType.cpp
-  IPAddressType.cpp
-  IPPrefixType.cpp)
+  HyperLogLogRegistration.cpp
+  IPAddressRegistration.cpp
+  IPPrefixRegistration.cpp
+  JsonCastOperator.cpp
+  JsonRegistration.cpp
+  TDigestRegistration.cpp
+  TimestampWithTimeZoneRegistration.cpp
+  UuidRegistration.cpp)
 
 velox_link_libraries(
   velox_presto_types

velox/functions/prestosql/types/HyperLogLogRegistration.cpp

@@ -0,0 +1,45 @@
+/*
+ * Copyright (c) Facebook, Inc. and its affiliates.
+ *
+ * Licensed under the Apache License, Version 2.0 (the "License");
+ * you may not use this file except in compliance with the License.
+ * You may obtain a copy of the License at
+ *
+ *     http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing, software
+ * distributed under the License is distributed on an "AS IS" BASIS,
+ * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+ * See the License for the specific language governing permissions and
+ * limitations under the License.
+ */
+
+#include "velox/functions/prestosql/types/HyperLogLogRegistration.h"
+
+#include "velox/functions/prestosql/types/HyperLogLogType.h"
+#include "velox/type/Type.h"
+
+namespace facebook::velox {
+namespace {
+class HyperLogLogTypeFactories : public CustomTypeFactories {
+ public:
+  TypePtr getType() const override {
+    return HYPERLOGLOG();
+  }
+
+  // HyperLogLog should be treated as Varbinary during type castings.
+  exec::CastOperatorPtr getCastOperator() const override {
+    return nullptr;
+  }
+
+  AbstractInputGeneratorPtr getInputGenerator(
+      const InputGeneratorConfig& /*config*/) const override {
+    return nullptr;
+  }
+};
+} // namespace
+void registerHyperLogLogType() {
+  registerCustomType(
+      "hyperloglog", std::make_unique<const HyperLogLogTypeFactories>());
+}
+} // namespace facebook::velox

velox/functions/prestosql/types/TDigestType.cpp → velox/functions/prestosql/types/HyperLogLogRegistration.h

@@ -13,12 +13,9 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-#include "velox/functions/prestosql/types/TDigestType.h"
 
-namespace facebook::velox {
-
-void registerTDigestType() {
-  registerCustomType("tdigest", std::make_unique<const TDigestTypeFactories>());
-}
+#pragma once
 
+namespace facebook::velox {
+void registerHyperLogLogType();
 } // namespace facebook::velox

velox/functions/prestosql/types/HyperLogLogType.h

@@ -17,7 +17,9 @@
 
 #include "velox/type/SimpleFunctionApi.h"
 #include "velox/type/Type.h"
-#include "velox/vector/VectorTypeUtils.h"
+
+// TODO: Remove this once Presto is updated.
+#include "velox/functions/prestosql/types/HyperLogLogRegistration.h"
 
 namespace facebook::velox {
 
@@ -71,23 +73,4 @@ struct HyperLogLogT {
 
 using HyperLogLog = CustomType<HyperLogLogT>;
 
-class HyperLogLogTypeFactories : public CustomTypeFactories {
- public:
-  TypePtr getType() const override {
-    return HYPERLOGLOG();
-  }
-
-  // HyperLogLog should be treated as Varbinary during type castings.
-  exec::CastOperatorPtr getCastOperator() const override {
-    return nullptr;
-  }
-
-  AbstractInputGeneratorPtr getInputGenerator(
-      const InputGeneratorConfig& /*config*/) const override {
-    return nullptr;
-  }
-};
-
-void registerHyperLogLogType();
-
 } // namespace facebook::velox

velox/functions/prestosql/types/IPAddressType.cpp → velox/functions/prestosql/types/IPAddressRegistration.cpp

@@ -14,15 +14,14 @@
  * limitations under the License.
  */
 
-#include "velox/functions/prestosql/types/IPAddressType.h"
+#include "velox/functions/prestosql/types/IPAddressRegistration.h"
+
 #include "velox/expression/CastExpr.h"
-#include "velox/expression/VectorWriters.h"
+#include "velox/functions/prestosql/types/IPAddressType.h"
 #include "velox/functions/prestosql/types/IPPrefixType.h"
 
 namespace facebook::velox {
-
 namespace {
-
 class IPAddressCastOperator : public exec::CastOperator {
  public:
   bool isSupportedFromType(const TypePtr& other) const override {
@@ -271,12 +270,10 @@ class IPAddressTypeFactories : public CustomTypeFactories {
     return nullptr;
   }
 };
-
 } // namespace
 
 void registerIPAddressType() {
   registerCustomType(
       "ipaddress", std::make_unique<const IPAddressTypeFactories>());
 }
-
 } // namespace facebook::velox

velox/functions/prestosql/types/HyperLogLogType.cpp → velox/functions/prestosql/types/IPAddressRegistration.h

@@ -13,13 +13,9 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-#include "velox/functions/prestosql/types/HyperLogLogType.h"
 
-namespace facebook::velox {
+#pragma once
 
-void registerHyperLogLogType() {
-  registerCustomType(
-      "hyperloglog", std::make_unique<const HyperLogLogTypeFactories>());
+namespace facebook::velox {
+void registerIPAddressType();
 }
-
-} // namespace facebook::velox

velox/functions/prestosql/types/IPAddressType.h

@@ -20,6 +20,9 @@
 #include "velox/type/SimpleFunctionApi.h"
 #include "velox/type/Type.h"
 
+// TODO: Remove this once Presto is updated.
+#include "velox/functions/prestosql/types/IPAddressRegistration.h"
+
 namespace facebook::velox {
 
 namespace ipaddress {
@@ -115,6 +118,4 @@ struct IPAddressT {
 
 using IPAddress = CustomType<IPAddressT, /*providesCustomComparison*/ true>;
 
-void registerIPAddressType();
-
 } // namespace facebook::velox

velox/functions/prestosql/types/IPPrefixType.cpp → velox/functions/prestosql/types/IPPrefixRegistration.cpp

@@ -13,15 +13,14 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
-#include <folly/small_vector.h>
+
+#include "velox/functions/prestosql/types/IPPrefixRegistration.h"
 
 #include "velox/expression/CastExpr.h"
 #include "velox/functions/prestosql/types/IPPrefixType.h"
 
 namespace facebook::velox {
-
 namespace {
-
 class IPPrefixCastOperator : public exec::CastOperator {
  public:
   bool isSupportedFromType(const TypePtr& other) const override {
@@ -196,12 +195,10 @@ class IPPrefixTypeFactories : public CustomTypeFactories {
     return nullptr;
   }
 };
-
 } // namespace
 
 void registerIPPrefixType() {
   registerCustomType(
       "ipprefix", std::make_unique<const IPPrefixTypeFactories>());
 }
-
 } // namespace facebook::velox