diff --git a/.github/workflows/prestocpp-linux-build-and-unit-test.yml b/.github/workflows/prestocpp-linux-build-and-unit-test.yml index 566146da290fd..aaba200c2f749 100644 --- a/.github/workflows/prestocpp-linux-build-and-unit-test.yml +++ b/.github/workflows/prestocpp-linux-build-and-unit-test.yml @@ -353,7 +353,7 @@ jobs: github.event_name == 'schedule' || needs.changes.outputs.codechange == 'true' run: | export PRESTO_SERVER_PATH="${GITHUB_WORKSPACE}/presto-native-execution/_build/release/presto_cpp/main/presto_server" - export TESTFILES=`find ./presto-native-tests/src/test -type f -name 'Test*.java'` + export TESTFILES=`find ./presto-native-tests/src/test -type f -name 'Test*.java' ! -path "*/presto-native-tests/src/test/java/com/facebook/presto/nativetests/cudf/*"` # Convert file paths to comma separated class names export TESTCLASSES= for test_file in $TESTFILES diff --git a/.gitmodules b/.gitmodules index 6fb925ff13ecf..eaa728d7dea11 100644 --- a/.gitmodules +++ b/.gitmodules @@ -1,3 +1,3 @@ [submodule "presto-native-execution/velox"] path = presto-native-execution/velox - url = https://github.com/facebookincubator/velox.git + url = https://github.com/pramodsatya/velox.git diff --git a/presto-native-execution/presto_cpp/main/CMakeLists.txt b/presto-native-execution/presto_cpp/main/CMakeLists.txt index 7de56fb0379c7..20b13c1787c6f 100644 --- a/presto-native-execution/presto_cpp/main/CMakeLists.txt +++ b/presto-native-execution/presto_cpp/main/CMakeLists.txt @@ -17,10 +17,7 @@ add_subdirectory(thrift) add_subdirectory(connectors) add_subdirectory(functions) add_subdirectory(tool) - -add_library(presto_session_properties SessionProperties.cpp) - -target_link_libraries(presto_session_properties ${FOLLY_WITH_DEPENDENCIES}) +add_subdirectory(properties) add_library( presto_server_lib @@ -66,7 +63,6 @@ target_link_libraries( presto_connectors presto_http presto_operators - presto_session_properties presto_velox_plan_conversion presto_hive_functions presto_theta_sketch_functions @@ -112,6 +108,10 @@ target_link_libraries( pthread ) +# presto_cudf_session_properties is always linked: velox_cudf_config is plain +# C++ (no CUDA), enabling the sidecar to expose cuDF session properties. +target_link_libraries(presto_server_lib presto_cudf_session_properties) + if(PRESTO_ENABLE_CUDF) target_link_libraries(presto_server_lib velox_cudf_exec) endif() diff --git a/presto-native-execution/presto_cpp/main/PrestoServer.cpp b/presto-native-execution/presto_cpp/main/PrestoServer.cpp index 358baf53cde32..4648c5f5f5413 100644 --- a/presto-native-execution/presto_cpp/main/PrestoServer.cpp +++ b/presto-native-execution/presto_cpp/main/PrestoServer.cpp @@ -22,7 +22,6 @@ #include "presto_cpp/main/CoordinatorDiscoverer.h" #include "presto_cpp/main/PeriodicMemoryChecker.h" #include "presto_cpp/main/PeriodicTaskManager.h" -#include "presto_cpp/main/SessionProperties.h" #include "presto_cpp/main/SignalHandler.h" #include "presto_cpp/main/TaskResource.h" #include "presto_cpp/main/common/ConfigReader.h" @@ -44,6 +43,7 @@ #include "presto_cpp/main/operators/ShuffleExchangeSource.h" #include "presto_cpp/main/operators/ShuffleRead.h" #include "presto_cpp/main/operators/ShuffleWrite.h" +#include "presto_cpp/main/properties/session/SessionProperties.h" #include "presto_cpp/main/types/ExpressionOptimizer.h" #include "presto_cpp/main/types/PrestoToVeloxQueryPlan.h" #include "presto_cpp/main/types/VeloxPlanConversion.h" @@ -76,8 +76,11 @@ #include "velox/serializers/PrestoSerializer.h" #include "velox/serializers/UnsafeRowSerializer.h" +// CudfSessionProperties and CudfConfig are plain C++ (no CUDA required). +// Always include them so the sidecar session endpoint exposes cuDF properties. +#include "presto_cpp/main/properties/session/CudfSessionProperties.h" +#include "velox/experimental/cudf/common/CudfConfig.h" #ifdef PRESTO_ENABLE_CUDF -#include "velox/experimental/cudf/CudfConfig.h" #include "velox/experimental/cudf/exec/ToCudf.h" #endif @@ -172,16 +175,15 @@ bool isSharedLibrary(const fs::path& path) { void registerVeloxCudf() { #ifdef PRESTO_ENABLE_CUDF - // Disable by default. - velox::cudf_velox::CudfConfig::getInstance().enabled = false; auto systemConfig = SystemConfig::instance(); - velox::cudf_velox::CudfConfig::getInstance().functionNamePrefix = - systemConfig->prestoDefaultNamespacePrefix(); if (systemConfig->values().contains( - velox::cudf_velox::CudfConfig::kCudfEnabled)) { - velox::cudf_velox::CudfConfig::getInstance().initialize( + velox::cudf_velox::CudfSystemConfig::kCudfEnabledEntry.name)) { + velox::cudf_velox::CudfSystemConfig::getInstance().updateConfigs( systemConfig->values()); - if (velox::cudf_velox::CudfConfig::getInstance().enabled) { + /// TODO(ps): Deprecate cudf.enabled from query config. + if (velox::cudf_velox::CudfSystemConfig::getInstance().cudfEnabled() || + velox::cudf_velox::CudfQueryConfig::getInstance().get( + velox::cudf_velox::CudfQueryConfig::kCudfEnabledEntry)) { velox::cudf_velox::registerCudf(); PRESTO_STARTUP_LOG(INFO) << "cuDF is registered."; } @@ -193,8 +195,9 @@ void unregisterVeloxCudf() { #ifdef PRESTO_ENABLE_CUDF auto systemConfig = SystemConfig::instance(); if (systemConfig->values().contains( - velox::cudf_velox::CudfConfig::kCudfEnabled) && - velox::cudf_velox::CudfConfig::getInstance().enabled) { + velox::cudf_velox::CudfSystemConfig::kCudfEnabledEntry.name) && + velox::cudf_velox::CudfSystemConfig::getInstance().get( + velox::cudf_velox::CudfSystemConfig::kCudfEnabledEntry)) { velox::cudf_velox::unregisterCudf(); PRESTO_SHUTDOWN_LOG(INFO) << "cuDF is unregistered."; } @@ -419,6 +422,15 @@ void PrestoServer::initializeConfigs() { nodeLocation_ = nodeConfig->nodeLocation(); nodePoolType_ = systemConfig->poolType(); prestoBuiltinFunctionPrefix_ = systemConfig->prestoDefaultNamespacePrefix(); + +#ifdef PRESTO_ENABLE_CUDF + // Set default values for cuDF in system config + velox::cudf_velox::CudfSystemConfig::getInstance().set( + velox::cudf_velox::CudfSystemConfig::kCudfEnabledEntry.name, "false"); + velox::cudf_velox::CudfSystemConfig::getInstance().set( + velox::cudf_velox::CudfSystemConfig::kCudfFunctionNamePrefixEntry.name, + prestoBuiltinFunctionPrefix_); +#endif } catch (const velox::VeloxUserError& e) { PRESTO_STARTUP_LOG(ERROR) << "Failed to start server due to " << e.what(); exit(EXIT_FAILURE); @@ -1830,9 +1842,11 @@ void PrestoServer::registerSidecarEndpoints() { proxygen::HTTPMessage* /*message*/, const std::vector>& /*body*/, proxygen::ResponseHandler* downstream) { - const auto* sessionProperties = SessionProperties::instance(); + const auto* sessionProperties = + facebook::presto::cudf::CudfSessionProperties::instance(); http::sendOkResponse(downstream, sessionProperties->serialize()); }); + httpServer_->registerGet( "/v1/functions", [](proxygen::HTTPMessage* /*message*/, diff --git a/presto-native-execution/presto_cpp/main/PrestoToVeloxQueryConfig.cpp b/presto-native-execution/presto_cpp/main/PrestoToVeloxQueryConfig.cpp index 0c3859b6c93e2..d7c2d6ba50884 100644 --- a/presto-native-execution/presto_cpp/main/PrestoToVeloxQueryConfig.cpp +++ b/presto-native-execution/presto_cpp/main/PrestoToVeloxQueryConfig.cpp @@ -13,10 +13,13 @@ */ #include "presto_cpp/main/PrestoToVeloxQueryConfig.h" -#include "presto_cpp/main/SessionProperties.h" +#include #include "presto_cpp/main/common/Configs.h" +#include "presto_cpp/main/properties/session/CudfSessionProperties.h" +#include "presto_cpp/main/properties/session/SessionProperties.h" #include "velox/common/compression/Compression.h" #include "velox/core/QueryConfig.h" +#include "velox/experimental/cudf/common/CudfConfig.h" #include "velox/type/tz/TimeZoneMap.h" namespace facebook::presto { @@ -239,8 +242,30 @@ void updateFromSystemConfigs( } } } + } // namespace +velox::cudf_velox::CudfQueryConfig toCudfConfigs( + const protocol::SessionRepresentation& session) { + std::unordered_map cudfConfigs; + auto* cudfSessionProperties = cudf::CudfSessionProperties::instance(); + + // Iterate through session properties and extract cuDF configs + for (const auto& [sessionPropName, sessionPropValue] : + session.systemProperties) { + // Use toVeloxConfig to get the mapped Velox config name + // CudfConfig::updateConfigs() will only process keys it recognizes + const auto veloxConfigName = + cudfSessionProperties->toVeloxConfig(sessionPropName); + + if (!veloxConfigName.empty()) { + cudfConfigs[veloxConfigName] = sessionPropValue; + } + } + + return velox::cudf_velox::CudfQueryConfig(std::move(cudfConfigs)); +} + std::unordered_map toVeloxConfigs( const protocol::SessionRepresentation& session) { std::unordered_map configs; diff --git a/presto-native-execution/presto_cpp/main/PrestoToVeloxQueryConfig.h b/presto-native-execution/presto_cpp/main/PrestoToVeloxQueryConfig.h index 8e6b4cbf9261e..7efbe7e244002 100644 --- a/presto-native-execution/presto_cpp/main/PrestoToVeloxQueryConfig.h +++ b/presto-native-execution/presto_cpp/main/PrestoToVeloxQueryConfig.h @@ -23,6 +23,10 @@ namespace facebook::velox::core { class QueryConfig; } +namespace facebook::velox::cudf_velox { +class CudfQueryConfig; +} + namespace facebook::presto { /// Translates Presto configs to Velox 'QueryConfig' config map. Presto query @@ -30,6 +34,10 @@ namespace facebook::presto { std::unordered_map toVeloxConfigs( const protocol::SessionRepresentation& session); +/// Translates Presto session properties to Velox query level cuDF configs. +velox::cudf_velox::CudfQueryConfig toCudfConfigs( + const protocol::SessionRepresentation& session); + /// Translates Presto configs to Velox 'QueryConfig' config map. It is the /// temporary overload that builds a QueryConfig from session properties and /// extraCredentials, including all extraCredentials so they can be consumed by diff --git a/presto-native-execution/presto_cpp/main/QueryContextManager.cpp b/presto-native-execution/presto_cpp/main/QueryContextManager.cpp index 3598603c300eb..97aa1f7e85b4d 100644 --- a/presto-native-execution/presto_cpp/main/QueryContextManager.cpp +++ b/presto-native-execution/presto_cpp/main/QueryContextManager.cpp @@ -14,11 +14,14 @@ #include "presto_cpp/main/QueryContextManager.h" #include +#include #include "presto_cpp/main/PrestoToVeloxQueryConfig.h" -#include "presto_cpp/main/SessionProperties.h" #include "presto_cpp/main/common/Configs.h" +#include "presto_cpp/main/properties/session/SessionProperties.h" #include "velox/connectors/hive/HiveConfig.h" #include "velox/core/QueryConfig.h" +// CudfConfig is plain C++ (no CUDA) - always include for CudfQueryConfig type. +#include "velox/experimental/cudf/common/CudfConfig.h" using namespace facebook::velox; @@ -110,6 +113,7 @@ QueryContextManager::findOrCreateQueryCtx( taskId, toVeloxConfigs( taskUpdateRequest.session, taskUpdateRequest.extraCredentials), + toCudfConfigs(taskUpdateRequest.session), toConnectorConfigs(taskUpdateRequest)); } @@ -122,6 +126,7 @@ QueryContextManager::findOrCreateBatchQueryCtx( taskId, toVeloxConfigs( taskUpdateRequest.session, taskUpdateRequest.extraCredentials), + toCudfConfigs(taskUpdateRequest.session), toConnectorConfigs(taskUpdateRequest)); if (queryCtx->pool()->aborted()) { // In Batch mode, only one query is running at a time. When tasks fail @@ -138,6 +143,7 @@ QueryContextManager::findOrCreateBatchQueryCtx( taskId, toVeloxConfigs( taskUpdateRequest.session, taskUpdateRequest.extraCredentials), + toCudfConfigs(taskUpdateRequest.session), toConnectorConfigs(taskUpdateRequest)); } return queryCtx; @@ -159,6 +165,7 @@ std::shared_ptr QueryContextManager::createAndCacheQueryCtxLocked( const QueryId& queryId, velox::core::QueryConfig&& queryConfig, + velox::cudf_velox::CudfQueryConfig&& cudfConfigs, std::unordered_map>&& connectorConfigs, std::shared_ptr&& pool) { @@ -169,13 +176,16 @@ QueryContextManager::createAndCacheQueryCtxLocked( cache::AsyncDataCache::getInstance(), std::move(pool), spillerExecutor_, - queryId); + queryId, + /* tokenProvider */ nullptr, + std::move(cudfConfigs)); return queryContextCache_.insert(queryId, std::move(queryCtx)); } std::shared_ptr QueryContextManager::findOrCreateQueryCtxLocked( const TaskId& taskId, velox::core::QueryConfig&& queryConfig, + velox::cudf_velox::CudfQueryConfig&& cudfConfigs, std::unordered_map>&& connectorConfigs) { const QueryId queryId{queryIdFromTaskId(taskId)}; @@ -208,6 +218,7 @@ std::shared_ptr QueryContextManager::findOrCreateQueryCtxLocked( return createAndCacheQueryCtxLocked( queryId, std::move(queryConfig), + std::move(cudfConfigs), std::move(connectorConfigs), std::move(pool)); } diff --git a/presto-native-execution/presto_cpp/main/QueryContextManager.h b/presto-native-execution/presto_cpp/main/QueryContextManager.h index 3a07b73c36335..d4220829e710f 100644 --- a/presto-native-execution/presto_cpp/main/QueryContextManager.h +++ b/presto-native-execution/presto_cpp/main/QueryContextManager.h @@ -23,6 +23,10 @@ #include "presto_cpp/presto_protocol/core/presto_protocol_core.h" #include "velox/core/QueryCtx.h" +namespace facebook::velox::cudf_velox { +class CudfQueryConfig; +} + namespace facebook::presto { class QueryContextCache { public: @@ -112,6 +116,7 @@ class QueryContextManager { virtual std::shared_ptr createAndCacheQueryCtxLocked( const protocol::QueryId& queryId, velox::core::QueryConfig&& queryConfig, + velox::cudf_velox::CudfQueryConfig&& cudfConfigs, std::unordered_map< std::string, std::shared_ptr>&& connectorConfigs, @@ -120,6 +125,7 @@ class QueryContextManager { std::shared_ptr findOrCreateQueryCtxLocked( const protocol::TaskId& taskId, velox::core::QueryConfig&& queryConfig, + velox::cudf_velox::CudfQueryConfig&& cudfConfigs, std::unordered_map< std::string, std::shared_ptr>&& connectorConfigStrings); diff --git a/presto-native-execution/presto_cpp/main/common/Utils.cpp b/presto-native-execution/presto_cpp/main/common/Utils.cpp index 6befe209fc0b8..e9c53b7df586b 100644 --- a/presto-native-execution/presto_cpp/main/common/Utils.cpp +++ b/presto-native-execution/presto_cpp/main/common/Utils.cpp @@ -23,6 +23,10 @@ namespace facebook::presto::util { +std::string boolToString(bool value) { + return value ? "true" : "false"; +} + DateTime toISOTimestamp(uint64_t timeMilli) { char buf[80]; time_t timeSecond = timeMilli / 1000; diff --git a/presto-native-execution/presto_cpp/main/common/Utils.h b/presto-native-execution/presto_cpp/main/common/Utils.h index 60e0a1a4a32d7..baf2022be883d 100644 --- a/presto-native-execution/presto_cpp/main/common/Utils.h +++ b/presto-native-execution/presto_cpp/main/common/Utils.h @@ -25,6 +25,9 @@ namespace facebook::presto::util { #define PRESTO_SHUTDOWN_LOG(severity) \ LOG(severity) << PRESTO_SHUTDOWN_LOG_PREFIX +/// Convert boolean to lowercase string representation. +std::string boolToString(bool value); + using DateTime = std::string; DateTime toISOTimestamp(uint64_t timeMilli); diff --git a/presto-native-execution/presto_cpp/main/connectors/Registration.cpp b/presto-native-execution/presto_cpp/main/connectors/Registration.cpp index c70b45db839a2..78230d15aa74d 100644 --- a/presto-native-execution/presto_cpp/main/connectors/Registration.cpp +++ b/presto-native-execution/presto_cpp/main/connectors/Registration.cpp @@ -26,7 +26,7 @@ #include "velox/connectors/tpcds/TpcdsConnector.h" #include "velox/connectors/tpch/TpchConnector.h" #ifdef PRESTO_ENABLE_CUDF -#include "velox/experimental/cudf/CudfConfig.h" +#include "velox/experimental/cudf/common/CudfConfig.h" #include "velox/experimental/cudf/connectors/hive/CudfHiveConnector.h" #endif diff --git a/presto-native-execution/presto_cpp/main/properties/CMakeLists.txt b/presto-native-execution/presto_cpp/main/properties/CMakeLists.txt new file mode 100644 index 0000000000000..15a18ff510afa --- /dev/null +++ b/presto-native-execution/presto_cpp/main/properties/CMakeLists.txt @@ -0,0 +1,13 @@ +# 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. + +add_subdirectory(session) diff --git a/presto-native-execution/presto_cpp/main/properties/session/CMakeLists.txt b/presto-native-execution/presto_cpp/main/properties/session/CMakeLists.txt new file mode 100644 index 0000000000000..e3969b6e9ae13 --- /dev/null +++ b/presto-native-execution/presto_cpp/main/properties/session/CMakeLists.txt @@ -0,0 +1,39 @@ +# 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. + +add_library(presto_session_properties_provider SessionPropertiesProvider.cpp) + +target_link_libraries( + presto_session_properties_provider + ${FOLLY_WITH_DEPENDENCIES} + presto_protocol + presto_common +) + +add_library(presto_session_properties SessionProperties.cpp) + +target_link_libraries(presto_session_properties presto_session_properties_provider) + +# presto_cudf_session_properties is always built (velox_cudf_config is plain +# C++ with no CUDA requirement) so the sidecar can expose cuDF session props. +add_library(presto_cudf_session_properties CudfSessionProperties.cpp) + +target_link_libraries( + presto_cudf_session_properties + presto_session_properties_provider + velox_cudf_config + ${FOLLY_WITH_DEPENDENCIES} +) + +if(PRESTO_ENABLE_TESTING) + add_subdirectory(tests) +endif() diff --git a/presto-native-execution/presto_cpp/main/properties/session/CudfSessionProperties.cpp b/presto-native-execution/presto_cpp/main/properties/session/CudfSessionProperties.cpp new file mode 100644 index 0000000000000..0a5c8b4e8e031 --- /dev/null +++ b/presto-native-execution/presto_cpp/main/properties/session/CudfSessionProperties.cpp @@ -0,0 +1,59 @@ +/* + * 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 "presto_cpp/main/properties/session/CudfSessionProperties.h" +#include "velox/experimental/cudf/common/CudfConfig.h" + +#include +#include + +#include + +namespace facebook::presto::cudf { + +using namespace facebook::velox; + +CudfSessionProperties* CudfSessionProperties::instance() { + static std::unique_ptr instance = + std::make_unique(); + return instance.get(); +} + +// Initialize GPU session properties from cuDF configuration +CudfSessionProperties::CudfSessionProperties() { + using facebook::velox::cudf_velox::CudfQueryConfig; + + auto sanitizeAndAddSessionProperty = + [this](const CudfQueryConfig::CudfConfigEntry& entry) { + auto sessionPropertyName = entry.name; + if (!boost::algorithm::starts_with(sessionPropertyName, "cudf.")) { + sessionPropertyName = fmt::format("cudf_{}", sessionPropertyName); + } + boost::algorithm::replace_all(sessionPropertyName, ".", "_"); + boost::algorithm::replace_all(sessionPropertyName, "-", "_"); + + addSessionProperty( + sessionPropertyName, + "cuDF configuration property mapped from Velox", + entry.type, + false, + entry.name, + entry.defaultValue); + }; + + for (const auto& entry : CudfQueryConfig::getInstance().getConfigs()) { + sanitizeAndAddSessionProperty(entry); + } +} + +} // namespace facebook::presto::cudf diff --git a/presto-native-execution/presto_cpp/main/properties/session/CudfSessionProperties.h b/presto-native-execution/presto_cpp/main/properties/session/CudfSessionProperties.h new file mode 100644 index 0000000000000..45c970e66faed --- /dev/null +++ b/presto-native-execution/presto_cpp/main/properties/session/CudfSessionProperties.h @@ -0,0 +1,75 @@ +/* + * 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. + */ +#pragma once + +#include "presto_cpp/external/json/nlohmann/json.hpp" +#include "presto_cpp/main/properties/session/SessionPropertiesProvider.h" +#include "presto_cpp/presto_protocol/core/presto_protocol_core.h" +#include "velox/type/Type.h" + +using json = nlohmann::json; + +namespace facebook::presto::cudf { + +/// Defines all cuDF GPU-specific session properties +class CudfSessionProperties + : public facebook::presto::SessionPropertiesProvider { + public: + /// Enable cuDF GPU acceleration + static constexpr const char* kCudfEnabled = "cudf_enabled"; + + /// Enable debug mode for cuDF operations + static constexpr const char* kCudfDebugEnabled = "cudf_debug_enabled"; + + /// GPU memory resource type + static constexpr const char* kCudfMemoryResource = "cudf_memory_resource"; + + /// GPU memory allocation percentage + static constexpr const char* kCudfMemoryPercent = "cudf_memory_percent"; + + /// Function name prefix for cuDF functions + static constexpr const char* kCudfFunctionNamePrefix = + "cudf_function_name_prefix"; + + /// Enable AST expression evaluation on GPU + static constexpr const char* kCudfAstExpressionEnabled = + "cudf_ast_expression_enabled"; + + /// Priority of AST expression evaluation + static constexpr const char* kCudfAstExpressionPriority = + "cudf_ast_expression_priority"; + + /// Enable JIT expression evaluation on GPU + static constexpr const char* kCudfJitExpressionEnabled = + "cudf_jit_expression_enabled"; + + /// Priority of JIT expression evaluation + static constexpr const char* kCudfJitExpressionPriority = + "cudf_jit_expression_priority"; + + /// Allow fallback to CPU execution if GPU operation fails + static constexpr const char* kCudfAllowCpuFallback = + "cudf_allow_cpu_fallback"; + + /// Log reasons for CPU fallback + static constexpr const char* kCudfLogFallback = "cudf_log_fallback"; + + /// Get singleton instance + static CudfSessionProperties* instance(); + + /// Constructor - initializes all GPU session properties from CudfConfig + CudfSessionProperties(); +}; + +} // namespace facebook::presto::cudf diff --git a/presto-native-execution/presto_cpp/main/SessionProperties.cpp b/presto-native-execution/presto_cpp/main/properties/session/SessionProperties.cpp similarity index 92% rename from presto-native-execution/presto_cpp/main/SessionProperties.cpp rename to presto-native-execution/presto_cpp/main/properties/session/SessionProperties.cpp index b9a45fd40226f..4c9c4985ae028 100644 --- a/presto-native-execution/presto_cpp/main/SessionProperties.cpp +++ b/presto-native-execution/presto_cpp/main/properties/session/SessionProperties.cpp @@ -11,36 +11,20 @@ * See the License for the specific language governing permissions and * limitations under the License. */ -#include "presto_cpp/main/SessionProperties.h" +#include "presto_cpp/main/properties/session/SessionProperties.h" +#include "presto_cpp/main/common/Utils.h" #include "velox/core/QueryConfig.h" using namespace facebook::velox; namespace facebook::presto { -namespace { -const std::string boolToString(bool value) { - return value ? "true" : "false"; -} -} // namespace - SessionProperties* SessionProperties::instance() { static std::unique_ptr instance = std::make_unique(); return instance.get(); } -void SessionProperties::addSessionProperty( - const std::string& name, - const std::string& description, - const TypePtr& type, - bool isHidden, - const std::optional veloxConfig, - const std::string& defaultValue) { - sessionProperties_[name] = std::make_shared( - name, description, type->toString(), isHidden, veloxConfig, defaultValue); -} - // List of native session properties is kept as the source of truth here. SessionProperties::SessionProperties() { using velox::core::QueryConfig; @@ -53,7 +37,7 @@ SessionProperties::SessionProperties() { BOOLEAN(), false, QueryConfig::kExprEvalSimplified, - boolToString(c.exprEvalSimplified())); + util::boolToString(c.exprEvalSimplified())); addSessionProperty( kExprMaxArraySizeInReduce, @@ -169,7 +153,7 @@ SessionProperties::SessionProperties() { BOOLEAN(), false, QueryConfig::kJoinSpillEnabled, - boolToString(c.joinSpillEnabled())); + util::boolToString(c.joinSpillEnabled())); addSessionProperty( kWindowSpillEnabled, @@ -177,7 +161,7 @@ SessionProperties::SessionProperties() { BOOLEAN(), false, QueryConfig::kWindowSpillEnabled, - boolToString(c.windowSpillEnabled())); + util::boolToString(c.windowSpillEnabled())); addSessionProperty( kWriterSpillEnabled, @@ -185,7 +169,7 @@ SessionProperties::SessionProperties() { BOOLEAN(), false, QueryConfig::kWriterSpillEnabled, - boolToString(c.writerSpillEnabled())); + util::boolToString(c.writerSpillEnabled())); addSessionProperty( kWriterFlushThresholdBytes, @@ -203,7 +187,7 @@ SessionProperties::SessionProperties() { BOOLEAN(), false, QueryConfig::kRowNumberSpillEnabled, - boolToString(c.rowNumberSpillEnabled())); + util::boolToString(c.rowNumberSpillEnabled())); addSessionProperty( kSpillerNumPartitionBits, @@ -220,7 +204,7 @@ SessionProperties::SessionProperties() { BOOLEAN(), false, QueryConfig::kTopNRowNumberSpillEnabled, - boolToString(c.topNRowNumberSpillEnabled())); + util::boolToString(c.topNRowNumberSpillEnabled())); addSessionProperty( kValidateOutputFromOperators, @@ -232,7 +216,7 @@ SessionProperties::SessionProperties() { BOOLEAN(), false, QueryConfig::kValidateOutputFromOperators, - boolToString(c.validateOutputFromOperators())); + util::boolToString(c.validateOutputFromOperators())); addSessionProperty( kDebugDisableExpressionWithPeeling, @@ -241,7 +225,7 @@ SessionProperties::SessionProperties() { BOOLEAN(), false, QueryConfig::kDebugDisableExpressionWithPeeling, - boolToString(c.debugDisableExpressionsWithPeeling())); + util::boolToString(c.debugDisableExpressionsWithPeeling())); addSessionProperty( kDebugDisableCommonSubExpressions, @@ -251,7 +235,7 @@ SessionProperties::SessionProperties() { BOOLEAN(), false, QueryConfig::kDebugDisableCommonSubExpressions, - boolToString(c.debugDisableCommonSubExpressions())); + util::boolToString(c.debugDisableCommonSubExpressions())); addSessionProperty( kDebugDisableExpressionWithMemoization, @@ -262,7 +246,7 @@ SessionProperties::SessionProperties() { BOOLEAN(), false, QueryConfig::kDebugDisableExpressionWithMemoization, - boolToString(c.debugDisableExpressionsWithMemoization())); + util::boolToString(c.debugDisableExpressionsWithMemoization())); addSessionProperty( kDebugDisableExpressionWithLazyInputs, @@ -272,7 +256,7 @@ SessionProperties::SessionProperties() { BOOLEAN(), false, QueryConfig::kDebugDisableExpressionWithLazyInputs, - boolToString(c.debugDisableExpressionsWithLazyInputs())); + util::boolToString(c.debugDisableExpressionsWithLazyInputs())); addSessionProperty( kDebugMemoryPoolNameRegex, @@ -306,7 +290,7 @@ SessionProperties::SessionProperties() { BOOLEAN(), false, QueryConfig::kSelectiveNimbleReaderEnabled, - boolToString(c.selectiveNimbleReaderEnabled())); + util::boolToString(c.selectiveNimbleReaderEnabled())); addSessionProperty( kQueryTraceEnabled, @@ -314,7 +298,7 @@ SessionProperties::SessionProperties() { BOOLEAN(), false, QueryConfig::kQueryTraceEnabled, - boolToString(c.queryTraceEnabled())); + util::boolToString(c.queryTraceEnabled())); addSessionProperty( kQueryTraceDir, @@ -660,27 +644,7 @@ SessionProperties::SessionProperties() { BOOLEAN(), false, QueryConfig::kAggregationMemoryCompactionReclaimEnabled, - boolToString(c.aggregationMemoryCompactionReclaimEnabled())); -} - -const std::string SessionProperties::toVeloxConfig( - const std::string& name) const { - auto it = sessionProperties_.find(name); - if (it != sessionProperties_.end() && - it->second->getVeloxConfig().has_value()) { - return it->second->getVeloxConfig().value(); - } - return name; -} - -json SessionProperties::serialize() const { - json j = json::array(); - json tj; - for (const auto& sessionProperty : sessionProperties_) { - protocol::to_json(tj, sessionProperty.second->getMetadata()); - j.push_back(tj); - } - return j; + util::boolToString(c.aggregationMemoryCompactionReclaimEnabled())); } bool SessionProperties::useVeloxGeospatialJoin() const { diff --git a/presto-native-execution/presto_cpp/main/SessionProperties.h b/presto-native-execution/presto_cpp/main/properties/session/SessionProperties.h similarity index 86% rename from presto-native-execution/presto_cpp/main/SessionProperties.h rename to presto-native-execution/presto_cpp/main/properties/session/SessionProperties.h index aa8a5a93b3b14..8d0f1dcba300c 100644 --- a/presto-native-execution/presto_cpp/main/SessionProperties.h +++ b/presto-native-execution/presto_cpp/main/properties/session/SessionProperties.h @@ -13,65 +13,14 @@ */ #pragma once -#include "presto_cpp/external/json/nlohmann/json.hpp" -#include "presto_cpp/presto_protocol/core/presto_protocol_core.h" -#include "velox/type/Type.h" - -using json = nlohmann::json; +#include "presto_cpp/main/properties/session/SessionPropertiesProvider.h" namespace facebook::presto { -/// This is the interface of the session property. -/// Note: This interface should align with java coordinator. -class SessionProperty { - public: - SessionProperty( - const std::string& name, - const std::string& description, - const std::string& typeSignature, - bool hidden, - const std::optional veloxConfig, - const std::string& defaultValue) - : metadata_({name, description, typeSignature, defaultValue, hidden}), - veloxConfig_(veloxConfig), - value_(defaultValue) {} - - const protocol::SessionPropertyMetadata getMetadata() { - return metadata_; - } - - const std::optional getVeloxConfig() { - return veloxConfig_; - } - - const std::string getValue() { - return value_; - } - - void updateValue(const std::string& value) { - value_ = value; - } - - bool operator==(const SessionProperty& other) const { - const auto otherMetadata = other.metadata_; - return metadata_.name == otherMetadata.name && - metadata_.description == otherMetadata.description && - metadata_.typeSignature == otherMetadata.typeSignature && - metadata_.hidden == otherMetadata.hidden && - metadata_.defaultValue == otherMetadata.defaultValue && - veloxConfig_ == other.veloxConfig_; - } - - private: - const protocol::SessionPropertyMetadata metadata_; - const std::optional veloxConfig_; - std::string value_; -}; - /// Defines all system session properties supported by native worker to ensure /// that they are the source of truth and to differentiate them from Java based /// session properties. Also maps the native session properties to velox. -class SessionProperties { +class SessionProperties : public SessionPropertiesProvider { public: /// Enable simplified path in expression evaluation. static constexpr const char* kExprEvalSimplified = @@ -426,47 +375,11 @@ class SessionProperties { static constexpr const char* kAggregationMemoryCompactionReclaimEnabled = "native_aggregation_memory_compaction_reclaim_enabled"; - inline bool hasVeloxConfig(const std::string& key) { - auto sessionProperty = sessionProperties_.find(key); - if (sessionProperty == sessionProperties_.end()) { - // In this case a queryConfig is being created so we should return - // true since it will also have a veloxConfig. - return true; - } - return sessionProperty->second->getVeloxConfig().has_value(); - } - - inline void updateSessionPropertyValue( - const std::string& key, - const std::string& value) { - auto sessionProperty = sessionProperties_.find(key); - VELOX_CHECK(sessionProperty != sessionProperties_.end()); - sessionProperty->second->updateValue(value); - } - static SessionProperties* instance(); SessionProperties(); - /// Utility function to translate a config name in Presto to its equivalent in - /// Velox. Returns 'name' as is if there is no mapping. - const std::string toVeloxConfig(const std::string& name) const; - - json serialize() const; - bool useVeloxGeospatialJoin() const; - - private: - void addSessionProperty( - const std::string& name, - const std::string& description, - const velox::TypePtr& type, - bool isHidden, - const std::optional veloxConfig, - const std::string& defaultValue); - - std::unordered_map> - sessionProperties_; }; } // namespace facebook::presto diff --git a/presto-native-execution/presto_cpp/main/properties/session/SessionPropertiesProvider.cpp b/presto-native-execution/presto_cpp/main/properties/session/SessionPropertiesProvider.cpp new file mode 100644 index 0000000000000..df62dfe236150 --- /dev/null +++ b/presto-native-execution/presto_cpp/main/properties/session/SessionPropertiesProvider.cpp @@ -0,0 +1,58 @@ +/* + * 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 "presto_cpp/main/properties/session/SessionPropertiesProvider.h" +#include +#include "presto_cpp/main/common/Utils.h" +#include "presto_cpp/presto_protocol/core/presto_protocol_core.h" + +namespace facebook::presto { + +void SessionPropertiesProvider::addSessionProperty( + const std::string& name, + const std::string& description, + const facebook::velox::TypePtr& type, + bool isHidden, + const std::optional veloxConfig, + const std::string& defaultValue) { + sessionProperties_[name] = std::make_shared( + name, + description, + boost::algorithm::to_lower_copy(type->toString()), + isHidden, + veloxConfig, + defaultValue); +} + +const std::string SessionPropertiesProvider::toVeloxConfig( + const std::string& name) const { + auto it = sessionProperties_.find(name); + if (it != sessionProperties_.end() && + it->second->getVeloxConfig().has_value()) { + return it->second->getVeloxConfig().value(); + } + return name; +} + +json SessionPropertiesProvider::serialize() const { + json j = json::array(); + json tj; + for (const auto& sessionProperty : sessionProperties_) { + protocol::to_json(tj, sessionProperty.second->getMetadata()); + j.push_back(tj); + } + return j; +} + +} // namespace facebook::presto diff --git a/presto-native-execution/presto_cpp/main/properties/session/SessionPropertiesProvider.h b/presto-native-execution/presto_cpp/main/properties/session/SessionPropertiesProvider.h new file mode 100644 index 0000000000000..29ba325eadb49 --- /dev/null +++ b/presto-native-execution/presto_cpp/main/properties/session/SessionPropertiesProvider.h @@ -0,0 +1,123 @@ +/* + * 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. + */ + +#pragma once + +#include +#include +#include "presto_cpp/external/json/nlohmann/json.hpp" +#include "presto_cpp/presto_protocol/core/presto_protocol_core.h" +#include "velox/type/Type.h" + +using json = nlohmann::json; + +namespace facebook::presto { + +/// This is the interface of the session property. +/// Note: This interface should align with java coordinator. +class SessionProperty { + public: + SessionProperty( + const std::string& name, + const std::string& description, + const std::string& typeSignature, + bool hidden, + const std::optional veloxConfig, + const std::string& defaultValue) + : metadata_({name, description, typeSignature, defaultValue, hidden}), + veloxConfig_(veloxConfig), + value_(defaultValue) {} + + const protocol::SessionPropertyMetadata getMetadata() { + return metadata_; + } + + const std::optional getVeloxConfig() { + return veloxConfig_; + } + + const std::string getValue() { + return value_; + } + + void updateValue(const std::string& value) { + value_ = value; + } + + bool operator==(const SessionProperty& other) const { + const auto otherMetadata = other.metadata_; + return metadata_.name == otherMetadata.name && + metadata_.description == otherMetadata.description && + metadata_.typeSignature == otherMetadata.typeSignature && + metadata_.hidden == otherMetadata.hidden && + metadata_.defaultValue == otherMetadata.defaultValue && + veloxConfig_ == other.veloxConfig_; + } + + private: + const protocol::SessionPropertyMetadata metadata_; + const std::optional veloxConfig_; + std::string value_; +}; + +/// Base class providing default implementations for session property +/// management. Subclasses can specialize initialization while inheriting common +/// serialization and configuration mapping functionality. +class SessionPropertiesProvider { + public: + virtual ~SessionPropertiesProvider() = default; + + /// Translate a config name to its equivalent Velox config name. + /// Returns 'name' as is if there is no mapping. + const std::string toVeloxConfig(const std::string& name) const; + + /// Serialize all properties to JSON. + json serialize() const; + + /// Check if a property has a corresponding Velox config. + inline bool hasVeloxConfig(const std::string& key) { + auto sessionProperty = sessionProperties_.find(key); + if (sessionProperty == sessionProperties_.end()) { + // In this case a queryConfig is being created so we should return + // true since it will also have a veloxConfig. + return true; + } + return sessionProperty->second->getVeloxConfig().has_value(); + } + + /// Update the value of a session property. + inline void updateSessionPropertyValue( + const std::string& key, + const std::string& value) { + auto sessionProperty = sessionProperties_.find(key); + VELOX_CHECK(sessionProperty != sessionProperties_.end()); + sessionProperty->second->updateValue(value); + } + + protected: + /// Add a session property with metadata (for use by subclasses during init). + void addSessionProperty( + const std::string& name, + const std::string& description, + const velox::TypePtr& type, + bool isHidden, + const std::optional veloxConfig, + const std::string& defaultValue); + + /// Map of session property name to SessionProperty + std::unordered_map> + sessionProperties_; +}; + +} // namespace facebook::presto diff --git a/presto-native-execution/presto_cpp/main/properties/session/tests/CMakeLists.txt b/presto-native-execution/presto_cpp/main/properties/session/tests/CMakeLists.txt new file mode 100644 index 0000000000000..19a6456f5db5b --- /dev/null +++ b/presto-native-execution/presto_cpp/main/properties/session/tests/CMakeLists.txt @@ -0,0 +1,55 @@ +# 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. + +add_executable(presto_session_properties_test SessionPropertiesTest.cpp) + +add_test(NAME presto_session_properties_test COMMAND presto_session_properties_test) + +target_link_libraries( + presto_session_properties_test + presto_session_properties + $ + velox_core + velox_exec + velox_functions_prestosql + GTest::gmock + GTest::gtest + GTest::gtest_main + ${FOLLY_WITH_DEPENDENCIES} + velox_dwio_common + velox_dwio_common_exception +) + +if(PRESTO_ENABLE_CUDF) + add_executable(presto_cudf_session_properties_test CudfSessionPropertiesTest.cpp) + + add_test( + NAME presto_cudf_session_properties_test + COMMAND presto_cudf_session_properties_test + WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR} + ) + + target_link_libraries( + presto_cudf_session_properties_test + presto_cudf_session_properties + $ + velox_core + velox_cudf_exec + GTest::gmock + GTest::gtest + GTest::gtest_main + ${FOLLY_WITH_DEPENDENCIES} + velox_dwio_common + velox_dwio_common_exception + velox_hive_connector + ) +endif() diff --git a/presto-native-execution/presto_cpp/main/properties/session/tests/CudfSessionPropertiesTest.cpp b/presto-native-execution/presto_cpp/main/properties/session/tests/CudfSessionPropertiesTest.cpp new file mode 100644 index 0000000000000..2344b73cddb3e --- /dev/null +++ b/presto-native-execution/presto_cpp/main/properties/session/tests/CudfSessionPropertiesTest.cpp @@ -0,0 +1,57 @@ +/* + * 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 + +#include "presto_cpp/main/properties/session/CudfSessionProperties.h" +#include "velox/experimental/cudf/common/CudfConfig.h" + +using namespace facebook::presto::cudf; +using namespace facebook::velox; + +class CudfSessionPropertiesTest : public testing::Test {}; + +TEST_F(CudfSessionPropertiesTest, validateMapping) { + // Verify that each Presto session property maps to the correct Velox + // CudfQueryConfig key + const std::unordered_map expectedMappings = { + {CudfSessionProperties::kCudfEnabled, + cudf_velox::CudfQueryConfig::kCudfEnabledEntry.name}, + {CudfSessionProperties::kCudfDebugEnabled, + cudf_velox::CudfQueryConfig::kCudfDebugEnabledEntry.name}, + {CudfSessionProperties::kCudfMemoryResource, + cudf_velox::CudfQueryConfig::kCudfMemoryResourceEntry.name}, + {CudfSessionProperties::kCudfMemoryPercent, + cudf_velox::CudfQueryConfig::kCudfMemoryPercentEntry.name}, + {CudfSessionProperties::kCudfFunctionNamePrefix, + cudf_velox::CudfQueryConfig::kCudfFunctionNamePrefixEntry.name}, + {CudfSessionProperties::kCudfAstExpressionEnabled, + cudf_velox::CudfQueryConfig::kCudfAstExpressionEnabledEntry.name}, + {CudfSessionProperties::kCudfAstExpressionPriority, + cudf_velox::CudfQueryConfig::kCudfAstExpressionPriorityEntry.name}, + {CudfSessionProperties::kCudfJitExpressionEnabled, + cudf_velox::CudfQueryConfig::kCudfJitExpressionEnabledEntry.name}, + {CudfSessionProperties::kCudfJitExpressionPriority, + cudf_velox::CudfQueryConfig::kCudfJitExpressionPriorityEntry.name}, + {CudfSessionProperties::kCudfAllowCpuFallback, + cudf_velox::CudfQueryConfig::kCudfAllowCpuFallbackEntry.name}, + {CudfSessionProperties::kCudfLogFallback, + cudf_velox::CudfQueryConfig::kCudfLogFallbackEntry.name}, + }; + + const auto sessionProperties = CudfSessionProperties::instance(); + for (const auto& [sessionProperty, expectedVeloxConfig] : expectedMappings) { + EXPECT_EQ( + expectedVeloxConfig, sessionProperties->toVeloxConfig(sessionProperty)); + } +} diff --git a/presto-native-execution/presto_cpp/main/tests/SessionPropertiesTest.cpp b/presto-native-execution/presto_cpp/main/properties/session/tests/SessionPropertiesTest.cpp similarity index 99% rename from presto-native-execution/presto_cpp/main/tests/SessionPropertiesTest.cpp rename to presto-native-execution/presto_cpp/main/properties/session/tests/SessionPropertiesTest.cpp index 546c79c866597..3cdf46bbab248 100644 --- a/presto-native-execution/presto_cpp/main/tests/SessionPropertiesTest.cpp +++ b/presto-native-execution/presto_cpp/main/properties/session/tests/SessionPropertiesTest.cpp @@ -13,7 +13,7 @@ */ #include -#include "presto_cpp/main/SessionProperties.h" +#include "presto_cpp/main/properties/session/SessionProperties.h" #include "velox/core/QueryConfig.h" using namespace facebook::presto; diff --git a/presto-native-execution/presto_cpp/main/tests/CMakeLists.txt b/presto-native-execution/presto_cpp/main/tests/CMakeLists.txt index 4deeaeb1f9111..b31ce9cf017be 100644 --- a/presto-native-execution/presto_cpp/main/tests/CMakeLists.txt +++ b/presto-native-execution/presto_cpp/main/tests/CMakeLists.txt @@ -21,7 +21,6 @@ add_executable( PrestoToVeloxQueryConfigTest.cpp QueryContextCacheTest.cpp ServerOperationTest.cpp - SessionPropertiesTest.cpp TaskManagerTest.cpp QueryContextManagerTest.cpp TaskInfoTest.cpp diff --git a/presto-native-execution/presto_cpp/main/tests/PrestoToVeloxQueryConfigTest.cpp b/presto-native-execution/presto_cpp/main/tests/PrestoToVeloxQueryConfigTest.cpp index 1d7b770558918..b049ac97744b7 100644 --- a/presto-native-execution/presto_cpp/main/tests/PrestoToVeloxQueryConfigTest.cpp +++ b/presto-native-execution/presto_cpp/main/tests/PrestoToVeloxQueryConfigTest.cpp @@ -16,8 +16,8 @@ #include #include "presto_cpp/main/PrestoToVeloxQueryConfig.h" -#include "presto_cpp/main/SessionProperties.h" #include "presto_cpp/main/common/Configs.h" +#include "presto_cpp/main/properties/session/SessionProperties.h" #include "presto_cpp/presto_protocol/core/presto_protocol_core.h" #include "velox/core/QueryConfig.h" diff --git a/presto-native-execution/presto_cpp/main/types/PrestoToVeloxQueryPlan.cpp b/presto-native-execution/presto_cpp/main/types/PrestoToVeloxQueryPlan.cpp index a869d0e3a26dd..ddb990fcb39ca 100644 --- a/presto-native-execution/presto_cpp/main/types/PrestoToVeloxQueryPlan.cpp +++ b/presto-native-execution/presto_cpp/main/types/PrestoToVeloxQueryPlan.cpp @@ -28,13 +28,13 @@ #include "velox/core/Expressions.h" // clang-format on -#include "presto_cpp/main/SessionProperties.h" #include "presto_cpp/main/common/Utils.h" #include "presto_cpp/main/connectors/PrestoToVeloxConnectorUtils.h" #include "presto_cpp/main/operators/BroadcastWrite.h" #include "presto_cpp/main/operators/PartitionAndSerialize.h" #include "presto_cpp/main/operators/ShuffleRead.h" #include "presto_cpp/main/operators/ShuffleWrite.h" +#include "presto_cpp/main/properties/session/SessionProperties.h" #include "presto_cpp/main/types/TypeParser.h" #include "velox/exec/TraceUtil.h" diff --git a/presto-native-execution/src/test/java/com/facebook/presto/nativeworker/PrestoNativeQueryRunnerUtils.java b/presto-native-execution/src/test/java/com/facebook/presto/nativeworker/PrestoNativeQueryRunnerUtils.java index f4281157b9123..c73efb2129dfd 100644 --- a/presto-native-execution/src/test/java/com/facebook/presto/nativeworker/PrestoNativeQueryRunnerUtils.java +++ b/presto-native-execution/src/test/java/com/facebook/presto/nativeworker/PrestoNativeQueryRunnerUtils.java @@ -276,6 +276,12 @@ public HiveQueryRunnerBuilder setExtraProperties(Map extraProper return this; } + public HiveQueryRunnerBuilder addExtraProperties(Map extraProperties) + { + this.extraProperties.putAll(extraProperties); + return this; + } + public HiveQueryRunnerBuilder setExtraCoordinatorProperties(Map extraCoordinatorProperties) { this.extraCoordinatorProperties.putAll(extraCoordinatorProperties); @@ -294,7 +300,7 @@ public QueryRunner build() Optional> externalWorkerLauncher = Optional.empty(); if (this.useExternalWorkerLauncher) { externalWorkerLauncher = getExternalWorkerLauncher("hive", "hive", serverBinary, cacheMaxSize, remoteFunctionServerUds, - pluginDirectory, failOnNestedLoopJoin, coordinatorSidecarEnabled, builtInWorkerFunctionsEnabled, enableRuntimeMetricsCollection, enableSsdCache, implicitCastCharNToVarchar); + pluginDirectory, failOnNestedLoopJoin, coordinatorSidecarEnabled, builtInWorkerFunctionsEnabled, enableRuntimeMetricsCollection, enableSsdCache, implicitCastCharNToVarchar); } return HiveQueryRunner.createQueryRunner( ImmutableList.of(), diff --git a/presto-native-execution/velox b/presto-native-execution/velox index 3b263a79f879f..37d73f68b714b 160000 --- a/presto-native-execution/velox +++ b/presto-native-execution/velox @@ -1 +1 @@ -Subproject commit 3b263a79f879f363fe59187cab286d358b4ec5a6 +Subproject commit 37d73f68b714b97f609bf7e1ee775fed0f2e78df diff --git a/presto-native-tests/src/test/java/com/facebook/presto/nativetests/cudf/TestCudfSidecarPlugin.java b/presto-native-tests/src/test/java/com/facebook/presto/nativetests/cudf/TestCudfSidecarPlugin.java new file mode 100644 index 0000000000000..ea46c096f694e --- /dev/null +++ b/presto-native-tests/src/test/java/com/facebook/presto/nativetests/cudf/TestCudfSidecarPlugin.java @@ -0,0 +1,121 @@ +/* + * 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. + */ +package com.facebook.presto.nativetests.cudf; + +import com.facebook.presto.Session; +import com.facebook.presto.nativetests.NativeTestsUtils; +import com.facebook.presto.sidecar.NativeSidecarPlugin; +import com.facebook.presto.sidecar.sessionpropertyproviders.NativeSystemSessionPropertyProviderFactory; +import com.facebook.presto.spi.session.PropertyMetadata; +import com.facebook.presto.testing.MaterializedResult; +import com.facebook.presto.testing.MaterializedRow; +import com.facebook.presto.testing.QueryRunner; +import com.facebook.presto.tests.AbstractTestQueryFramework; +import com.google.common.collect.ImmutableMap; +import org.testng.annotations.Test; + +import java.util.Optional; + +import static com.facebook.presto.nativeworker.PrestoNativeQueryRunnerUtils.nativeHiveQueryRunnerBuilder; +import static org.testng.Assert.assertEquals; +import static org.testng.Assert.assertFalse; +import static org.testng.Assert.assertTrue; +import static org.testng.Assert.fail; + +public class TestCudfSidecarPlugin + extends AbstractTestQueryFramework +{ + private final String storageFormat = "PARQUET"; + + @Override + protected QueryRunner createQueryRunner() + throws Exception + { + QueryRunner queryRunner = nativeHiveQueryRunnerBuilder() + .setStorageFormat("PARQUET") + .setAddStorageFormatToPath(true) + .setUseThrift(true) + .addExtraProperties(ImmutableMap.of( + "coordinator-sidecar-enabled", "true", + "exclude-invalid-worker-session-properties", "true")) + .setBuiltInWorkerFunctionsEnabled(true) + .build(); + + queryRunner.installCoordinatorPlugin(new NativeSidecarPlugin()); + queryRunner.loadSessionPropertyProvider( + NativeSystemSessionPropertyProviderFactory.NAME, + ImmutableMap.of()); + return queryRunner; + } + + @Override + protected void createTables() + { + NativeTestsUtils.createTables(storageFormat); + } + + @Test + public void testCudfSessionPropertyProvider() + { + // Verify CUDF session property is available + String propertyName = "cudf_allow_cpu_fallback"; + Optional> propertyMetadata = getQueryRunner() + .getMetadata() + .getSessionPropertyManager() + .getSystemSessionPropertyMetadata(propertyName); + assertTrue(propertyMetadata.isPresent(), "Expected cuDF session property metadata to be registered"); + String defaultValue = String.valueOf(propertyMetadata.get().getDefaultValue()); + assertEquals(defaultValue, "true", "Default matches CUDF enablement"); + + assertQuerySucceeds("SET SESSION cudf_allow_cpu_fallback = true"); + MaterializedResult showResult = getQueryRunner() + .execute("SHOW SESSION LIKE 'cudf_allow_cpu_fallback'"); + MaterializedRow row = showResult.getMaterializedRows().get(0); + assertEquals(row.getField(0), propertyName, "Session property name should match"); + assertEquals(row.getField(1), "true", "Session property value should be set to true"); + + // Verify that non-CUDF session properties are not available with native sidecar + String nonCudfProperty = "legacy_array_agg"; + Optional> nonCudfPropertyMetadata = getQueryRunner() + .getMetadata() + .getSessionPropertyManager() + .getSystemSessionPropertyMetadata(nonCudfProperty); + assertFalse(nonCudfPropertyMetadata.isPresent(), "Expected non-CUDF session property to NOT be available"); + + // Verify that setting a non-CUDF session property fails + assertQueryFails("SET SESSION legacy_array_agg = true", ".*Session property.*does not exist.*"); + + // Verify that session properties flow through to Velox config with proper validation + // When cudf_memory_percent is set to 0, it should trigger a validation error + Session failingSession = Session.builder(getSession()) + .setSystemProperty("cudf_memory_percent", "0") + .setSystemProperty("cudf_allow_cpu_fallback", "false") + .build(); + + try { + computeActual( + failingSession, + "select min(nationkey) from nation"); + fail("Expected query to fail when cudf_memory_percent is zero"); + } + catch (RuntimeException e) { + assertTrue( + e.getMessage().matches("(?s).*cudf\\.memory_percent.*greater than 0.*"), + "Expected cudf.memory_percent validation to propagate to cuDF config"); + } + + // Verify that queries succeed with valid default session properties + assertQuerySucceeds("select min(nationkey) from nation"); + } +}