Skip to content

Conversation

@viirya
Copy link
Member

@viirya viirya commented Feb 28, 2024

What changes were proposed in this pull request?

This patch removes an out-of-dated code comment that was added by #35975.

Why are the changes needed?

Removed out-of-dated comment to reduce misunderstanding and confusion.

Does this PR introduce any user-facing change?

No

How was this patch tested?

Existing test

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

No

@viirya viirya changed the title minor: Remove out-of-dated comment in CollectLimitExec MINOR: Remove out-of-dated comment in CollectLimitExec Feb 28, 2024
@github-actions github-actions bot added the SQL label Feb 28, 2024
@viirya viirya changed the title MINOR: Remove out-of-dated comment in CollectLimitExec [MINOR][SQL] Remove out-of-dated comment in CollectLimitExec Feb 28, 2024
// collect the first `limit` + `offset` elements and then to drop the first `offset` elements.
// For example: limit is 1 and offset is 2 and the child output two partition.
// The first partition output [1, 2] and the Second partition output [3, 4, 5].
// Then [1, 2, 3] will be taken and output [3].
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

While I reviewed something related to this, the comment read confused to me. I traced it to original PR #35975, but the code was already updated by later PRs. This comment is actually out-of-dated and not matched to the code.

@LuciferYang
Copy link
Contributor

Merged into master for Spark 4.0. Thanks @viirya @HyukjinKwon

Copy link
Member

@dongjoon-hyun dongjoon-hyun left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

+1, LGTM.

@viirya
Copy link
Member Author

viirya commented Feb 28, 2024

Thank you @LuciferYang @HyukjinKwon @dongjoon-hyun

TakawaAkirayo pushed a commit to TakawaAkirayo/spark that referenced this pull request Mar 4, 2024
### What changes were proposed in this pull request?

This patch removes an out-of-dated code comment that was added by apache#35975.

### Why are the changes needed?

Removed out-of-dated comment to reduce misunderstanding and confusion.

### Does this PR introduce _any_ user-facing change?

No

### How was this patch tested?

Existing test

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

No

Closes apache#45311 from viirya/remove_comment.

Authored-by: Liang-Chi Hsieh <[email protected]>
Signed-off-by: yangjie01 <[email protected]>
ericm-db pushed a commit to ericm-db/spark that referenced this pull request Mar 5, 2024
### What changes were proposed in this pull request?

This patch removes an out-of-dated code comment that was added by apache#35975.

### Why are the changes needed?

Removed out-of-dated comment to reduce misunderstanding and confusion.

### Does this PR introduce _any_ user-facing change?

No

### How was this patch tested?

Existing test

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

No

Closes apache#45311 from viirya/remove_comment.

Authored-by: Liang-Chi Hsieh <[email protected]>
Signed-off-by: yangjie01 <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants