anshulbaliga7 opened a new pull request, #58999: URL: https://github.com/apache/spark/pull/58999
### What changes were proposed in this pull request? Mark `CallMethodViaReflection` as `stateful` so that `freshCopyIfContainsStatefulExpression()` gives each evaluation its own instance, and add a comment naming the protected state: the reusable `buffer` array that `evalInternal` fills with the argument values before handing it to the reflective call. Two tests are added to `CallMethodViaReflectionSuite`: - the expression reports `stateful` and a fresh copy is a distinct instance that still evaluates; - a two-thread test in which the first evaluation blocks in its second argument, after writing its first argument into the buffer, while a second evaluation runs to completion. On a shared instance the second evaluation overwrites the first one's argument and the first call returns `m20` instead of `m10`; with fresh copies both results are correct. This supersedes #57371 by @huanghanyu-vungle, narrowed per @peter-toth's review there: the `XPathExtract` and `UserDefinedGenerator` overrides were judged inert (`XPathExtract` is `RuntimeReplaceable` and its state lives in a `Literal`-captured evaluator; generators never reach a fresh-copy site), so only the `CallMethodViaReflection` change is kept. The JIRA title still names all three expressions; it can be narrowed to match when this is merged. ### Why are the changes needed? Sub-task of SPARK-58154. `CallMethodViaReflection` keeps a per-instance `buffer` for the reflection arguments but was not marked stateful, so the fresh-copy mechanism skipped it. When one instance is evaluated from two threads at once, for example sibling DataFrames derived from a common parent whose projections over a `LocalRelation` are evaluated on the driver by `ConvertToLocalRelation`, one thread's arguments can be overwritten by the other's between filling the buffer and making the call, producing silently wrong results. ### Does this PR introduce _any_ user-facing change? No. Concurrent evaluation of the same `reflect` / `java_method` call now returns the correct result instead of racing; single-threaded behavior is unchanged. ### How was this patch tested? New tests in `CallMethodViaReflectionSuite`: ``` build/sbt "catalyst/testOnly org.apache.spark.sql.catalyst.expressions.CallMethodViaReflectionSuite" ``` All 13 tests pass (11 existing, 2 new). Removing the `stateful` override makes the two-thread test fail deterministically with `m20 did not equal m10`. ### Was this patch authored or co-authored using generative AI tooling? No -- 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]
