[BUG] Pagination does not work properly

[Simon128]

Describe the bug or question
Pagination in EndpointCreator will produce inconsistent results, as there is no ordering applied together with the offset and limit.
Postgres explanation on why we need order by when doing pagination: https://www.postgresql.org/docs/current/queries-limit.html

To Reproduce

Description
Expecting the arguments "sort_columns" and "sort_orders" of FastCRUD.get_multi to be set, if applying pagination https://github.com/igorbenav/fastcrud/blob/82b6b17ec6bdae0ef6069f7ad7fdaf7518d6714b/fastcrud/endpoint/endpoint_creator.py#L413 Could for example just use the _get_primary_keys function and "desc" as default sort order to achieve this.

Will have a performance impact, as SQL order by is not free lunch.

[Simon128]

I just stumbled upon the get_multi_by_cursor method of FASTCrud. It might also be an idea to exchange the current get_multi method when paginating the EndpointCreator with the cursor method, as deletion of items while using pagination can lead to inconsistent results as well. Would require a change for the paginated_response function to include the cursor, such that the requester does not have to keep track of it themself. This would be a breaking change though, as all users of FastCRUD utilizing the endpoint would need to adapt their code accordingly.

[igorbenav]

We could add a boolean cursor parameter to make this opt in and add a default order by to the regular offset pagination. I think adding a warning (saying that now it has an order by which leads to a performance impact, and saying that cursor based is better for this) for a few versions would be good

[igorbenav]

Would you like to do this?

[Simon128]

Sure, I can implement this

[Simon128]

One additional thing I encountered, is that _apply_sorting in FastCRUD does only allow to execute an orderby on columns of the specified model. This becomes an issue, if I use get_multi_joined with pagination as I can not order by the columns of the joined entities. This could be fixed by changing the lines https://github.com/igorbenav/fastcrud/blob/82b6b17ec6bdae0ef6069f7ad7fdaf7518d6714b/fastcrud/crud/fast_crud.py#L604-L607 to

            affected_tables = stmt.get_final_froms()
            one_table = len(affected_tables) == 1 and not affected_tables[0]._is_join
            allowed_columns = dict()

            for tbl in stmt.get_final_froms():
                for col in tbl.columns:
                    allowed_columns[col.name if one_table else f"{col.table.name}.{col.name}"] = col

            for idx, column_name in enumerate(sort_columns):
                column = allowed_columns.get(column_name, None)
                if column is None:

I.e. allowing to sort by any column from any of the by the statement affected tables. If only one table is affected by the query (e.g. a simple select on a single table), then it's just the column name that needs to be provided for sorting. If multiple tables are affected (e.g. when joining tables), the sorting column must be provided like "[table].[column]". Shall I include this in the changes?

[igorbenav]

Yeah, it makes sense. If possible, please also add the corresponding tests