Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[#2389] fix:(remote merge): Fixed the issue of losing data when calling hasNext multiple times. #2390

Merged
merged 3 commits into from
Mar 12, 2025

Conversation

zhengchenyu
Copy link
Collaborator

What changes were proposed in this pull request?

KeyValueReader::next combines the semantics of hasNext and next, which is unreasonable and should be separated.

Why are the changes needed?

Fix: #2389

Does this PR introduce any user-facing change?

No.

How was this patch tested?

unit test and integration test

Copy link

github-actions bot commented Mar 11, 2025

Test Results

 2 996 files  ±0   2 996 suites  ±0   6h 31m 21s ⏱️ -46s
 1 105 tests ±0   1 103 ✅ ±0   2 💤 ±0  0 ❌ ±0 
13 849 runs  ±0  13 819 ✅ ±0  30 💤 ±0  0 ❌ ±0 

Results for commit 2fff5bf. ± Comparison against base commit 86da646.

♻️ This comment has been updated with latest results.

Copy link
Contributor

@jerqi jerqi left a comment

Choose a reason for hiding this comment

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

LGTM.

@zhengchenyu zhengchenyu merged commit f65d09e into apache:master Mar 12, 2025
41 checks passed
zhengchenyu added a commit to zhengchenyu/uniffle that referenced this pull request Mar 12, 2025
… calling hasNext multiple times. (apache#2390) (#32)

### What changes were proposed in this pull request?

KeyValueReader::next combines the semantics of hasNext and next, which is unreasonable and should be separated.

### Why are the changes needed?

Fix: apache#2389 

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

No.

### How was this patch tested?

unit test and integration test
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Bug] [RemoteMerge] Fixed the issue of losing data when calling hasNext multiple times
2 participants