Copilot commented on code in PR #12309:
URL: https://github.com/apache/gluten/pull/12309#discussion_r3450239328
##########
backends-clickhouse/src/main/scala/org/apache/gluten/execution/CHBroadcastNestedLoopJoinExecTransformer.scala:
##########
@@ -58,20 +58,21 @@ case class CHBroadcastNestedLoopJoinExecTransformer(
GlutenDriverEndpoint.collectResources(executionId, buildBroadcastTableId)
} else {
logWarning(
- s"Can't not trace broadcast table data $buildBroadcastTableId" +
+ s"Can not trace broadcast table data $buildBroadcastTableId" +
s" because execution id is null." +
Review Comment:
The warning string uses "Can not", which is grammatically awkward in
user-visible logs. Prefer "Cannot" (or "Can't") for readability and consistency
with other messages.
##########
backends-clickhouse/src/main/java/org/apache/gluten/vectorized/StorageJoinBuilder.java:
##########
@@ -36,11 +36,12 @@ public class StorageJoinBuilder {
public static native long nativeCloneBuildHashTable(long hashTableData);
private static native long nativeBuild(
- String buildHashTableId,
+ String buildTableId,
byte[] in,
long rowCount,
String joinKeys,
Review Comment:
The PR description says the build table ID was changed from `String` to
`Int`, but this JNI boundary still uses a `String buildTableId` (and related
Scala context fields are also `String`). Either update the PR description to
match the implementation, or follow through by changing the ID type across
Scala/Java/C++ and the cache key parsing.
--
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]
