HyukjinKwon commented on PR #59194:
URL: https://github.com/apache/spark/pull/59194#issuecomment-5975983887
Thanks for the ping. The earlier inline point is addressed: narrowing the
comments and the test to the merged `LocalLimit` is the right scoping, and
folding it with `ConstantFolding.constantFolding(Add(le, oe))` is correct since
`le`/`oe` are analysis-guaranteed foldable integers and the sum is
overflow-checked at analysis. One small follow-up on this change: the PR
description still mentions a `1 + 1` test case that was removed and says "Only
queries that failed when ConstantFolding was excluded now succeed"; since a
user-written non-literal `LIMIT`/`OFFSET` still relies on `ConstantFolding`,
consider scoping that wording to the merged limit. See the review comment for
the full assessment.
<!-- SPARK_DEV_REVIEW_REPLY
{"feedback_id":"conversation:5957831501","thread_id":"conversation:5957831501","verdict_sha256":"c76b5d035771d845a6fa2069e28ce94f788910e6872bcccfe7eb345801bf93ce"}
-->
--
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]
