Skip to content

Commit

Permalink
Fix HiveTypeSerializer.unregisteredOpaque (#11401)
Browse files Browse the repository at this point in the history
Summary:
Pull Request resolved: #11401

Currently the opaque types are registed into a global static variable that is shared
across the process. If multiple tests within the same Suite use the same alias,
they can interfere with the test's expectations. Therefore, this introduces a way
to unregister the type to allow isolation between tests.

Reviewed By: kunigami

Differential Revision: D65290938

fbshipit-source-id: 9ce33ffcb956b7195ec0e1e9eb32c981bac57eb9
  • Loading branch information
Bikramjeet Vig authored and facebook-github-bot committed Oct 31, 2024
1 parent 788ba2a commit a3d0bc3
Show file tree
Hide file tree
Showing 3 changed files with 13 additions and 0 deletions.
10 changes: 10 additions & 0 deletions velox/type/Type.h
Original file line number Diff line number Diff line change
Expand Up @@ -2037,6 +2037,16 @@ bool registerOpaqueType(const std::string& alias) {
getOpaqueAliasByTypeIndex().emplace(typeIndex, alias).second;
}

/// Unregisters an opaque type. Returns true if the type was unregistered.
/// Currently, it is only used for testing to provide isolation between tests
/// when using the same alias.
template <typename Class>
bool unregisterOpaqueType(const std::string& alias) {
auto typeIndex = std::type_index(typeid(Class));
return getTypeIndexByOpaqueAlias().erase(alias) == 1 &&
getOpaqueAliasByTypeIndex().erase(typeIndex) == 1;
}

/// Return true if a custom type with the specified name exists.
bool customTypeExists(const std::string& name);

Expand Down
2 changes: 2 additions & 0 deletions velox/type/fbhive/tests/HiveTypeParserTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,7 @@ TEST(FbHive, parseOpaque) {
HiveTypeParser parser;
auto t = parser.parse("opaque<bar>");
ASSERT_EQ(t->toString(), "OPAQUE<facebook::velox::type::fbhive::Foo>");
unregisterOpaqueType<Foo>("bar");
}

TEST(FbHive, parseUnregisteredOpaque) {
Expand All @@ -192,5 +193,6 @@ TEST(FbHive, parseUnregisteredOpaque) {
VELOX_ASSERT_THROW(
parser.parse("opaque<Foo>"),
"Could not find type 'Foo'. Did you call registerOpaqueType?");
unregisterOpaqueType<Foo>("bar");
}
} // namespace facebook::velox::type::fbhive
1 change: 1 addition & 0 deletions velox/type/fbhive/tests/HiveTypeSerializerTests.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@ TEST(HiveTypeSerializer, opaque) {
std::shared_ptr<const Type> type = OPAQUE<Foo>();
auto result = HiveTypeSerializer::serialize(type);
EXPECT_EQ(result, "opaque<bar>");
unregisterOpaqueType<Foo>("bar");
}

TEST(HiveTypeSerializer, unregisteredOpaque) {
Expand Down

0 comments on commit a3d0bc3

Please sign in to comment.