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

Arel nulls first/last implementation Mysql #50079

Merged
merged 1 commit into from Dec 8, 2023

Conversation

tttffff
Copy link
Contributor

@tttffff tttffff commented Nov 16, 2023

Motivation / Background

Fixes #50078

This Pull Request has been created because I needed to sort a column ASC with nulls last with a MySQL database and wanted to avoid using an SQL fragment to achieve this. I discovered that Arel supports this for all databases apart from MySQL. When investigating the MySQL implementation, I noticed inconsistencies with how it is implemented.

Originally, I was just aiming to make the functionality consistent, but then had a go at implementing the missing functionality.

Detail

This Pull Request changes the implementation of .nulls_first() and .nulls_last() for MySQL databases. These methods now work as designed.

Checklist

Before submitting the PR make sure the following are checked:

  • This Pull Request is related to one change. Changes that are unrelated should be opened in separate PRs.
  • Commit message has a detailed description of what changed and why. If this PR fixes a related issue include it in the commit message. Ex: [Fix #issue-number]
  • Tests are added or updated if you fix a bug or add a feature.
  • CHANGELOG files are updated for the changed libraries if there is a behavior change or additional feature. Minor bug fixes and documentation changes should not be included.

@byroot
Copy link
Member

byroot commented Nov 17, 2023

Please squash your commits.

@tttffff tttffff force-pushed the mysql_null_first_last_consistency branch from 75c5e63 to 98a85ec Compare November 17, 2023 20:57
Fixes rails#50079 where there is unexpected behaviour for MySQL

Previously:
 - Calling nulls_first on asc worked as expected
 - Calling nulls_first on desc ordered desc nulls last
 - Calling nuls_last on asc raised error
 - Calling nulls_last on desc raised error

Now:
 - Calling nulls_first on asc works as expected
 - Calling nulls_first on desc works as expected
 - Calling nulls_last on asc works as expected
 - Calling nulls_last on desc works as expected
@tttffff tttffff force-pushed the mysql_null_first_last_consistency branch from 98a85ec to 059caa2 Compare November 17, 2023 21:01
@tenderlove tenderlove merged commit ab8f5c9 into rails:main Dec 8, 2023
4 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Arel - MySQL has inconsistent behavour when attempting to use ordering with nulls_first/nulls_last
3 participants