Skip to content

fix float NaN sorting order on some platforms#79847

Merged
GrigoryPervakov merged 4 commits intomasterfrom
float-sort
May 19, 2025
Merged

fix float NaN sorting order on some platforms#79847
GrigoryPervakov merged 4 commits intomasterfrom
float-sort

Conversation

@GrigoryPervakov
Copy link
Copy Markdown
Member

@GrigoryPervakov GrigoryPervakov commented May 5, 2025 •
edited
Loading

https://fiddle.clickhouse.com/b65b1fac-e0b3-45bf-94c1-b6b203b1c989
Closes #73653
Some platforms return -nan, but current sorting implementations expect only positive NaN

Changelog category (leave one):

  • Bug Fix (user-visible misbehavior in an official stable release)

Changelog entry (a user-readable short description of the changes that goes to CHANGELOG.md):

Fix the sorting order of the NaNs with a negative sign bit

@GrigoryPervakov GrigoryPervakov added the pr-must-backport Pull request should be backported intentionally. Use this label with great care! label May 5, 2025
@clickhouse-gh
Copy link
Copy Markdown
Contributor

clickhouse-gh bot commented May 5, 2025 •
edited
Loading

Workflow [PR], commit [b3cf1c0]

@clickhouse-gh clickhouse-gh bot added the pr-bugfix Pull request with bugfix, not backported by default label May 5, 2025
@divanik divanik self-assigned this May 6, 2025
divanik
divanik reviewed May 12, 2025
View reviewed changes
Copy link
Copy Markdown
Member

@divanik divanik left a comment

Choose a reason for hiding this comment

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

Minor issues

@GrigoryPervakov GrigoryPervakov enabled auto-merge May 19, 2025 15:04
@GrigoryPervakov GrigoryPervakov added this pull request to the merge queue May 19, 2025
@GrigoryPervakov
Copy link
Copy Markdown
Member Author

GrigoryPervakov commented May 19, 2025

Check failure is not related, details in private sync pr

Merged via the queue into master with commit 6375dfd May 19, 2025
120 checks passed
@GrigoryPervakov GrigoryPervakov deleted the float-sort branch May 19, 2025 17:29
@robot-clickhouse-ci-2 robot-clickhouse-ci-2 added the pr-synced-to-cloud The PR is synced to the cloud repo label May 19, 2025
This was referenced May 19, 2025
Merged
Merged
robot-clickhouse-ci-2 added a commit that referenced this pull request May 19, 2025
@robot-clickhouse-ci-2
Merge pull request #80526 from ClickHouse/cherrypick/25.2/79847
bdc892c 
Cherry pick #79847 to 25.2: fix float NaN sorting order on some platforms
robot-clickhouse added a commit that referenced this pull request May 19, 2025
@robot-clickhouse
Backport #79847 to 25.2: fix float NaN sorting order on some platforms
0947b29
This was referenced May 19, 2025
Closed
Merged
robot-clickhouse-ci-2 added a commit that referenced this pull request May 19, 2025
@robot-clickhouse-ci-2
Merge pull request #80528 from ClickHouse/cherrypick/25.3/79847
ed06df2 
Cherry pick #79847 to 25.3: fix float NaN sorting order on some platforms
robot-clickhouse added a commit that referenced this pull request May 19, 2025
@robot-clickhouse
Backport #79847 to 25.3: fix float NaN sorting order on some platforms
e57c5a1
This was referenced May 19, 2025
Merged
Merged
robot-clickhouse-ci-2 added a commit that referenced this pull request May 19, 2025
@robot-clickhouse-ci-2
Merge pull request #80530 from ClickHouse/cherrypick/25.4/79847
7b0fa27 
Cherry pick #79847 to 25.4: fix float NaN sorting order on some platforms
robot-clickhouse added a commit that referenced this pull request May 19, 2025
@robot-clickhouse
Backport #79847 to 25.4: fix float NaN sorting order on some platforms
86deac0
This was referenced May 19, 2025
Merged
Merged
robot-clickhouse-ci-2 added a commit that referenced this pull request May 19, 2025
@robot-clickhouse-ci-2
Merge pull request #80532 from ClickHouse/cherrypick/25.5/79847
8f483b4 
Cherry pick #79847 to 25.5: fix float NaN sorting order on some platforms
robot-clickhouse added a commit that referenced this pull request May 19, 2025
@robot-clickhouse
Backport #79847 to 25.5: fix float NaN sorting order on some platforms
ff38d0d
@robot-clickhouse-ci-2 robot-clickhouse-ci-2 mentioned this pull request May 19, 2025
Merged
alexey-milovidov added a commit that referenced this pull request May 20, 2025
@alexey-milovidov
Merge pull request #80533 from ClickHouse/backport/25.5/79847
4a28466 
Backport #79847 to 25.5: fix float NaN sorting order on some platforms
GrigoryPervakov added a commit that referenced this pull request May 20, 2025
@GrigoryPervakov
Merge pull request #80531 from ClickHouse/backport/25.4/79847
aabc8d9 
Backport #79847 to 25.4: fix float NaN sorting order on some platforms
GrigoryPervakov added a commit that referenced this pull request May 20, 2025
@GrigoryPervakov
Merge pull request #80529 from ClickHouse/backport/25.3/79847
323a7b4 
Backport #79847 to 25.3: fix float NaN sorting order on some platforms
@robot-clickhouse-ci-2 robot-clickhouse-ci-2 added the pr-backports-created-cloud deprecated label, NOOP label May 20, 2025
robot-clickhouse-ci-1 added a commit that referenced this pull request May 20, 2025
@robot-clickhouse-ci-1
Merge pull request #80525 from ClickHouse/cherrypick/24.8/79847
880300f 
Cherry pick #79847 to 24.8: fix float NaN sorting order on some platforms
robot-clickhouse added a commit that referenced this pull request May 20, 2025
@robot-clickhouse
Backport #79847 to 24.8: fix float NaN sorting order on some platforms
3ab8247
@robot-clickhouse-ci-1 robot-clickhouse-ci-1 mentioned this pull request May 20, 2025
Merged
@robot-ch-test-poll robot-ch-test-poll added the pr-backports-created Backport PRs are successfully created, it won't be processed by CI script anymore label May 20, 2025
GrigoryPervakov added a commit that referenced this pull request May 23, 2025
@GrigoryPervakov
Merge pull request #80582 from ClickHouse/backport/24.8/79847
55330db 
Backport #79847 to 24.8: fix float NaN sorting order on some platforms
@robot-clickhouse robot-clickhouse added the pr-must-backport-synced The `*-must-backport` labels are synced into the cloud Sync PR label Jul 2, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@divanik divanik divanik approved these changes

Assignees

@divanik divanik

Labels

pr-backports-created Backport PRs are successfully created, it won't be processed by CI script anymore pr-backports-created-cloud deprecated label, NOOP pr-bugfix Pull request with bugfix, not backported by default pr-must-backport Pull request should be backported intentionally. Use this label with great care! pr-must-backport-synced The `*-must-backport` labels are synced into the cloud Sync PR pr-synced-to-cloud The PR is synced to the cloud repo

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Logical error: 'Rows are not sorted with permutation, position 1, previous_row index 0, current_row index 1'.

5 participants

@GrigoryPervakov @divanik @robot-clickhouse @robot-ch-test-poll @robot-clickhouse-ci-2