Proposal to stop mixing `limit` and `offset` with the `Range` header

laurenceisla commented 8 months ago

Problem

Mixing limits and offsets with Range goes against the RFC 7233 as it was mentioned in #1089.
Some unexpected error responses occur when mixing the logic of limits and ranges, e.g. this convo: https://github.com/PostgREST/postgrest/issues/2824#issuecomment-1747971985, or this hack to allow LIMIT 0: https://github.com/PostgREST/postgrest/pull/2269

Proposal

Convert limit and offset as-is to SQL, e.g. ?offset=1&limit=3 to OFFSET 1 AND LIMIT 3 without taking into consideration the values in Range. The query builder generates an aggregate query that should apply the value of the Range, and the sub-query that should apply the limit and offset.
Limits and offsets will return the status 200 instead of 206 if it has no Range header, complying with RFC 7233 section-4.1.

Caveats

The selected resource will now include limits and offsets, so the Range values will apply over that selected resource, e.g. this query:

curl "http://localhost:3000/projects?select=id,name&offset=1" -H "Range: 1-"

Will no longer return

[{"id":2,"name":"Windows 10"},
 {"id":3,"name":"IOS"},
 {"id":4,"name":"OSX"},
 {"id":5,"name":"Orphan"}]

Instead, it will show:

[{"id":3,"name":"IOS"},
 {"id":4,"name":"OSX"},
 {"id":5,"name":"Orphan"}]

It does not mix offset=1 with Range: 1-, it applies the offset first and then the first position range value over the already offset query.

wolfgangwalther commented 8 months ago

This sounds like a great idea. It also has very nice semantics: Changing the URL actually changes the requested resource ("a list of 3" is a different thing from "a list of 5"), but using Range headers just "browses through the same resource".

steve-chavez commented 8 months ago

@laurenceisla Great idea! This will also make fixing the Range header https://github.com/PostgREST/postgrest/pull/2204 less breaking.

taimoorzaeem commented 3 weeks ago

Hmm, this would take a lot of change since limit and range is currently too mixed up. For starters, how would you like the following data structures to change:

-- data ApiRequest
...
, iRange               :: HM.HashMap Text NonnegRange    -- ^ Requested range of rows within response
, iTopLevelRange       :: NonnegRange                    -- ^ Requested range of rows from the top level
...

I think we can change them to two ranges, so one would represent the offset and limit range and other would represent the range header. So it may be like:

-- data ApiRequest
...
, iOffsetLimit               :: NonnegRange      -- ^ Requested range of rows within the selected resource
, iHeaderRange               :: NonnegRange      -- ^ Requested range of rows from the selected resource
...

@laurenceisla @steve-chavez @wolfgangwalther WDYT?

taimoorzaeem commented 3 weeks ago

@steve-chavez @laurenceisla

or this hack to allow LIMIT=0: Allow limit=0 in query params to return an empty array #2269

Also, does this change mean we need to remove the limit=0 feature?

laurenceisla commented 3 weeks ago

Hmm, this would take a lot of change since limit and range is currently too mixed up.

Yes, it may need a fair amount of refactoring and/or redesign to be done.

I think we can change them to two ranges

It's a valid approach, although I remembered I wanted to send limit and offset to the DB as-is and let it return the error.

Also, does this change mean we need to remove the limit=0 feature?

According to what I described above then limit=0 is LIMIT 0 in PostgreSQL, so that still should be valid. Edit: Ah, I see the confusion, I mistakenly put LIMIT=0 when it should be LIMIT 0 in the original post. It's fixed now.

PostgREST / postgrest