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

@sortBy: Nulls ordering (NULLS FIRST/NULLS LAST) #110

Closed
2 tasks done
LastDragon-ru opened this issue Jan 8, 2024 · 2 comments · Fixed by #111 or #119
Closed
2 tasks done

@sortBy: Nulls ordering (NULLS FIRST/NULLS LAST) #110

LastDragon-ru opened this issue Jan 8, 2024 · 2 comments · Fixed by #111 or #119
Assignees
Labels
! Breaking change pkg: graphql scope: feat New feature or request
Milestone

Comments

@LastDragon-ru
Copy link
Owner

LastDragon-ru commented Jan 8, 2024

NULLs order different in different databases. Sometimes we need to change it. There is no default/built-it support in Laravel nor Lighthouse.

Useful links:

There are two parts:

For the second part, I think we can add nulls extra operator that will change default behaviour. The problem (also related to default config) is: there is no way to know if the column/expression is nullable or not. This is mean that NULLS FIRST/NULLS LAST will be added into all ORDER BY clauses. It may be slow for databases without native NULLS FIRST/NULLS LAST support (MySQL and SQL Server). Unfortunately, I have no idea how to solve it.

The proposed syntax is shown below. Probably both fields should be required (to avoid ambiguity).

query {
    comments(order: [
        {nulls: {asc: First, desc: Last }},
        {user: {name: Asc}}
        {text: Desc}
    ])
}

Or better (it will affect only one clause):

query {
    comments(order: [
        {nullsFirst: {user: {name: Asc}}}
        {nullsLast: {text: Desc}}
    ])
}

Related to nuwave/lighthouse#2486, nuwave/lighthouse#2056

@LastDragon-ru LastDragon-ru added scope: feat New feature or request pkg: graphql ! Breaking change labels Jan 8, 2024
@LastDragon-ru LastDragon-ru added this to the Next milestone Jan 8, 2024
@LastDragon-ru LastDragon-ru self-assigned this Jan 8, 2024
@LastDragon-ru LastDragon-ru linked a pull request Jan 8, 2024 that will close this issue
@LastDragon-ru LastDragon-ru removed a link to a pull request Jan 8, 2024
@LastDragon-ru LastDragon-ru linked a pull request Jan 8, 2024 that will close this issue
@LastDragon-ru
Copy link
Owner Author

LastDragon-ru commented Jan 8, 2024

Default implementation and default (global) settings

Available in the main branch 🎉 Any feedback greatly appreciated.

@LastDragon-ru
Copy link
Owner Author

The following syntax was added and will be available in the next major (v6) 🥳

query {
    comments(order: [
        {nullsFirst: {user: {name: Asc}}}
        {nullsLast: {text: Desc}}
    ])
}

LastDragon-ru added a commit that referenced this issue Feb 3, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
! Breaking change pkg: graphql scope: feat New feature or request
Projects
None yet
1 participant