Copilot commented on code in PR #12309:
URL: https://github.com/apache/gluten/pull/12309#discussion_r3437679771
##########
cpp/velox/jni/VeloxJniWrapper.cc:
##########
@@ -960,7 +960,7 @@ JNIEXPORT jobject JNICALL
Java_org_apache_gluten_execution_IcebergWriteJniWrappe
JNIEXPORT jlong JNICALL
Java_org_apache_gluten_vectorized_HashJoinBuilder_nativeBuild( // NOLINT
JNIEnv* env,
jobject wrapper,
- jstring tableId,
+ jint tableId,
Review Comment:
`tableId` is no longer used in `HashJoinBuilder_nativeBuild`. With `-Wall
-Werror` enabled, this is likely to trigger `-Wunused-parameter` and break the
native build. Either use the value or mark it unused in the signature/body.
##########
backends-clickhouse/src/main/scala/org/apache/spark/sql/execution/joins/ClickHouseBuildSideRelation.scala:
##########
@@ -71,13 +70,13 @@ case class ClickHouseBuildSideRelation(
def reset(): Unit = synchronized {
existHashTableData = 0
- existBroadCastHashJoinContext = null
+ existBroadcastHashJoinContext = null
}
- private def couldReuseHashTableData(broadCastContext:
BroadCastHashJoinContext): Boolean = {
+ private def couldReuseHashTableData(broadcastContext: BroadcastJoinContext):
Boolean = {
if (existHashTableData != 0) {
- existBroadCastHashJoinContext.joinType == broadCastContext.joinType &&
- existBroadCastHashJoinContext.hasMixedFiltCondition ==
broadCastContext.hasMixedFiltCondition
+ existBroadcastHashJoinContext.joinType == broadcastContext.joinType &&
+ existBroadcastHashJoinContext.hasMixedFiltCondition ==
broadcastContext.hasMixedFiltCondition
}
Review Comment:
`couldReuseHashTableData` currently always returns `false` because the
result of the `if` block is discarded and the last expression is `false`. This
prevents reuse of existing hash tables and forces rebuilds even when the
context matches.
##########
backends-clickhouse/src/main/scala/org/apache/gluten/execution/CHHashJoinExecTransformer.scala:
##########
@@ -119,7 +119,7 @@ case class CHShuffledHashJoinExecTransformer(
}
override def genJoinParameters(): Any = {
- val (isBHJ, isNullAwareAntiJoin, buildHashTableId): (Int, Int, String) =
(0, 0, "")
+ val (isBHJ, isNullAwareAntiJoin, buildHashTableId): (Int, Int, Int) = (0,
0, 0)
Review Comment:
`HashJoinLikeExecTransformer` now declares `genJoinParametersInternal` as
abstract (Int, Int, Int). `CHShuffledHashJoinExecTransformer` extends
`ShuffledHashJoinExecTransformerBase` but doesn’t implement
`genJoinParametersInternal`, which will fail compilation even though it
overrides `genJoinParameters()`. Add an override to satisfy the trait (SHJ
should return isBHJ=0).
--
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]
