Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add support for raw() to orderBy #5110

Closed
bozorgmehr96 opened this issue Jan 10, 2024 · 7 comments
Closed

Add support for raw() to orderBy #5110

bozorgmehr96 opened this issue Jan 10, 2024 · 7 comments
Labels
enhancement New feature or request

Comments

@bozorgmehr96
Copy link

Is your feature request related to a problem? Please describe.
The only reason I'm using query builder now, is because of lack of this feature.

Describe the solution you'd like
Add support to use raw() as orderBy object keys, like what we do now in the where object.

await em.find(
  User,
  { [raw('similarity("name", ?)', [searchTerm])]: { $gte: 0.3 } },
  {
    orderBy: {
      [raw('similarity("name", ?)', [searchTerm])]: QueryOrder.DESC,
    },
  },
);```
@bozorgmehr96 bozorgmehr96 added the enhancement New feature or request label Jan 10, 2024
@B4nan
Copy link
Member

B4nan commented Jan 10, 2024

EM is capable of the same things as QB already, it's actually all handled on the QB level.

Please provide a complete repro for this. I got a similar report on slack, so some issue is surely there.

@B4nan
Copy link
Member

B4nan commented Jan 10, 2024

I couldn't find any existing tests for this, weird, I am sure we have some. But that kinda confirms it can be buggy :]

This is the part that handles it btw:

https://github.com/mikro-orm/mikro-orm/blob/master/packages/knex/src/query/QueryBuilderHelper.ts#L613

(again, em.find is not doing anything special, it uses QB abstraction for all of this - so if you can do something with QB, you can surely do the same with EM)

@bozorgmehr96
Copy link
Author

I think it's buggy when we use the populate option on find methods or QB's joinAndSelect() methods.
I'm testing it right now and will come back with more details.

@bozorgmehr96
Copy link
Author

bozorgmehr96 commented Jan 10, 2024

When using raw() helper as orderBy key this error will throw:
Trying to order by not existing property User.[raw]: similarity([::alias::].name, ?) (#36)

code:

em.findAndCount(
  User,
  {
    // some filters
  },
  {
    orderBy: {
      [raw((alias) => `similarity(${alias}.name, ?)`, [search])]: QueryOrder.DESC_NULLS_LAST,
    },
  },
);

Trace:

"type": "Error",
      "message": "Trying to order by not existing property User.[raw]: similarity([::alias::].name, ?) (#36)",
      "stack":
          Error: Trying to order by not existing property User.[raw]: similarity([::alias::].name, ?) (#36)
              at PostgreSqlDriver.buildPopulateOrderBy (/home/mohammad/workbench/odin/node_modules/@mikro-orm/knex/AbstractSqlDriver.js:997:27)
              at PostgreSqlDriver.buildOrderBy (/home/mohammad/workbench/odin/node_modules/@mikro-orm/knex/AbstractSqlDriver.js:986:38)
              at PostgreSqlDriver.find (/home/mohammad/workbench/odin/node_modules/@mikro-orm/knex/AbstractSqlDriver.js:36:30)
              at SqlEntityManager.find (/home/mohammad/workbench/odin/node_modules/@mikro-orm/core/EntityManager.js:144:41)
              at processTicksAndRejections (node:internal/process/task_queues:95:5)
              at async Promise.all (index 0)
              at SqlEntityManager.findAndCount (/home/mohammad/workbench/odin/node_modules/@mikro-orm/core/EntityManager.js:380:35)
              at UsersService.findAll (/libs/common/src/users/users.service.ts:63:29)
              at UsersController.findAll (/apps/public/src/users/users.controller.ts:41:12)

@bozorgmehr96
Copy link
Author

bozorgmehr96 commented Jan 10, 2024

using QB everything works until we use joinAndSelect() methods:

w/o leftJoinAndSelect()

em
  .qb(User, 'u')
  .select('*')
  .where({
    [raw('similarity("u"."name", ?)', [search])]: { $gte: 0.3 }
  })
  .orderBy({
    [raw('similarity("u"."name", ?)', [search])]: QueryOrder.DESC_NULLS_LAST,
  })
  .limit(100)
  .offset(0);

generated query:

select "u".*
from "users" as "u"
where similarity("u"."name", 'john') >= 0.3
order by similarity("u"."name", 'john') desc nulls last
limit 100

w/ leftJoinAndSelect()

em
  .qb(User, 'u')
  .select('*')
  .leftJoinAndSelect('addresses', 'a')
  .where({
    [raw('similarity("u"."name", ?)', [search])]: { $gte: 0.3 }
  })
  .orderBy({
    [raw('similarity("u"."name", ?)', [search])]: QueryOrder.DESC_NULLS_LAST,
  })
  .limit(100)
  .offset(0);
"type": "Error",
      "message": "Expected 0 bindings, saw 1",
      "stack":
          Error: Expected 0 bindings, saw 1
              at replaceRawArrBindings (/home/mohammad/workbench/odin/node_modules/knex/lib/formatter/rawFormatter.js:27:11)
              at Ref.toSQL (/home/mohammad/workbench/odin/node_modules/knex/lib/raw.js:77:13)
              at Ref.toSQL (/home/mohammad/workbench/odin/node_modules/knex/lib/ref.js:35:18)
              at Ref.Target.toQuery (/home/mohammad/workbench/odin/node_modules/knex/lib/builder-interface-augmenter.js:9:21)
              at Ref.toString (/home/mohammad/workbench/odin/node_modules/knex/lib/raw.js:70:17)
              at QueryBuilder.wrapPaginateSubQuery (/home/mohammad/workbench/odin/node_modules/@mikro-orm/knex/query/QueryBuilder.js:1141:70)
              at QueryBuilder.finalize (/home/mohammad/workbench/odin/node_modules/@mikro-orm/knex/query/QueryBuilder.js:1072:18)
              at QueryBuilder.getKnexQuery (/home/mohammad/workbench/odin/node_modules/@mikro-orm/knex/query/QueryBuilder.js:460:14)
              at QueryBuilder.execute (/home/mohammad/workbench/odin/node_modules/@mikro-orm/knex/query/QueryBuilder.js:584:28)
              at QueryBuilder.getResultList (/home/mohammad/workbench/odin/node_modules/@mikro-orm/knex/query/QueryBuilder.js:629:32)

@B4nan
Copy link
Member

B4nan commented Jan 10, 2024

          at PostgreSqlDriver.buildPopulateOrderBy (/home/mohammad/workbench/odin/node_modules/@mikro-orm/knex/AbstractSqlDriver.js:997:27)

Ok now I get it, it's caused by a rather recent refactor which came much later than the raw queries one, that's what broke it and is missing the support (and its value is inferred from the orderBy option by default).

@B4nan B4nan closed this as completed in 7bf986c Jan 10, 2024
@B4nan
Copy link
Member

B4nan commented Jan 11, 2024

Reopening as the issue with qb.joinAndSelect() is still there, I only fixed the populateOrderBy one.

@B4nan B4nan reopened this Jan 11, 2024
@B4nan B4nan closed this as completed in 67ee6f5 Jan 12, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

2 participants