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

Support NullsFirst for all databases. #42245

Merged
merged 1 commit into from May 19, 2021
Merged

Conversation

kbrock
Copy link
Contributor

@kbrock kbrock commented May 18, 2021

Summary

Most databases order tables with the NULL value first, having it before
all other data values. Postgres has NULLS last.

Fortunately, ANSI SQL has an option to allow the database to specify where NULLS
come out in this sort order

    ORDER BY column ASC NULLS FIRST

MS SQL, SQLite, Oracle, and PostgreSQL all follow this syntax. Unfortunately, MySQL does not.

Before:

PostgreSQL: both .nulls_first() and .nulls_last() work as designed.
Others: both raise a runtime error.

After:

PostgreSQL: both work as designed.
MySQL: .nulls_first() works as designed.
MySQL: .nulls_last() raises a runtime error
Others: both work as designed

#38131 introduced this to Postgres, this PR adds the great feature to the rest of the databases.

@pixeltrix
Copy link
Contributor

@kbrock thanks for your PR - looks good 👍🏻

Can you please squash your commits down into a single commit and I'll merge it.

@kbrock
Copy link
Contributor Author

kbrock commented May 18, 2021

thanks @pixeltrix all set

Before, it didn't allow me to fixup my spelling commits - something about privileges with the build. All set now though.

@pixeltrix
Copy link
Contributor

Sorry, I missed this - shouldn't these tests in visitors/postgres_test.rb be moved to visitors/to_sql_test.rb since they apply to other databases now?

Most databases order tables with the `NULL` value first, having it before
all other data values. Postgres has `NULLS` last.

Fortunately, ANSI SQL has an option to allow the database to specify where NULLS
come out in this sort order

    ORDER BY column ASC NULLS FIRST

MS SQL, SQLite, Oracle, and Postgres all follow this syntax. Unfortunately, MySql
does not.

Before:

PostgreSQL: both `.nulls_first()` and `.nulls_last()` work as designed.
Others: both raise a runtime error.

After:

MySQL: `.nulls_first()` works as designed.
MySQL: `.nulls_last()` raises a runtime error
Others: both work as designed
@kbrock
Copy link
Contributor Author

kbrock commented May 19, 2021

thank you @pixeltrix I was concerned running it on the main branch with an override in mysql would complain. looks like it is working, though all those time and date failures distract me from finding anything that I caused

@pixeltrix pixeltrix merged commit 9614d0b into rails:main May 19, 2021
@pixeltrix
Copy link
Contributor

@kbrock thanks for the prompt update! 👍🏻

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.

None yet

2 participants