[SPARK-56250][SQL] Remove confusing defensive code in SortExec.rowSorter and add warning comment

[viirya]

What changes were proposed in this pull request?

SPARK-52609 added some defensive code to SortExec. The defensive has some issues and was fixed by SPARK-56203.

But actually the defensive code is not for Spark itself but to guard multithreading access to SortExec that isn't an issue to Spark itself. The defensive code could easily confuse others. After rethinking about it, it might be better to revert it and add some warning comment.

Why are the changes needed?

To simplify the code and reduce confusion.

Does this PR introduce any user-facing change?

No

How was this patch tested?

Existing tests

Was this patch authored or co-authored using generative AI tooling?

Generated-by: Claude Sonnet 4.6

[peter-toth]

Let me doublecheck something today and get back to you @viirya .

[peter-toth]

Yeah, using theadlocal there doesn't make much sense. I was actually investigating a very different issue when the theoretical problem in #55006 came up, but it had nothing to do with my root problem. Reverting both commits looks ok to me.

[peter-toth]

Test failures seem real, but unrelated to this change.

Maybe #54854 is causing them? cc @HeartSaVioR

[dongjoon-hyun]

Got it. +1 for reverting.

[dongjoon-hyun]

BTW, @viirya , I believe we should remove some discussion part in the PR description because it never happens in the community.

After some discussion and rethinking about it, it might be better to revert it and add some warning comment.

In addition, the PR title is correct but very misleading because the code itself is Reverting instead of simply adding a comment. This PR includes a code change. It would be great if the PR title clarifies that we are chaining the code itself.

[viirya]

Thank you @dongjoon-hyun. I updated the PR description and title.

[dongjoon-hyun]

Thank you so much. BTW, I saw that this is created Improvement. However, can we have this version of code change in branch-4.1 too?

[viirya]

Thank you so much. BTW, I saw that this is created Improvement. However, can we have this version of code change in branch-4.1 too?

Hmm, yes, maybe bug is more proper as I want to backport this.

[viirya]

I updated the JIRA ticket's title and changed it to bug.

[dongjoon-hyun]

Thank you again~

[peter-toth]

Test failures seem real, but unrelated to this change.

Maybe #54854 is causing them? cc @HeartSaVioR

Same failure again: read stream-stream join v4 state change feed *** FAILED ***. Also happened on my #55036 (comment). This is very likely due to that recently commited PR.

[viirya]

Same failure again: read stream-stream join v4 state change feed *** FAILED ***. Also happened on my #55036 (comment). This is very likely due to that recently commited PR.

If it is too flaky, maybe we should consider revert it first to unblock CI.

[HeartSaVioR]

I'm looking into this now. You can revert but we can probably fix forward if we can wait for a couple hours.

[HeartSaVioR]

#55071 waiting for CI

[viirya]

Thank you @HeartSaVioR