Copilot commented on code in PR #12309:
URL: https://github.com/apache/gluten/pull/12309#discussion_r3438846526
##########
cpp-ch/local-engine/Parser/AdvancedParametersParseUtil.cpp:
##########
@@ -138,6 +138,7 @@ JoinOptimizationInfo JoinOptimizationInfo::parse(const
String & advance)
tryAssign(kvs, "isBHJ", info.is_broadcast);
tryAssign(kvs, "isSMJ", info.is_smj);
tryAssign(kvs, "buildHashTableId", info.storage_join_key);
+ tryAssign(kvs, "buildBroadcastTableId", info.storage_join_key);
tryAssign(kvs, "isNullAwareAntiJoin", info.is_null_aware_anti_join);
Review Comment:
`JoinOptimizationInfo::is_broadcast` is still derived only from the `isBHJ`
parameter, but `CHBroadcastNestedLoopJoinExecTransformer` now emits `isBHJ=0`
for BNLJ. That makes `JoinRelParser`/`CrossRelParser` skip
`BroadcastJoinBuilder::getJoin(...)`, so broadcast nested-loop joins will fall
back to non-broadcast join execution (or fail to reuse the cached build side).
Consider treating `buildBroadcastTableId` as an indicator of a broadcast join
when parsing parameters (i.e., set `is_broadcast=true` when
`buildBroadcastTableId` is present), while keeping `isBHJ` to mean "BHJ vs
BNLJ".
##########
backends-velox/src/main/scala/org/apache/gluten/execution/VeloxBroadcastBuildSideCache.scala:
##########
@@ -75,14 +75,14 @@ object VeloxBroadcastBuildSideCache
)
}
- /** This is callback from c++ backend. */
+ /** This is called from c++ side. */
def get(broadcastHashtableId: String): Long = {
- Option(buildSideRelationCache.getIfPresent(broadcastHashtableId))
+ Option(buildSideRelationCache.getIfPresent(broadcastHashtableId.toInt))
.map(_.pointer)
.getOrElse(0)
}
Review Comment:
`VeloxBroadcastBuildSideCache.get` now calls `broadcastHashtableId.toInt`,
which will throw `NumberFormatException` if the C++ side ever passes a
non-numeric ID (e.g., due to mismatched plan versions). Since this method is
invoked from JNI, it’s safer to avoid throwing across the JNI boundary and
instead return 0 when parsing fails.
--
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]
