From 81716e5dc4e8ec7fdd0822d71ae25e1045e91aff Mon Sep 17 00:00:00 2001 From: Masha Basmanova Date: Thu, 6 Nov 2025 06:07:25 -0800 Subject: [PATCH] refactor(native): Populate HiveColumnHandle::columnHandles property (#26525) Summary: Follow-up to https://github.com/facebookincubator/velox/issues/15348 Reviewed By: xiaoxmeng Differential Revision: D86198314 --- .../connectors/HivePrestoToVeloxConnector.cpp | 28 +++++------ .../connectors/HivePrestoToVeloxConnector.h | 2 +- .../IcebergPrestoToVeloxConnector.cpp | 46 ++++++++----------- .../IcebergPrestoToVeloxConnector.h | 2 +- .../connectors/PrestoToVeloxConnector.cpp | 4 +- .../main/connectors/PrestoToVeloxConnector.h | 6 +-- .../PrestoToVeloxConnectorUtils.cpp | 15 ++---- .../connectors/PrestoToVeloxConnectorUtils.h | 2 + .../main/connectors/SystemConnector.cpp | 2 +- .../main/connectors/SystemConnector.h | 2 +- .../ArrowPrestoToVeloxConnector.cpp | 2 +- .../ArrowPrestoToVeloxConnector.h | 2 +- 12 files changed, 52 insertions(+), 61 deletions(-) diff --git a/presto-native-execution/presto_cpp/main/connectors/HivePrestoToVeloxConnector.cpp b/presto-native-execution/presto_cpp/main/connectors/HivePrestoToVeloxConnector.cpp index ec9136b4beebd..ab865a751b4ea 100644 --- a/presto-native-execution/presto_cpp/main/connectors/HivePrestoToVeloxConnector.cpp +++ b/presto-native-execution/presto_cpp/main/connectors/HivePrestoToVeloxConnector.cpp @@ -336,17 +336,7 @@ HivePrestoToVeloxConnector::toVeloxTableHandle( const protocol::TableHandle& tableHandle, const VeloxExprConverter& exprConverter, const TypeParser& typeParser, - velox::connector::ColumnHandleMap& assignments) const { - auto addSynthesizedColumn = [&](const std::string& name, - protocol::hive::ColumnType columnType, - const protocol::ColumnHandle& column) { - if (toHiveColumnType(columnType) == - velox::connector::hive::HiveColumnHandle::ColumnType::kSynthesized) { - if (assignments.count(name) == 0) { - assignments.emplace(name, toVeloxColumnHandle(&column, typeParser)); - } - } - }; + const velox::connector::ColumnHandleMap& assignments) const { auto hiveLayout = std::dynamic_pointer_cast( tableHandle.connectorTableLayout); @@ -354,13 +344,24 @@ HivePrestoToVeloxConnector::toVeloxTableHandle( hiveLayout, "Unexpected layout type {}", tableHandle.connectorTableLayout->_type); + + std::unordered_set columnNames; + std::vector columnHandles; for (const auto& entry : hiveLayout->partitionColumns) { - assignments.emplace(entry.name, toVeloxColumnHandle(&entry, typeParser)); + if (columnNames.emplace(entry.name).second) { + columnHandles.emplace_back( + std::dynamic_pointer_cast( + std::shared_ptr(toVeloxColumnHandle(&entry, typeParser)))); + } } // Add synthesized columns to the TableScanNode columnHandles as well. for (const auto& entry : hiveLayout->predicateColumns) { - addSynthesizedColumn(entry.first, entry.second.columnType, entry.second); + if (columnNames.emplace(entry.second.name).second) { + columnHandles.emplace_back( + std::dynamic_pointer_cast( + std::shared_ptr(toVeloxColumnHandle(&entry.second, typeParser)))); + } } auto hiveTableHandle = @@ -384,6 +385,7 @@ HivePrestoToVeloxConnector::toVeloxTableHandle( tableName, hiveLayout->dataColumns, tableHandle, + columnHandles, hiveLayout->tableParameters, exprConverter, typeParser); diff --git a/presto-native-execution/presto_cpp/main/connectors/HivePrestoToVeloxConnector.h b/presto-native-execution/presto_cpp/main/connectors/HivePrestoToVeloxConnector.h index 03f78817ebfcb..399fb084ccb22 100644 --- a/presto-native-execution/presto_cpp/main/connectors/HivePrestoToVeloxConnector.h +++ b/presto-native-execution/presto_cpp/main/connectors/HivePrestoToVeloxConnector.h @@ -42,7 +42,7 @@ class HivePrestoToVeloxConnector final : public PrestoToVeloxConnector { const protocol::TableHandle& tableHandle, const VeloxExprConverter& exprConverter, const TypeParser& typeParser, - velox::connector::ColumnHandleMap& assignments) const final; + const velox::connector::ColumnHandleMap& assignments) const final; std::unique_ptr toVeloxInsertTableHandle( diff --git a/presto-native-execution/presto_cpp/main/connectors/IcebergPrestoToVeloxConnector.cpp b/presto-native-execution/presto_cpp/main/connectors/IcebergPrestoToVeloxConnector.cpp index 303ee763d4a60..fc8852ef72a09 100644 --- a/presto-native-execution/presto_cpp/main/connectors/IcebergPrestoToVeloxConnector.cpp +++ b/presto-native-execution/presto_cpp/main/connectors/IcebergPrestoToVeloxConnector.cpp @@ -51,7 +51,8 @@ std::unique_ptr toIcebergTableHandle( const std::string& tableName, const protocol::List& dataColumns, const protocol::TableHandle& tableHandle, - const protocol::Map& tableParameters, + const std::vector& + columnHandles, const VeloxExprConverter& exprConverter, const TypeParser& typeParser) { velox::common::SubfieldFilters subfieldFilters; @@ -96,14 +97,6 @@ std::unique_ptr toIcebergTableHandle( finalDataColumns = ROW(std::move(names), std::move(types)); } - std::unordered_map finalTableParameters = {}; - if (!tableParameters.empty()) { - finalTableParameters.reserve(tableParameters.size()); - for (const auto& [key, value] : tableParameters) { - finalTableParameters[key] = value; - } - } - return std::make_unique( tableHandle.connectorId, tableName, @@ -111,7 +104,8 @@ std::unique_ptr toIcebergTableHandle( std::move(subfieldFilters), remainingFilter, finalDataColumns, - finalTableParameters); + std::unordered_map{}, + columnHandles); } } // namespace @@ -212,18 +206,7 @@ IcebergPrestoToVeloxConnector::toVeloxTableHandle( const protocol::TableHandle& tableHandle, const VeloxExprConverter& exprConverter, const TypeParser& typeParser, - velox::connector::ColumnHandleMap& assignments) const { - auto addSynthesizedColumn = [&](const std::string& name, - protocol::hive::ColumnType columnType, - const protocol::ColumnHandle& column) { - if (toHiveColumnType(columnType) == - velox::connector::hive::HiveColumnHandle::ColumnType::kSynthesized) { - if (assignments.count(name) == 0) { - assignments.emplace(name, toVeloxColumnHandle(&column, typeParser)); - } - } - }; - + const velox::connector::ColumnHandleMap& assignments) const { auto icebergLayout = std::dynamic_pointer_cast< const protocol::iceberg::IcebergTableLayoutHandle>( tableHandle.connectorTableLayout); @@ -232,14 +215,25 @@ IcebergPrestoToVeloxConnector::toVeloxTableHandle( "Unexpected layout type {}", tableHandle.connectorTableLayout->_type); + std::unordered_set columnNames; + std::vector columnHandles; for (const auto& entry : icebergLayout->partitionColumns) { - assignments.emplace( - entry.columnIdentity.name, toVeloxColumnHandle(&entry, typeParser)); + if (columnNames.emplace(entry.columnIdentity.name).second) { + columnHandles.emplace_back( + std::dynamic_pointer_cast< + const velox::connector::hive::HiveColumnHandle>( + std::shared_ptr(toVeloxColumnHandle(&entry, typeParser)))); + } } // Add synthesized columns to the TableScanNode columnHandles as well. for (const auto& entry : icebergLayout->predicateColumns) { - addSynthesizedColumn(entry.first, entry.second.columnType, entry.second); + if (columnNames.emplace(entry.second.columnIdentity.name).second) { + columnHandles.emplace_back( + std::dynamic_pointer_cast< + const velox::connector::hive::HiveColumnHandle>( + std::shared_ptr(toVeloxColumnHandle(&entry.second, typeParser)))); + } } auto icebergTableHandle = @@ -265,7 +259,7 @@ IcebergPrestoToVeloxConnector::toVeloxTableHandle( tableName, icebergLayout->dataColumns, tableHandle, - {}, + columnHandles, exprConverter, typeParser); } diff --git a/presto-native-execution/presto_cpp/main/connectors/IcebergPrestoToVeloxConnector.h b/presto-native-execution/presto_cpp/main/connectors/IcebergPrestoToVeloxConnector.h index c6b6b8850fa53..824af18ecf220 100644 --- a/presto-native-execution/presto_cpp/main/connectors/IcebergPrestoToVeloxConnector.h +++ b/presto-native-execution/presto_cpp/main/connectors/IcebergPrestoToVeloxConnector.h @@ -37,7 +37,7 @@ class IcebergPrestoToVeloxConnector final : public PrestoToVeloxConnector { const protocol::TableHandle& tableHandle, const VeloxExprConverter& exprConverter, const TypeParser& typeParser, - velox::connector::ColumnHandleMap& assignments) const final; + const velox::connector::ColumnHandleMap& assignments) const final; std::unique_ptr createConnectorProtocol() const final; diff --git a/presto-native-execution/presto_cpp/main/connectors/PrestoToVeloxConnector.cpp b/presto-native-execution/presto_cpp/main/connectors/PrestoToVeloxConnector.cpp index a35841f3aa3ba..c5a1e4dc5c8b4 100644 --- a/presto-native-execution/presto_cpp/main/connectors/PrestoToVeloxConnector.cpp +++ b/presto-native-execution/presto_cpp/main/connectors/PrestoToVeloxConnector.cpp @@ -102,7 +102,7 @@ TpchPrestoToVeloxConnector::toVeloxTableHandle( const protocol::TableHandle& tableHandle, const VeloxExprConverter& exprConverter, const TypeParser& typeParser, - velox::connector::ColumnHandleMap& assignments) const { + const velox::connector::ColumnHandleMap& assignments) const { auto tpchLayout = std::dynamic_pointer_cast( tableHandle.connectorTableLayout); @@ -154,7 +154,7 @@ TpcdsPrestoToVeloxConnector::toVeloxTableHandle( const protocol::TableHandle& tableHandle, const VeloxExprConverter& exprConverter, const TypeParser& typeParser, - velox::connector::ColumnHandleMap& assignments) const { + const velox::connector::ColumnHandleMap& assignments) const { auto tpcdsLayout = std::dynamic_pointer_cast( tableHandle.connectorTableLayout); diff --git a/presto-native-execution/presto_cpp/main/connectors/PrestoToVeloxConnector.h b/presto-native-execution/presto_cpp/main/connectors/PrestoToVeloxConnector.h index a3fce5054dfce..c5baff1fbcc87 100644 --- a/presto-native-execution/presto_cpp/main/connectors/PrestoToVeloxConnector.h +++ b/presto-native-execution/presto_cpp/main/connectors/PrestoToVeloxConnector.h @@ -59,7 +59,7 @@ class PrestoToVeloxConnector { const protocol::TableHandle& tableHandle, const VeloxExprConverter& exprConverter, const TypeParser& typeParser, - velox::connector::ColumnHandleMap& assignments) const = 0; + const velox::connector::ColumnHandleMap& assignments) const = 0; [[nodiscard]] virtual std::unique_ptr< velox::connector::ConnectorInsertTableHandle> @@ -133,7 +133,7 @@ class TpchPrestoToVeloxConnector final : public PrestoToVeloxConnector { const protocol::TableHandle& tableHandle, const VeloxExprConverter& exprConverter, const TypeParser& typeParser, - velox::connector::ColumnHandleMap& assignments) const final; + const velox::connector::ColumnHandleMap& assignments) const final; std::unique_ptr createConnectorProtocol() const final; @@ -157,7 +157,7 @@ class TpcdsPrestoToVeloxConnector final : public PrestoToVeloxConnector { const protocol::TableHandle& tableHandle, const VeloxExprConverter& exprConverter, const TypeParser& typeParser, - velox::connector::ColumnHandleMap& assignments) const final; + const velox::connector::ColumnHandleMap& assignments) const final; std::unique_ptr createConnectorProtocol() const final; diff --git a/presto-native-execution/presto_cpp/main/connectors/PrestoToVeloxConnectorUtils.cpp b/presto-native-execution/presto_cpp/main/connectors/PrestoToVeloxConnectorUtils.cpp index c0ace0ae77c08..f95b7c9f27a13 100644 --- a/presto-native-execution/presto_cpp/main/connectors/PrestoToVeloxConnectorUtils.cpp +++ b/presto-native-execution/presto_cpp/main/connectors/PrestoToVeloxConnectorUtils.cpp @@ -750,6 +750,8 @@ std::unique_ptr toHiveTableHandle( const std::string& tableName, const protocol::List& dataColumns, const protocol::TableHandle& tableHandle, + const std::vector& + columnHandles, const protocol::Map& tableParameters, const VeloxExprConverter& exprConverter, const TypeParser& typeParser) { @@ -788,16 +790,6 @@ std::unique_ptr toHiveTableHandle( finalDataColumns = ROW(std::move(names), std::move(types)); } - if (tableParameters.empty()) { - return std::make_unique( - tableHandle.connectorId, - tableName, - isPushdownFilterEnabled, - std::move(subfieldFilters), - remainingFilter, - finalDataColumns); - } - std::unordered_map finalTableParameters = {}; finalTableParameters.reserve(tableParameters.size()); for (const auto& [key, value] : tableParameters) { @@ -811,7 +803,8 @@ std::unique_ptr toHiveTableHandle( std::move(subfieldFilters), remainingFilter, finalDataColumns, - finalTableParameters); + finalTableParameters, + columnHandles); } } // namespace facebook::presto diff --git a/presto-native-execution/presto_cpp/main/connectors/PrestoToVeloxConnectorUtils.h b/presto-native-execution/presto_cpp/main/connectors/PrestoToVeloxConnectorUtils.h index 2045c6c786dc5..aec1f169ffad1 100644 --- a/presto-native-execution/presto_cpp/main/connectors/PrestoToVeloxConnectorUtils.h +++ b/presto-native-execution/presto_cpp/main/connectors/PrestoToVeloxConnectorUtils.h @@ -57,6 +57,8 @@ std::unique_ptr toHiveTableHandle( const std::string& tableName, const protocol::List& dataColumns, const protocol::TableHandle& tableHandle, + const std::vector& + columnHandles, const protocol::Map& tableParameters, const VeloxExprConverter& exprConverter, const TypeParser& typeParser); diff --git a/presto-native-execution/presto_cpp/main/connectors/SystemConnector.cpp b/presto-native-execution/presto_cpp/main/connectors/SystemConnector.cpp index 1d2ecb6ed181e..3e0596922b6e7 100644 --- a/presto-native-execution/presto_cpp/main/connectors/SystemConnector.cpp +++ b/presto-native-execution/presto_cpp/main/connectors/SystemConnector.cpp @@ -377,7 +377,7 @@ SystemPrestoToVeloxConnector::toVeloxTableHandle( const protocol::TableHandle& tableHandle, const VeloxExprConverter& exprConverter, const TypeParser& typeParser, - velox::connector::ColumnHandleMap& assignments) const { + const velox::connector::ColumnHandleMap& assignments) const { auto systemLayout = std::dynamic_pointer_cast( tableHandle.connectorTableLayout); diff --git a/presto-native-execution/presto_cpp/main/connectors/SystemConnector.h b/presto-native-execution/presto_cpp/main/connectors/SystemConnector.h index 536d8d2ab6838..15590cb8c5d7a 100644 --- a/presto-native-execution/presto_cpp/main/connectors/SystemConnector.h +++ b/presto-native-execution/presto_cpp/main/connectors/SystemConnector.h @@ -192,7 +192,7 @@ class SystemPrestoToVeloxConnector final : public PrestoToVeloxConnector { const protocol::TableHandle& tableHandle, const VeloxExprConverter& exprConverter, const TypeParser& typeParser, - velox::connector::ColumnHandleMap& assignments) const final; + const velox::connector::ColumnHandleMap& assignments) const final; std::unique_ptr createConnectorProtocol() const final; diff --git a/presto-native-execution/presto_cpp/main/connectors/arrow_flight/ArrowPrestoToVeloxConnector.cpp b/presto-native-execution/presto_cpp/main/connectors/arrow_flight/ArrowPrestoToVeloxConnector.cpp index e2fde3b20c9ae..8f33fee60e6cb 100644 --- a/presto-native-execution/presto_cpp/main/connectors/arrow_flight/ArrowPrestoToVeloxConnector.cpp +++ b/presto-native-execution/presto_cpp/main/connectors/arrow_flight/ArrowPrestoToVeloxConnector.cpp @@ -48,7 +48,7 @@ ArrowPrestoToVeloxConnector::toVeloxTableHandle( const protocol::TableHandle& tableHandle, const VeloxExprConverter& /*exprConverter*/, const TypeParser& /*typeParser*/, - velox::connector::ColumnHandleMap& assignments) const { + const velox::connector::ColumnHandleMap& /*assignments*/) const { return std::make_unique( tableHandle.connectorId); } diff --git a/presto-native-execution/presto_cpp/main/connectors/arrow_flight/ArrowPrestoToVeloxConnector.h b/presto-native-execution/presto_cpp/main/connectors/arrow_flight/ArrowPrestoToVeloxConnector.h index 5729c18372068..1bbe3ebb097c9 100644 --- a/presto-native-execution/presto_cpp/main/connectors/arrow_flight/ArrowPrestoToVeloxConnector.h +++ b/presto-native-execution/presto_cpp/main/connectors/arrow_flight/ArrowPrestoToVeloxConnector.h @@ -35,7 +35,7 @@ class ArrowPrestoToVeloxConnector final : public PrestoToVeloxConnector { const protocol::TableHandle& tableHandle, const VeloxExprConverter& exprConverter, const TypeParser& typeParser, - velox::connector::ColumnHandleMap& assignments) const final; + const velox::connector::ColumnHandleMap& assignments) const final; std::unique_ptr createConnectorProtocol() const final;