forked from facebookincubator/velox
-
Notifications
You must be signed in to change notification settings - Fork 0
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
refactor: Break up Presto custom types into Declaration and Registrat…
…ion files (facebookincubator#12393) Summary: Pull Request resolved: facebookincubator#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
- Loading branch information
1 parent
027452c
commit b24d30d
Showing
49 changed files
with
529 additions
and
356 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
46 changes: 46 additions & 0 deletions
46
velox/functions/prestosql/types/HyperLogLogRegistration.cpp
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,46 @@ | ||
/* | ||
* 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 std::vector<TypeParameter>& parameters) const override { | ||
VELOX_CHECK(parameters.empty()); | ||
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 |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.