Skip to content

Commit 1011baf

Browse files
Ashish Misrafacebook-github-bot
authored andcommitted
refactor: Implement one step type registration and serialization/deserialization for OpaqueCustomType (#15843)
Summary: ## Background. - Today, a CustomType (inner type) may be wrapped by an OpaqueType (outer type), forming a OpaqueCustomType. - When an OpaqueCustomType is defined, it needs to be registered in two place. - Custom Type Registry - a mapping from type name to an instance of CustomTypeFactory that provides an instance of the type - OpaqueSerdeRegistry - a mapping from type name to functions that serialize and deserialize opaque type values - Today, this requires two separate calls to the API. These separate calls are inconvenient and error prone. ## Implementation - OpaqueCustomTypes.h - This PR changes the OpaqueCustomTypeRegister::registerType() function to accept optional serialization/deserialization functions, thus eliminating the need for a separate API call. - By keep the function arguments optional, it provides backward compatibility with existing code. ## Testing Changes - OpaqueCustomTypesTest.cpp - Implements unit-tests that ensure that serialization/deserialization functions can be provided in the registerType function optionally. - Unit-tests ensure that serialization/deserialization functions can be provided utmost once. Differential Revision: D89686612
1 parent b54cda6 commit 1011baf

File tree

3 files changed

+126
-4
lines changed

3 files changed

+126
-4
lines changed

velox/type/OpaqueCustomTypes.h

Lines changed: 38 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -34,15 +34,26 @@ class CastOperator;
3434
template <typename T, const char* customTypeName>
3535
class OpaqueCustomTypeRegister {
3636
public:
37-
static bool registerType() {
38-
return facebook::velox::registerCustomType(
39-
customTypeName, std::make_unique<const TypeFactory>());
40-
}
37+
using SerializeFunc = OpaqueType::SerializeFunc<T>;
38+
using DeserializeFunc = OpaqueType::DeserializeFunc<T>;
39+
40+
// Optional serialization and deserialization functions. If not provided, the
41+
// the registerSerialization() can be invoked utmost once later.
42+
static bool registerType(
43+
SerializeFunc serialize = nullptr,
44+
DeserializeFunc deserialize = nullptr);
4145

4246
static bool unregisterType() {
4347
return facebook::velox::unregisterCustomType(customTypeName);
4448
}
4549

50+
// Can be invoked only if serialization/deserialization functions were not
51+
// provided in the registerType function. In such a case, it can be invoked
52+
// only once.
53+
static void registerSerialization(
54+
SerializeFunc serialize,
55+
DeserializeFunc deserialize);
56+
4657
// Type used in the simple function interface as CustomType<TypeT>.
4758
struct TypeT {
4859
using type = std::shared_ptr<T>;
@@ -108,4 +119,27 @@ class OpaqueCustomTypeRegister {
108119
}
109120
};
110121
};
122+
123+
template <typename T, const char* customTypeName>
124+
bool OpaqueCustomTypeRegister<T, customTypeName>::registerType(
125+
SerializeFunc serialize,
126+
DeserializeFunc deserialize) {
127+
if (facebook::velox::registerCustomType(
128+
customTypeName, std::make_unique<const TypeFactory>())) {
129+
if (serialize || deserialize) {
130+
registerSerialization(serialize, deserialize);
131+
}
132+
return true;
133+
}
134+
return false;
135+
}
136+
137+
template <typename T, const char* customTypeName>
138+
void OpaqueCustomTypeRegister<T, customTypeName>::registerSerialization(
139+
SerializeFunc serialize,
140+
DeserializeFunc deserialize) {
141+
using Type = typename TypeT::type::element_type;
142+
OpaqueType::registerSerialization<Type>(
143+
customTypeName, serialize, deserialize);
144+
}
111145
} // namespace facebook::velox

velox/type/tests/CMakeLists.txt

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@ add_executable(
2121
FilterSerDeTest.cpp
2222
FloatingPointUtilTest.cpp
2323
HugeIntTest.cpp
24+
OpaqueCustomTypesTest.cpp
2425
StringViewTest.cpp
2526
SubfieldTest.cpp
2627
TimestampConversionTest.cpp
Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,87 @@
1+
/*
2+
* Copyright (c) Facebook, Inc. and its affiliates.
3+
*
4+
* Licensed under the Apache License, Version 2.0 (the "License");
5+
* you may not use this file except in compliance with the License.
6+
* You may obtain a copy of the License at
7+
*
8+
* http://www.apache.org/licenses/LICENSE-2.0
9+
*
10+
* Unless required by applicable law or agreed to in writing, software
11+
* distributed under the License is distributed on an "AS IS" BASIS,
12+
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
13+
* See the License for the specific language governing permissions and
14+
* limitations under the License.
15+
*/
16+
17+
#include <gtest/gtest.h>
18+
19+
#include "velox/common/base/tests/GTestUtils.h"
20+
#include "velox/type/OpaqueCustomTypes.h"
21+
22+
using namespace facebook;
23+
using namespace facebook::velox;
24+
25+
namespace {
26+
27+
TEST(OpaqueCustomTypeTest, RegisterWithoutSerde) {
28+
// This is the C++ type registered within the opaque capsule. This could be
29+
// anything.
30+
struct TestCustomType {};
31+
static constexpr char testCustomTypeName[] = "my_test_custom_type";
32+
using TestCustomTypeRegister =
33+
OpaqueCustomTypeRegister<TestCustomType, testCustomTypeName>;
34+
35+
// Register type without serialization/deserialization and build a TypePtr.
36+
TestCustomTypeRegister::registerType();
37+
auto testCustomType = TestCustomTypeRegister::VeloxType::get();
38+
39+
// Register serialization/deserialization should succeed since no
40+
// serialization/deserialization was registered during type registration.
41+
TestCustomTypeRegister::registerSerialization(
42+
/*serialize=*/[](const std::shared_ptr<TestCustomType>&) { return ""; },
43+
/*deserialize=*/
44+
[](const std::string&) { return std::make_shared<TestCustomType>(); });
45+
46+
// Trying to re-register serialization/deserialization should fail.
47+
VELOX_ASSERT_THROW(
48+
TestCustomTypeRegister::registerSerialization(
49+
/*serialize=*/[](const std::shared_ptr<
50+
TestCustomType>&) { return ""; },
51+
/*deserialize=*/
52+
[](const std::string&) {
53+
return std::make_shared<TestCustomType>();
54+
}),
55+
"Trying to register duplicated serialization information for type OPAQUE");
56+
}
57+
58+
TEST(OpaqueCustomTypeTest, RegisterWithSerde) {
59+
// This is the C++ type registered within the opaque capsule. This could be
60+
// anything.
61+
struct TestCustomType {};
62+
static constexpr char testCustomTypeName[] = "my_test_custom_type";
63+
using TestCustomTypeRegister =
64+
OpaqueCustomTypeRegister<TestCustomType, testCustomTypeName>;
65+
66+
// Register type with serialization/deserialization and build a TypePtr.
67+
TestCustomTypeRegister::registerType(
68+
/*serialize=*/[](const std::shared_ptr<TestCustomType>&) { return ""; },
69+
/*deserialize=*/
70+
[](const std::string&) { return std::make_shared<TestCustomType>(); });
71+
auto testCustomType = TestCustomTypeRegister::VeloxType::get();
72+
73+
// Trying to re-register serialization/deserialization should fail since
74+
// serialization/deserialization was already registered during type
75+
// registration.
76+
VELOX_ASSERT_THROW(
77+
TestCustomTypeRegister::registerSerialization(
78+
/*serialize=*/[](const std::shared_ptr<
79+
TestCustomType>&) { return ""; },
80+
/*deserialize=*/
81+
[](const std::string&) {
82+
return std::make_shared<TestCustomType>();
83+
}),
84+
"Trying to register duplicated serialization information for type OPAQUE");
85+
}
86+
87+
} // namespace

0 commit comments

Comments
 (0)