postgrest-js
postgrest-js copied to clipboard
Supabase default of nullsLast can cause performance issues with ascending:false.
Bug report
Describe the bug
I'm entering this as a bug as it is not expected default behavior, but it could be feature request or documentation enhancement.
There can be a performance issue for large tables with an index and using limit when ordered by descending because of default choice enforced by postgrest.js.
In Postgres NULLS FIRST is the default for ORDER BY DESC and NULLS LAST is default for the default ORDER BY ASC.
In PostgREST nullsfirst or nullslast is an optional parameter and would allow Postgres to sort by default order.
Supabase postgrest.js defaults to nullslast and can only be over ridden with a nullsFirst:true option.
In the case of using ascending:false option in Supabase.js (postgrest.js), nullslast is sent to postgREST by default and then Postgres will not use the default NULLS FIRST.
Many users will not even be thinking of nullsfirst or nullslast as default normally works fine.
To Reproduce
The issue is discussed and "resolved" in this thread on discord:
https://discord.com/channels/839993398554656828/928590123011031050
Expected behavior
Either it should be documented that it is recommended to also set nullsFirst:true when using ascending:false and why OR better postgrest.js should take optional nullsFirst AND nullsLast parameter so default is not specifying and letting PostgREST and Postgres make their default choices.
Screenshots

System information
Lastest Supabase.
Additional context
Add any other context about the problem here.
cc @steve-chavez - thoughts? I'm happy to document this one if you prefer
@kiwicopple I think postgrest-js should not default to a NULLS LAST, otherwise the intuitive way of creating an index to speed up order won't work:
CREATE INDEX index_name ON TABLE table_name (date)
-- they'd have to create it as
-- CREATE INDEX index_name ON TABLE table_name (date DESC NULLS LAST)
better postgrest.js should take optional nullsFirst AND nullsLast parameter so default is not specifying and letting PostgREST and Postgres make their default choices.
Agree, this would be a breaking change in postgrest-js though. Something for supabase-js 2.0 https://github.com/supabase/supabase-js/issues/170.
Some references:
- https://stackoverflow.com/questions/45719419/how-to-create-an-index-properly-for-desc-nulls-last-sorting
- https://dba.stackexchange.com/questions/254731/why-does-order-by-nulls-last-affect-the-query-plan-on-a-primary-key
Default NULLS LAST happens here:
https://github.com/supabase/postgrest-js/blob/ff790caf45cf3a3d8fe84e743600393988fc7122/src/lib/PostgrestTransformBuilder.ts#L40-L44
Resolved in https://github.com/supabase/postgrest-js/pull/283