LuciferYang commented on PR #58665: URL: https://github.com/apache/spark/pull/58665#issuecomment-5653859456
Addressed all four, thanks. An unreferenced definition no longer vetoes extraction: argument discovery takes a `With`'s child and follows a definition only where a reference reaches it, which is what `doGenCode` generates. `Between` and `NullIf` are the only builders today and neither drops a reference, so nothing reaches this yet, and the new unit case pins it. Adding that turned up the same ordering being wrong in the other direction. `Expression.genCode` consults `subExprEliminationExprs` before `doGenCode`, and `With` overrides only `doGenCode`, so a `With` that subexpression elimination has computed is read off its state and the definition body assigns from the state's locals, which the method was not taking as parameters. Reaching a node subexpression elimination has computed now keeps the inline body instead. That covers `Alias`, `Collate` and an identity `Cast` regenerating their child as well, and it keeps the parameter list inside what `getLocalInputVariableValues` gives the methods `ExpandExec` and the aggregates move this call into. Reaching it needs the same node to occur twice outside any `With`, since `EquivalentExpressions` does not descend into one, so no query hits it today either. Both untested fallbacks have a case now, in a test for the shapes a method is not possible for. The parameter-length one goes through `spark.sql.CodeGenerator.validParamLength`, the conf the aggregate and subexpression splitting paths already test with. Fixed the same-scope comment: a sibling definition of the same `With` resolves through `currentCommonExprs`, so those references are not only from enclosing scopes. The description now lists the fourth refused shape and says what each test covers. -- 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]
