LuciferYang opened a new pull request, #58881: URL: https://github.com/apache/spark/pull/58881
### What changes were proposed in this pull request? `ExpandExec.doConsume` allocates a mutable state for each output column that varies across the Expand's branches, and hands it to the consumer as `JavaCode.variable(value, dataType)`. This changes it to `JavaCode.global` / `JavaCode.isNullGlobal`. `CodegenContext.addMutableState` returns a field of the generated class: a name of its own when it can inline the type, otherwise a slot in a compacted array, e.g. `mutableStateArray_0[3]`. Either form is reachable from every method of that class, and neither is a local variable. Whole-stage subexpression elimination already wraps its own `addMutableState` results with `JavaCode.global` / `JavaCode.isNullGlobal`, so this brings `ExpandExec` in line with it. `GlobalValue` and `VariableValue` print the same text, so the generated code is unchanged wherever the classification was not asked about. ### Why are the changes needed? `CodeGenerator.getLocalInputVariableValues` collects the `VariableValue`s among an operator's input so that a caller can pass them into a split function; `AggregateCodegenSupport.splitAggregateExpressions` and `CommonExprSlots.fill` both do that. Taking a field for a local variable adds a parameter that shadows the field, and for the compacted form the parameter name is a slot expression, which does not compile. SPARK-59295 added an "is this a Java identifier" check over the candidate parameters partly for this source; that check stays, since it also covers whatever else a consumer may hold, but it no longer has to catch this one. No query is known to reach the bad parameter today, so this closes a latent trap rather than a failure anyone can hit. With a consume function per operator -- the default -- the value is passed as an argument to a fresh parameter, and with that turned off, neither a CUBE over string columns nor a regular aggregate over a varying string column under a distinct rewrite puts the slot in a parameter list. If reviewers would rather see a reachable failure before taking this, that is a fair objection to raise. ### Does this PR introduce _any_ user-facing change? No. ### How was this patch tested? A new case in `WholeStageCodegenSuite` asserts the invariant on the shape that comes closest to reaching the trap: the child of a regular aggregate under a distinct rewrite is a varying `string` column, the buffers stay fixed width so the plan is a `HashAggregateExec`, aggregate function splitting is forced with a threshold of 1, and the consume function per operator is turned off so that the aggregate sees the Expand's own expressions rather than fresh parameters. It checks that the compacted slot is in the generated code and that no `private void ...(...)` parameter list holds one. Being an invariant on a shape that does not currently break, it passes before this change as well. `WholeStageCodegenSuite` (63 passed, 3 ignored) and `DataFrameAggregateSuite` (175 passed); `sql/scalastyle` and `sql/Test/scalastyle` clean. ### Was this patch authored or co-authored using generative AI tooling? Generated-by: Claude Opus 5 -- 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]
