Copilot commented on code in PR #12487:
URL: https://github.com/apache/gluten/pull/12487#discussion_r3557490072
##########
cpp/velox/substrait/SubstraitToVeloxPlan.cc:
##########
@@ -458,6 +458,34 @@ core::PlanNodePtr
SubstraitToVeloxPlanConverter::toVeloxPlan(const ::substrait::
} else if (
sJoin.has_advanced_extension() &&
SubstraitParser::configSetInOptimization(sJoin.advanced_extension(),
"isBHJ=")) {
+ const std::string hashTableId = sJoin.hashtableid();
+ bool useHashTableCache = false;
+ if (!hashTableId.empty()) {
+ try {
+ const auto handle = getJoin(hashTableId);
+ if (handle != 0) {
+ auto hashTableBuilder =
ObjectStore::retrieve<gluten::HashTableBuilder>(handle);
+ useHashTableCache = (hashTableBuilder != nullptr);
+ }
+ } catch (const std::exception& e) {
+ LOG(WARNING) << "Failed to retrieve pre-built HashTableBuilder for
cache key: " << hashTableId
+ << ", error: " << e.what() << ". Disable hash table cache
and build a new table.";
+ }
+ }
Review Comment:
Deciding `useHashTableCache` by calling `getJoin(hashTableId)` /
`ObjectStore::retrieve(...)` during plan conversion is unsafe for broadcast
hash join: the broadcast hash table is typically built/deserialized later at
runtime (e.g., `VeloxSerializedBroadcastRDD` and `VeloxBroadcastBuildSideRDD`
return `Iterator.empty` after populating the cache). If `useHashTableCache`
becomes false here, the `HashJoinNode` will fall back to building from the
build-side child, which may be empty and can produce incorrect results.
Instead of checking cache presence here, enable hash table cache based on
the presence of `hashTableId` (it should be non-empty for BHJ) and let runtime
populate/lookup the cache by key.
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
