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

Bug fix: update range end when merging overlapping subarray ranges. #5268

Merged
merged 6 commits into from
Sep 3, 2024

Conversation

bekadavis9
Copy link
Contributor

@bekadavis9 bekadavis9 commented Aug 28, 2024

Bug fix: update range end when merging overlapping subarray ranges.

Previously, when merging overlapping ranges, the new range end would always be updated to that of the merged range. This was a clear issue if one range was fully encompassed within another. For example, if range {3, 4} were merged into {1, 10}, the old behavior would produce a new range of {1, 4}. This PR fixes the bug simply, taking the larger of the two range ends on merge.

[sc-53970]


TYPE: BUG
DESC: Update range end when merging overlapping subarray ranges.

@bekadavis9 bekadavis9 requested review from davisp and KiterLuc August 28, 2024 18:00
@KiterLuc
Copy link
Contributor

I think the change is the right one. Aside from fixing the current tests, we should add tests that mimic the regression test in: "RangeSetAndSuperset::sort_and_merge_ranges - numeric",
"[sort_and_merge_ranges][numeric]",

Copy link
Contributor

@davisp davisp left a comment

Choose a reason for hiding this comment

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

I haven't spent nearly enough time on these parts of core so I'm not going to give a thumps up or down. However, based on this diff and surrounding context it certainly seems like this would have been the cause and that this should fix the issue.

@KiterLuc
Copy link
Contributor

KiterLuc commented Sep 2, 2024

@bekadavis9 let's describe the bug in more details in the PR description.

@KiterLuc KiterLuc merged commit 7fd6328 into dev Sep 3, 2024
63 checks passed
@KiterLuc KiterLuc deleted the rd/bugfix-subarray_overlapping_ranges branch September 3, 2024 16:49
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.

3 participants