LimitOffset implementation

[miunau]

To address #234

I've added a skeleton for a limitOffset next to limit that would pass in an offset value for pg-typed and mysql-typed.

This probably still needs some work but I just wanted to ask if you're open for such a feature.

I also added some tests that seem to run.

[rollingversions]

There is no change log for this pull request yet.

Create a changelog

[ForbesLindesay]

I'm very open to a feature like this. Offset is often the easiest way to handle pagination. I think rather than a combined limitOffset function that does both the offset and limit, I'd prefer that we add an .offset(count: number) method that you can then chain a .limit(count: number) onto:

export interface OrderedSelectQueryWithOffset<TRecord> extends SelectQuery<TRecord> {
  first(): Promise<TRecord | null>;
  limit(count: number): Promise<TRecord[]>;
}
export interface OrderedSelectQuery<TRecord> extends OrderedSelectQueryWithOffset<TRecord> {
  offset(count: number): Promise<OrderedSelectQueryWithOffset<TRecord>>;
}

Splitting out these two interfaces ensures you cannot call .offset(...) multiple times.

Usage would be:

import db, {users} from './database';

// Example for simple offset-based pagination
export async function offsetPaginatedEmails(offset?: number = 0) {
  const records = await users(db)
    .find()
    .orderByAsc(`email`)
    .offset(offset)
    .limit(10);
  return {
    records: records.map((record) => record.email),
  };
}

[ForbesLindesay]

P.S. it would save me time when merging if you also fill in the change log on Rolling Versions (see automated comment) for pg-typed and mysql-typed.

[miunau]

OK I'm moving house right now but will take a look soon.

[miunau]

@ForbesLindesay I gave it a shot but I'm not sure how the interface composing should turn out based on your example. I've ended up with tests not recognising offset or limit, or orderByAsc/orderByDesc breaking due to mismatched types. For example, returning Promise<OrderedSelectQueryWithOffset<TRecord>> from offset() will block .limit chaining. Could you give more guidance on how you were thinking about the implementation?

[ForbesLindesay]

I can't see any attempt here to make the suggested change. I can offer feedback if you have a go and it doesn't work, but all this code just uses limitOffset as a single method. Please push what you have, even if it's not perfect, so I can see/review it.

[ForbesLindesay]

I don't think this comment makes sense. There's nothing inherently "endless" about this pagination. It's commonly known as "token based pagination"

[ForbesLindesay]

This will need updating for the new .offset(count).limit(count) API.

[ForbesLindesay]

Again, I don't think "endless" adds clarity here. I think this example can be left more or less as is. If you want to give it a name, the two types of pagination are "offset based pagination" and "token based pagination".

[tripflex]

i agree to match chaining it should be .offset honestly i'm surprised this doesn't exist already

[tripflex]

I'm very open to a feature like this. Offset is often the easiest way to handle pagination. I think rather than a combined limitOffset function that does both the offset and limit, I'd prefer that we add an .offset(count: number) method that you can then chain a .limit(count: number) onto:

export interface OrderedSelectQueryWithOffset<TRecord> extends SelectQuery<TRecord> {
  first(): Promise<TRecord | null>;
  limit(count: number): Promise<TRecord[]>;
}
export interface OrderedSelectQuery<TRecord> extends OrderedSelectQueryWithOffset<TRecord> {
  offset(count: number): Promise<OrderedSelectQueryWithOffset<TRecord>>;
}

Splitting out these two interfaces ensures you cannot call .offset(...) multiple times.

Usage would be:

import db, {users} from './database';

// Example for simple offset-based pagination
export async function offsetPaginatedEmails(offset?: number = 0) {
  const records = await users(db)
    .find()
    .orderByAsc(`email`)
    .offset(offset)
    .limit(10);
  return {
    records: records.map((record) => record.email),
  };
}

Seems pretty simple to implement, should I start a new PR or did you want to do it or what would you prefer?

[tripflex]

@ForbesLindesay I took my first stab at this, can you please take a look and let me know of any changes required or any input on handling this?

#300

Thanks!!