fix: MapComparator overflow causes SQL stat sort to fail#6628
Open
daguimu wants to merge 1 commit intoalibaba:masterfrom
Open
fix: MapComparator overflow causes SQL stat sort to fail#6628daguimu wants to merge 1 commit intoalibaba:masterfrom
daguimu wants to merge 1 commit intoalibaba:masterfrom
Conversation
The Long/Date/Number comparisons in MapComparator used (int)(a - b) which overflows for large values, violating the Comparator contract. This causes Collections.sort to produce unpredictable results that don't change between ascending and descending, making the SQL monitoring page sort appear broken. Replace with Long.compare/Double.compare which handle all value ranges correctly. Fixes alibaba#6624 Co-Authored-By: Claude Opus 4.6 (1M context) <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Problem
SQL monitoring page sort doesn't work — clicking column headers toggles the ▲▼ icons but the data order doesn't change. Descending sort has no effect.
Root cause:
MapComparatoruses(int)(longA - longB)for Long/Date/Number comparisons. When values are large (which is common for cumulative SQL stats likeExecuteCountandTotalTime), the subtraction overflowsintrange, producing wrong comparison results that violate theComparatorcontract.Example:
(int)(3000000000L - 1L)=-1294967297(negative! should be positive)This causes
Collections.sortto produce unpredictable ordering that doesn't change between ascending and descending mode — exactly matching the reported behavior.Fix
Replace all 3 overflow-prone comparisons with safe alternatives:
(int)(long1 - long2)Long.compare(long1, long2)(int)(double1 - double2)Double.compare(double1, double2)(int)(date1.getTime() - date2.getTime())Long.compare(date1.getTime(), date2.getTime())The
Double.comparefix also resolves a precision bug where small differences (e.g.,100.5 - 100.0 = 0.5) were truncated to0by the(int)cast, treating close values as equal.Tests Added
7 new test cases in
MapComparatorOverflowTest.java:test_long_sort_ascending_large_values— ASC sort with Long.MAX_VALUE rangetest_long_sort_descending_large_values— DESC sort with Long.MAX_VALUE rangetest_long_sort_desc_differs_from_asc— verifies DESC produces reverse of ASCtest_long_overflow_values— specific overflow case: 3B vs 1test_date_sort_large_time_difference— Date comparison with epoch vs nowtest_number_small_difference_precision— 100.5 vs 100.0 precisiontest_random_long_sort_consistency— 100 random Longs, no TimSort contract violationAll 4 existing
MapComparatorTesttests continue to pass.Impact
Fixes SQL statement monitoring page sort for all stat columns (
ExecuteCount,TotalTime,MaxTimespan, etc.). The fix is minimal (3 line changes) with no behavioral change for values withinintrange.Fixes #6624