LuciferYang commented on PR #58045: URL: https://github.com/apache/spark/pull/58045#issuecomment-5586693462
Thanks, all five are real. Fixed in 1133af6 and 85fd1a4. **P2, fallback initialization.** Right, and it lands on the one thing this PR is the first to depend on: a `With` is not a `CodegenFallback`, so when it falls back it relies on `CodegenFallback.generate` walking the subtree and registering every `Nondeterministic` for partition initialization. Both tests now build `Counter()` directly and let `proj.initialize(0)` reach it. Checked that this is load-bearing: narrowing the registration to direct children fails the nested case with `Nondeterministic expression ... should be initialized before eval`. **P2, migration guide.** Agreed, the entry promised more than `canSubstitute` delivers. It now says what changed -- a nondeterministic common expression read more than once in a branch is evaluated once -- and names the exception: a definition that is cheap and reports itself deterministic is still substituted at every read. The `aes_encrypt` case you name is that exception; `canSubstitute`'s scaladoc records it, along with why the fix belongs on those expressions or in a purity predicate shared with `isSafeToDuplicate` rather than here. The PR description is narrowed the same way. **P3, zero references.** Correct, `!multiplyReferenced.contains(id)` admits a definition with no references at all. Changed to `referenced at most once` in `canSubstitute` and `inlineDefsThatGainNothing`, and in the description. **P3, PythonUDF agreement.** Fixed. **P3, "the failure to want".** That sentence is gone: 1133af6 cut the comments in these files roughly in half (they had grown to about 50% of the added lines across five review rounds), and this paragraph went with it. Nothing to reword. -- 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]
