Skip to content

[native] Enable the use of the cuDF parquet reader #25899

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
devavret wants to merge 1 commit into from
Closed

[native] Enable the use of the cuDF parquet reader #25899

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
5 changes: 5 additions & 0 deletions presto-native-execution/presto_cpp/main/connectors/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,5 +19,10 @@ if(PRESTO_ENABLE_ARROW_FLIGHT_CONNECTOR)
target_link_libraries(presto_connectors presto_flight_connector)
endif()

if(PRESTO_ENABLE_CUDF)
target_link_libraries(presto_connectors velox_cudf_hive_connector
cudf::cudf)
Copy link
Copy Markdown
Collaborator

@majetideepak majetideepak Sep 22, 2025

Choose a reason for hiding this comment

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

Is it required for cudf::cudf to be explicit here? Should we be adding it to velox_cudf_hive_connector instead?

endif()

target_link_libraries(presto_connectors presto_velox_expr_conversion
velox_type_fbhive)
16 changes: 16 additions & 0 deletions presto-native-execution/presto_cpp/main/connectors/Registration.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,10 @@
#include "presto_cpp/main/connectors/arrow_flight/ArrowPrestoToVeloxConnector.h"
#endif

#ifdef PRESTO_ENABLE_CUDF
#include "velox/experimental/cudf/connectors/hive/CudfHiveConnector.h"
#endif

#include "velox/connectors/hive/HiveConnector.h"
#include "velox/connectors/tpch/TpchConnector.h"

Expand All @@ -31,6 +35,16 @@ constexpr char const* kIcebergConnectorName = "iceberg";
void registerConnectorFactories() {
// These checks for connector factories can be removed after we remove the
// registrations from the Velox library.
#ifdef PRESTO_ENABLE_CUDF
Copy link
Copy Markdown
Collaborator

@majetideepak majetideepak Sep 22, 2025

Choose a reason for hiding this comment

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

We can likely push this #ifdef inside the if check and reduce some code. Please add a brief comment stating that CudfHiveConnectorFactory has been extended from the HiveConnectorFactory, and any unsupported feature will fall back to it for execution on the CPU.

if (!velox::connector::hasConnectorFactory(
velox::connector::hive::HiveConnectorFactory::kHiveConnectorName)) {
velox::connector::registerConnectorFactory(
std::make_shared<facebook::velox::cudf_velox::connector::hive::CudfHiveConnectorFactory>());
velox::connector::registerConnectorFactory(
std::make_shared<facebook::velox::cudf_velox::connector::hive::CudfHiveConnectorFactory>(
kHiveHadoop2ConnectorName));
}
#else
if (!velox::connector::hasConnectorFactory(
velox::connector::hive::HiveConnectorFactory::kHiveConnectorName)) {
velox::connector::registerConnectorFactory(
Expand All @@ -39,6 +53,8 @@ void registerConnectorFactories() {
std::make_shared<velox::connector::hive::HiveConnectorFactory>(
kHiveHadoop2ConnectorName));
}
#endif

if (!velox::connector::hasConnectorFactory(
velox::connector::tpch::TpchConnectorFactory::kTpchConnectorName)) {
velox::connector::registerConnectorFactory(
Expand Down
1 change: 1 addition & 0 deletions presto-native-execution/presto_cpp/main/types/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ target_link_libraries(presto_velox_expr_conversion velox_presto_types

add_library(presto_types PrestoToVeloxQueryPlan.cpp VeloxPlanValidator.cpp
PrestoToVeloxSplit.cpp)
target_compile_definitions(presto_types PUBLIC VELOX_ENABLE_BACKWARD_COMPATIBILITY)
Copy link
Copy Markdown
Collaborator

@majetideepak majetideepak Sep 22, 2025

Choose a reason for hiding this comment

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

Can this be removed?

target_link_libraries(
presto_types
presto_velox_expr_conversion
Expand Down
Loading