LuciferYang commented on code in PR #58881:
URL: https://github.com/apache/spark/pull/58881#discussion_r4454462014


##########
sql/core/src/main/scala/org/apache/spark/sql/execution/aggregate/AggregateCodegenSupport.scala:
##########
@@ -351,6 +351,7 @@ trait AggregateCodegenSupport
       if (inputVars.forall(_.isDefined)) {
         val splitCodes = inputVars.flatten.zipWithIndex.map { case (args, i) =>
           val doAggFunc = ctx.freshName(s"doAggregate_${aggNames(i)}")
+          ctx.assertParametersAreNames(args.map(_.variableName), doAggFunc)

Review Comment:
   Done in d6cf7c05e87: `assertMethodParameters` runs both checks, and the 
three sites call it.
   



##########
sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeGenerator.scala:
##########
@@ -1411,13 +1417,16 @@ class CodegenContext extends Logging {
     }
   }
 
-  /**
-   * Whether `name` is a slot of a compacted mutable state array, such as 
`mutableStateArray_0[3]`,
-   * which `addMutableState` hands out for a state it does not inline: a field 
of the class.
-   */
-  private def isCompactedSlot(name: String): Boolean = name match {
-    case CodeGenerator.CompactedSlot(array) => 
mutableStateNames.contains(array)
-    case _ => false
+  /** Whether `name` can name a parameter: a slot of a compacted array, 
`array[i]`, cannot. */
+  private def isJavaName(name: String): Boolean =
+    name.nonEmpty && Character.isJavaIdentifierStart(name.head) &&
+      name.forall(Character.isJavaIdentifierPart)

Review Comment:
   Applied in d6cf7c05e87.
   



##########
sql/core/src/main/scala/org/apache/spark/sql/execution/datasources/v2/MergeRowsExec.scala:
##########
@@ -156,7 +156,7 @@ case class MergeRowsExec(
             |}
             |$rowIdBitmap.add(${currentRowId.value});
      """.stripMargin
-    ExprCode(code, FalseLiteral, JavaCode.variable(rowIdBitmap, bitmapClass))
+    ExprCode(code, FalseLiteral, JavaCode.global(rowIdBitmap, bitmapClass))

Review Comment:
   Reverted in d6cf7c05e87, and the description no longer mentions it.
   



-- 
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]

Reply via email to