Query orderBy property gets overridden by buildPaginator

[DiogoBatista]

Hello Benjamin,

First I want to thank you for this awesome library it really speeds up the process of pagination and I've been using for some time.

I have a weird case where the library is overriding the orderBy defined in the query.

 const query = this.query.createQueryBuilder('table').orderBy('LOWER(name)', 'ASC').where(SOME_CONDITION)

If I log the SQL that this query will create it gives me the intended SQL with the ORDER BY LOWER("name") ASC in the end of the SQL.

But when I pass the query to the paginator It gets overridden with ORDER BY "name" ASC

In our case, our database is sorted with case sensitive so we need to introduce the LOWER on the order to ensure that we have a proper order.

I tested this on the new v0.9.1 thinking that your latest release might have fixed it but I still have it.

Is there anything that I'm missing here? Is there a way to make the paginator not override the orderBy from the query?

[DiogoBatista]

From a brief check on the Paginator.ts It seems that the library is not taking into account the passed query orderBy.

    buildOrder() {
        let { order } = this;
        if (!this.hasAfterCursor() && this.hasBeforeCursor()) {
            order = this.flipOrder(order);
        }
        const orderByCondition = {};
        this.paginationKeys.forEach((key) => {
            orderByCondition[`${this.alias}.${key}`] = order;  <<--- Here is where the override is happening.
        });
        return orderByCondition;
    }

[DiogoBatista]

@benjamin658 do you have any suggestions on how to tackle this one?

[benjamin658]

Hi @DiogoBatista ,
I have tried to add a custom order before, however, the TypeORM seems to have a bug on the addOrderBy nested column with take, you can check this issue: #4

[benjamin658]

This PR tries to fix the issue: #42

I haven't merged it since it doesn't have a test case, but maybe you can try it to see if it works.

PR is welcome.

[DiogoBatista]

Thank you for the response @benjamin658 I created a PR with a test for the @ruifernando7 fix. Do let me know if the test makes sense and hope @ruifernando7 merges it.

PR: ruifernando7#1

[DiogoBatista]

@benjamin658 I just tested @ruifernando7 and I'm not sure if the fix addresses my issue.
with that code, I'm getting the following error.

TypeORMError: "LOWER(table_alias" alias was not found. Maybe you forgot to join it?

@ruifernand7 code is still using the buildOrder method which still builds the orderBy using alias and key.

    this.paginationKeys.forEach((key) => {
      orderByCondition[`${this.alias}.${key}`] = order;
    });

@ruifernando7 code allows for column order bys to be passed but not when you have an order by like LOWER(column_name) because of the Postgres LOWER part.

I will investigate a bit more what we could do to support this use case but do let me know if you have an idea on how to support this.

[benjamin658]

Are you using the name column as the pagination key?

[DiogoBatista]

Yes, I am. Below you can find the query I'm trying to use with the paginator.
I found out today that the error I'm having is thanks to the leftJoinAndSelect that I have in the query

const query = this.customerRepo
      .createQueryBuilder('customer')
      .leftJoinAndSelect(1_relation)
      .leftJoinAndSelect(2_relation)
      .leftJoinAndSelect(3_relation)
      .where(where_clause)
      .orderBy('LOWER(customer.name)', 'ASC');
      
      
   const paginator = buildPaginator({
      entity: Customer,
      alias: 'customer',
      paginationKeys: ['name'],
      query: {
        limit: inputParams.limit ?? 30,
        order: 'ASC',
        afterCursor: inputParams?.cursor?.afterCursor,
        beforeCursor: inputParams?.cursor?.beforeCursor,
      },
    });

If I remove them I don't have the error anymore and I can see a SQL command with the LOWER (ORDER BY LOWER("customer"."name") ASC, "customer"."name" ASC LIMIT 31) which is the intended result.

But this error makes me think that you shouldn't merge ruifernandes code because it seems that is breaking core functionality.

[benjamin658]

It seems to have something to do with the TypeORM bug I mentioned above.

[ruifernando7]

Hi @benjamin658
Sorry for late reply, the PR from @DiogoBatista to add test has been merged to my PR

Thanks

[mtthsnc]

There still seems to be issues related to the ordering being overwritten by buildOrder. A feature in the future could be to allow users to disable the default order function which would allow users to handle the order flow through TypeORM's built in query builder.

I don't have time to reproduce an example unfortunately but for now I have ported most of the library to my own custom solution to handle the ordering of built queries.

On that note, thanks for library. It has served me well for a long time!