Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions presto-docs/src/main/sphinx/presto_cpp/properties.rst
Original file line number Diff line number Diff line change
Expand Up @@ -95,6 +95,14 @@ The configuration properties of Presto C++ workers are described here, in alphab

In-memory cache.

``presto.default-namespace``
^^^^^^^^^^^^^^^^^^^^^^^^^^^^

* **Type:** ``string``
* **Default value:** ``presto.default``

Specifies the namespace prefix for native C++ functions.

``query.max-memory-per-node``
^^^^^^^^^^^^^^^^^^^^^^^^^^^^^

Expand Down
10 changes: 6 additions & 4 deletions presto-native-execution/presto_cpp/main/PrestoServer.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,7 @@ void PrestoServer::run() {
address_ = fmt::format("[{}]", address_);
}
nodeLocation_ = nodeConfig->nodeLocation();
prestoBuiltinFunctionPrefix_ = systemConfig->prestoDefaultNamespacePrefix();
} catch (const VeloxUserError& e) {
PRESTO_STARTUP_LOG(ERROR) << "Failed to start server due to " << e.what();
exit(EXIT_FAILURE);
Expand Down Expand Up @@ -1166,11 +1167,12 @@ void PrestoServer::registerCustomOperators() {
}

void PrestoServer::registerFunctions() {
static const std::string kPrestoDefaultPrefix{"presto.default."};
velox::functions::prestosql::registerAllScalarFunctions(kPrestoDefaultPrefix);
velox::functions::prestosql::registerAllScalarFunctions(
prestoBuiltinFunctionPrefix_);
velox::aggregate::prestosql::registerAllAggregateFunctions(
kPrestoDefaultPrefix);
velox::window::prestosql::registerAllWindowFunctions(kPrestoDefaultPrefix);
prestoBuiltinFunctionPrefix_);
velox::window::prestosql::registerAllWindowFunctions(
prestoBuiltinFunctionPrefix_);
if (SystemConfig::instance()->registerTestFunctions()) {
velox::functions::prestosql::registerAllScalarFunctions(
"json.test_schema.");
Expand Down
1 change: 1 addition & 0 deletions presto-native-execution/presto_cpp/main/PrestoServer.h
Original file line number Diff line number Diff line change
Expand Up @@ -269,6 +269,7 @@ class PrestoServer {
std::string address_;
std::string nodeLocation_;
folly::SSLContextPtr sslContext_;
std::string prestoBuiltinFunctionPrefix_;
};

} // namespace facebook::presto
Original file line number Diff line number Diff line change
Expand Up @@ -228,7 +228,7 @@ SystemConfig::SystemConfig() {
STR_PROP(kCacheVeloxTtlThreshold, "2d"),
STR_PROP(kCacheVeloxTtlCheckInterval, "1h"),
BOOL_PROP(kEnableRuntimeMetricsCollection, false),
};
STR_PROP(kPrestoDefaultNamespacePrefix, "presto.default")};
}

SystemConfig* SystemConfig::instance() {
Expand Down Expand Up @@ -683,6 +683,10 @@ bool SystemConfig::enableRuntimeMetricsCollection() const {
return optionalProperty<bool>(kEnableRuntimeMetricsCollection).value();
}

std::string SystemConfig::prestoDefaultNamespacePrefix() const {
return optionalProperty(kPrestoDefaultNamespacePrefix).value().append(".");
}

NodeConfig::NodeConfig() {
registeredProps_ =
std::unordered_map<std::string, folly::Optional<std::string>>{
Expand Down
5 changes: 5 additions & 0 deletions presto-native-execution/presto_cpp/main/common/Configs.h
Original file line number Diff line number Diff line change
Expand Up @@ -585,6 +585,10 @@ class SystemConfig : public ConfigBase {
static constexpr std::string_view kDriverMaxPagePartitioningBufferSize{
"driver.max-page-partitioning-buffer-size"};

// Specifies the default Presto namespace prefix.
static constexpr std::string_view kPrestoDefaultNamespacePrefix{
"presto.default-namespace"};

SystemConfig();

virtual ~SystemConfig() = default;
Expand Down Expand Up @@ -802,6 +806,7 @@ class SystemConfig : public ConfigBase {
bool enableRuntimeMetricsCollection() const;

bool prestoNativeSidecar() const;
std::string prestoDefaultNamespacePrefix() const;
};

/// Provides access to node properties defined in node.properties file.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,8 +18,13 @@ target_link_libraries(
presto_common_test
presto_common
presto_exception
velox_aggregates
velox_exception
velox_file
velox_functions_prestosql
velox_function_registry
velox_presto_types
velox_window
${RE2}
gtest
gtest_main)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -12,17 +12,59 @@
* limitations under the License.
*/
#include <gtest/gtest.h>
#include <unordered_set>
#include "presto_cpp/main/common/ConfigReader.h"
#include "presto_cpp/main/common/Configs.h"
#include "velox/common/base/Exceptions.h"
#include "velox/common/base/tests/GTestUtils.h"
#include "velox/common/file/File.h"
#include "velox/common/file/FileSystems.h"
#include "velox/exec/Aggregate.h"
#include "velox/exec/Window.h"
#include "velox/functions/FunctionRegistry.h"
#include "velox/functions/prestosql/aggregates/RegisterAggregateFunctions.h"
#include "velox/functions/prestosql/registration/RegistrationFunctions.h"
#include "velox/functions/prestosql/window/WindowFunctionsRegistration.h"

namespace facebook::presto::test {

using namespace velox;

namespace {

template <typename FunctionMap>
bool validateDefaultNamespacePrefix(
const FunctionMap& functionMap,
const std::string& prestoDefaultNamespacePrefix) {
static const std::unordered_set<std::string> kBlockList = {
"row_constructor", "in", "is_null"};

std::vector<std::string> prestoDefaultNamespacePrefixParts;
folly::split(
'.',
prestoDefaultNamespacePrefix,
prestoDefaultNamespacePrefixParts,
true);

for (const auto& [functionName, _] : functionMap) {
// Skip internal functions. They don't have any prefix.
if ((kBlockList.count(functionName) != 0) ||
(functionName.find("$internal$") != std::string::npos)) {
continue;
}

std::vector<std::string> parts;
folly::split('.', functionName, parts, true);
if ((parts.size() != 3) ||
(parts[0] != prestoDefaultNamespacePrefixParts[0]) ||
(parts[1] != prestoDefaultNamespacePrefixParts[1])) {
return false;
}
}
return true;
}
} // namespace

class ConfigTest : public testing::Test {
protected:
void setUpConfigFilePath() {
Expand Down Expand Up @@ -255,4 +297,35 @@ TEST_F(ConfigTest, optionalNodeId) {
EXPECT_EQ(nodeId, config.nodeId());
}

TEST_F(ConfigTest, prestoDefaultNamespacePrefix) {
SystemConfig config;
init(
config,
{{std::string(SystemConfig::kPrestoDefaultNamespacePrefix),
"presto.default"}});
std::string prestoBuiltinFunctionPrefix =
config.prestoDefaultNamespacePrefix();

velox::functions::prestosql::registerAllScalarFunctions(
prestoBuiltinFunctionPrefix);
velox::aggregate::prestosql::registerAllAggregateFunctions(
prestoBuiltinFunctionPrefix);
velox::window::prestosql::registerAllWindowFunctions(
prestoBuiltinFunctionPrefix);

// Get all registered scalar functions in Velox
auto scalarFunctions = getFunctionSignatures();
ASSERT_TRUE(validateDefaultNamespacePrefix(
scalarFunctions, prestoBuiltinFunctionPrefix));

// Get all registered aggregate functions in Velox
auto aggregateFunctions = exec::aggregateFunctions().copy();
ASSERT_TRUE(validateDefaultNamespacePrefix(
aggregateFunctions, prestoBuiltinFunctionPrefix));

// Get all registered window functions in Velox
auto windowFunctions = exec::windowFunctions();
ASSERT_TRUE(validateDefaultNamespacePrefix(
windowFunctions, prestoBuiltinFunctionPrefix));
}
} // namespace facebook::presto::test
Original file line number Diff line number Diff line change
Expand Up @@ -33,24 +33,41 @@ std::string toJsonString(const T& value) {
}

std::string mapScalarFunction(const std::string& name) {
static const std::string prestoDefaultNamespacePrefix =
SystemConfig::instance()->prestoDefaultNamespacePrefix();
static const std::unordered_map<std::string, std::string> kFunctionNames = {
// Operator overrides: com.facebook.presto.common.function.OperatorType
{"presto.default.$operator$add", "presto.default.plus"},
{"presto.default.$operator$between", "presto.default.between"},
{"presto.default.$operator$divide", "presto.default.divide"},
{"presto.default.$operator$equal", "presto.default.eq"},
{"presto.default.$operator$greater_than", "presto.default.gt"},
{"presto.default.$operator$greater_than_or_equal", "presto.default.gte"},
{"presto.default.$operator$add",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@pdabre12 : High level question about this... Should we be using prestoDefaultNamespacePrefix for the key function name in Velox as well ? Since we are registering functions with this prefix, then it should be used here as well.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aditi-pandit You are right. Thank you for catching this.
Updated my PR.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@aditi-pandit Reverted the changes related to the operators and magic literal functions. These functions aren't reported by Prestissimo. Since, these don't get reported by Prestissimo they won't be present in the namespace specified by the presto.default-namespace property for eg. native.default namespace. As the operator types and magic literal functions are always passed down by the Java coordinator, it can be a fair assumption that they will have the presto.default namespace prefix.

formatDefaultNamespacePrefix(prestoDefaultNamespacePrefix, "plus")},
{"presto.default.$operator$between",
formatDefaultNamespacePrefix(prestoDefaultNamespacePrefix, "between")},
{"presto.default.$operator$divide",
formatDefaultNamespacePrefix(prestoDefaultNamespacePrefix, "divide")},
{"presto.default.$operator$equal",
formatDefaultNamespacePrefix(prestoDefaultNamespacePrefix, "eq")},
{"presto.default.$operator$greater_than",
formatDefaultNamespacePrefix(prestoDefaultNamespacePrefix, "gt")},
{"presto.default.$operator$greater_than_or_equal",
formatDefaultNamespacePrefix(prestoDefaultNamespacePrefix, "gte")},
{"presto.default.$operator$is_distinct_from",
"presto.default.distinct_from"},
{"presto.default.$operator$less_than", "presto.default.lt"},
{"presto.default.$operator$less_than_or_equal", "presto.default.lte"},
{"presto.default.$operator$modulus", "presto.default.mod"},
{"presto.default.$operator$multiply", "presto.default.multiply"},
{"presto.default.$operator$negation", "presto.default.negate"},
{"presto.default.$operator$not_equal", "presto.default.neq"},
{"presto.default.$operator$subtract", "presto.default.minus"},
{"presto.default.$operator$subscript", "presto.default.subscript"},
formatDefaultNamespacePrefix(
prestoDefaultNamespacePrefix, "distinct_from")},
{"presto.default.$operator$less_than",
formatDefaultNamespacePrefix(prestoDefaultNamespacePrefix, "lt")},
{"presto.default.$operator$less_than_or_equal",
formatDefaultNamespacePrefix(prestoDefaultNamespacePrefix, "lte")},
{"presto.default.$operator$modulus",
formatDefaultNamespacePrefix(prestoDefaultNamespacePrefix, "mod")},
{"presto.default.$operator$multiply",
formatDefaultNamespacePrefix(prestoDefaultNamespacePrefix, "multiply")},
{"presto.default.$operator$negation",
formatDefaultNamespacePrefix(prestoDefaultNamespacePrefix, "negate")},
{"presto.default.$operator$not_equal",
formatDefaultNamespacePrefix(prestoDefaultNamespacePrefix, "neq")},
{"presto.default.$operator$subtract",
formatDefaultNamespacePrefix(prestoDefaultNamespacePrefix, "minus")},
{"presto.default.$operator$subscript",
formatDefaultNamespacePrefix(prestoDefaultNamespacePrefix, "subscript")},
// Special form function overrides.
{"presto.default.in", "in"},
};
Expand All @@ -66,11 +83,15 @@ std::string mapScalarFunction(const std::string& name) {
}

std::string mapAggregateOrWindowFunction(const std::string& name) {
static const std::string prestoDefaultNamespacePrefix =
SystemConfig::instance()->prestoDefaultNamespacePrefix();
static const std::unordered_map<std::string, std::string> kFunctionNames = {
{"presto.default.$internal$max_data_size_for_stats",
"presto.default.max_data_size_for_stats"},
formatDefaultNamespacePrefix(
prestoDefaultNamespacePrefix, "max_data_size_for_stats")},
{"presto.default.$internal$sum_data_size_for_stats",
"presto.default.sum_data_size_for_stats"},
formatDefaultNamespacePrefix(
prestoDefaultNamespacePrefix, "sum_data_size_for_stats")},
};
std::string lowerCaseName = boost::to_lower_copy(name);
auto it = kFunctionNames.find(name);
Expand Down Expand Up @@ -160,6 +181,8 @@ std::vector<TypedExprPtr> VeloxExprConverter::toVeloxExpr(

namespace {
static const char* kVarchar = "varchar";
static const std::string prestoDefaultNamespacePrefix =
SystemConfig::instance()->prestoDefaultNamespacePrefix();

/// Convert cast of varchar to substr if target type is varchar with max length.
/// Throw an exception for cast of varchar to varchar with max length.
Expand Down Expand Up @@ -190,7 +213,7 @@ std::optional<TypedExprPtr> convertCastToVarcharWithMaxLength(
std::make_shared<ConstantTypedExpr>(velox::BIGINT(), 1LL),
std::make_shared<ConstantTypedExpr>(velox::BIGINT(), (int64_t)length),
},
"presto.default.substr");
formatDefaultNamespacePrefix(prestoDefaultNamespacePrefix, "substr"));
}

/// Converts cast and try_cast functions to CastTypedExpr with nullOnFailure
Expand All @@ -208,7 +231,8 @@ std::optional<TypedExprPtr> tryConvertCast(
const std::vector<TypedExprPtr>& args,
const TypeParser* typeParser) {
static const char* kCast = "presto.default.$operator$cast";
static const char* kTryCast = "presto.default.try_cast";
static const std::string kTryCast =
formatDefaultNamespacePrefix(prestoDefaultNamespacePrefix, "try_cast");
static const char* kJsonToArrayCast =
"presto.default.$internal$json_string_to_array_cast";
static const char* kJsonToMapCast =
Expand Down Expand Up @@ -237,7 +261,10 @@ std::optional<TypedExprPtr> tryConvertCast(
return std::make_shared<CastTypedExpr>(
type,
std::vector<TypedExprPtr>{std::make_shared<CallTypedExpr>(
velox::JSON(), args, "presto.default.json_parse")},
velox::JSON(),
args,
formatDefaultNamespacePrefix(
prestoDefaultNamespacePrefix, "json_parse"))},
false);
} else {
return std::nullopt;
Expand Down Expand Up @@ -301,7 +328,8 @@ std::optional<TypedExprPtr> tryConvertLiteralArray(
velox::memory::MemoryPool* pool,
const TypeParser* typeParser) {
static const char* kLiteralArray = "presto.default.$literal$array";
static const char* kFromBase64 = "presto.default.from_base64";
static const std::string kFromBase64 =
formatDefaultNamespacePrefix(prestoDefaultNamespacePrefix, "from_base64");

if (signature.kind != protocol::FunctionKind::SCALAR) {
return std::nullopt;
Expand Down Expand Up @@ -342,7 +370,10 @@ std::optional<TypedExprPtr> tryConvertLiteralArray(

std::optional<TypedExprPtr> VeloxExprConverter::tryConvertDate(
const protocol::CallExpression& pexpr) const {
static const char* kDate = "presto.default.date";
static const std::string prestoDefaultNamespacePrefix =
SystemConfig::instance()->prestoDefaultNamespacePrefix();
static const std::string kDate =
formatDefaultNamespacePrefix(prestoDefaultNamespacePrefix, "date");

auto builtin = std::static_pointer_cast<protocol::BuiltInFunctionHandle>(
pexpr.functionHandle);
Expand All @@ -363,8 +394,12 @@ std::optional<TypedExprPtr> VeloxExprConverter::tryConvertDate(

std::optional<TypedExprPtr> VeloxExprConverter::tryConvertLike(
const protocol::CallExpression& pexpr) const {
static const char* kLike = "presto.default.like";
static const char* kLikePatternType = "presto.default.like_pattern";
static const std::string prestoDefaultNamespacePrefix =
SystemConfig::instance()->prestoDefaultNamespacePrefix();
static const std::string kLike =
formatDefaultNamespacePrefix(prestoDefaultNamespacePrefix, "like");
static const std::string kLikePatternType = formatDefaultNamespacePrefix(
prestoDefaultNamespacePrefix, "like_pattern");
static const char* kLikeReturnType = "LikePattern";
static const char* kCast = "presto.default.$operator$cast";

Expand Down Expand Up @@ -500,9 +535,13 @@ bool isTrueConstant(const TypedExprPtr& expression) {
std::shared_ptr<const CallTypedExpr> makeEqualsExpr(
const TypedExprPtr& a,
const TypedExprPtr& b) {
static const std::string prestoDefaultNamespacePrefix =
SystemConfig::instance()->prestoDefaultNamespacePrefix();
std::vector<TypedExprPtr> inputs{a, b};
return std::make_shared<CallTypedExpr>(
velox::BOOLEAN(), std::move(inputs), "presto.default.eq");
velox::BOOLEAN(),
std::move(inputs),
formatDefaultNamespacePrefix(prestoDefaultNamespacePrefix, "eq"));
}

std::shared_ptr<const CastTypedExpr> makeCastExpr(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -14,12 +14,21 @@
#pragma once

#include <stdexcept>
#include "presto_cpp/main/common/Configs.h"
#include "presto_cpp/main/types/TypeParser.h"
#include "presto_cpp/presto_protocol/presto_protocol.h"
#include "velox/core/Expressions.h"

namespace facebook::presto {

namespace {
inline std::string formatDefaultNamespacePrefix(
const std::string& functionName,
const std::string& prestoDefaultNamespacePrefix) {
return fmt::format("{}{}", prestoDefaultNamespacePrefix, functionName);
}
} // namespace

class VeloxExprConverter {
public:
VeloxExprConverter(velox::memory::MemoryPool* pool, TypeParser* typeParser)
Expand Down
Loading