From 778d3142fab9bd290da52bb3e82dde103041b770 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Isma=C3=ABl=20Mej=C3=ADa?= Date: Sun, 28 Jun 2026 16:59:49 +0200 Subject: [PATCH] [GLUTEN][VL] Fix redundant copies in Delta DV plan conversion VeloxPlanConverter.cc (parseDeltaSplitInfo): - Fix double dynamic_pointer_cast: the original code called dynamic_pointer_cast twice (once in the ternary condition, once in the true branch). Use a single cast + null check. - Avoid unnecessary std::string copy from protobuf: the accessor serialized_deletion_vector() returns a const std::string& to the internal protobuf field, but 'auto' (by value) triggered a full string copy into a local variable. Changed to 'const auto&' to bind directly to the protobuf internal field, eliminating the intermediate copy. The shared_ptr construction then copies once from the reference (unavoidable since it needs ownership). Assisted-by: GitHub Copilot:claude-opus-4.6 --- cpp/velox/compute/VeloxPlanConverter.cc | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/cpp/velox/compute/VeloxPlanConverter.cc b/cpp/velox/compute/VeloxPlanConverter.cc index f47ca61050e..22a7c49ac37 100644 --- a/cpp/velox/compute/VeloxPlanConverter.cc +++ b/cpp/velox/compute/VeloxPlanConverter.cc @@ -104,9 +104,10 @@ delta::DeltaRowIndexFilterType parseDeltaRowIndexFilterType(int filterType) { std::shared_ptr parseDeltaSplitInfo( const substrait::ReadRel_LocalFiles_FileOrFiles& file, std::shared_ptr splitInfo) { - auto deltaSplitInfo = std::dynamic_pointer_cast(splitInfo) - ? std::dynamic_pointer_cast(splitInfo) - : std::make_shared(*splitInfo); + auto deltaSplitInfo = std::dynamic_pointer_cast(splitInfo); + if (!deltaSplitInfo) { + deltaSplitInfo = std::make_shared(*splitInfo); + } deltaSplitInfo->format = dwio::common::FileFormat::PARQUET; const auto& deltaReadOptions = file.delta(); @@ -118,14 +119,14 @@ std::shared_ptr parseDeltaSplitInfo( return deltaSplitInfo; } - auto serializedPayload = deltaReadOptions.serialized_deletion_vector(); + const auto& serializedPayload = deltaReadOptions.serialized_deletion_vector(); VELOX_USER_CHECK(!serializedPayload.empty(), "Delta split has a deletion vector without a serialized payload"); VELOX_USER_CHECK_LE( serializedPayload.size(), static_cast(std::numeric_limits::max()), "Delta deletion vector serialized payload is too large"); const auto cardinality = static_cast(deltaReadOptions.deletion_vector_cardinality()); - auto payload = std::make_shared(std::move(serializedPayload)); + auto payload = std::make_shared(serializedPayload); const SplitPayloadBufferView payloadView{ reinterpret_cast(payload->data()), static_cast(payload->size())}; deltaSplitInfo->deletionVectors.emplace_back(