Github user mgaido91 commented on a diff in the pull request: https://github.com/apache/spark/pull/20021#discussion_r157820471 --- Diff: sql/catalyst/src/main/scala/org/apache/spark/sql/catalyst/expressions/codegen/CodeGenerator.scala --- @@ -930,6 +930,18 @@ class CodegenContext { // inline execution if only one block blocks.head } else { + if (Utils.isTesting) { + // Passing global variables to the split method is dangerous, as any mutating to it is + // ignored and may lead to unexpected behavior. + // We don't need to check `arrayCompactedMutableStates` here, as it results to array access --- End diff -- what if we declare a variable with the same name of an `arrayCompactedMutableStates `? Let's say that we have: ``` public class Foo { private Object[] ourArray; // .... private void ourMethod() { Object[] ourArray = new Object[1]; ourSplitFunction(ourArray); } private void ourSplitFunction(Object[] ourArray) { ourArray[0] = null; } ```
--- --------------------------------------------------------------------- To unsubscribe, e-mail: reviews-unsubscr...@spark.apache.org For additional commands, e-mail: reviews-h...@spark.apache.org