Virtual entity does not work with cursor pagination

[bakura10]

Describe the bug

Cursor pagination currently does not work with virtual entities.

To Reproduce

Considering an entity:

@Entity({
  expression: `SELECT item FROM table_name`
})
export class Activity {
  @Property()
  item!: string;
}

Cursor pagination won't work and will trigger an error:

"select count(*) as count from (SELECT item FROM table_name) as \"c0\" order by \"c0\".\"item\" asc - column \"c0.item\" must appear in the GROUP BY clause or be used in an aggregate function"

I tried to apply my "includeCount" branch and set it to false, which removes the error, but then the after or before cursor has no effect (no where conditions are applied).

I don't have yet sufficient knowledge of the code base to tackle this myself, but in case this would be an easy fix that someone would like to try :)

Expected behavior

Cursor pagination should work the same with virtual entities.

Versions

Dependency	Version
mikro-orm	6.0.0-beta89
driver	postgresql (v15)

[B4nan]

Please don't ignore the issue template. Use discussions if you want to have a chat about something instead of reporting a bug.

[bakura10]

Reformated with the template.

[B4nan]

Thanks! The formatting is not super important, the reproduction was clear enough but the specific version number is often very important, same with the driver (seeing the quoting, it's postgres, right?).

---

Not sure how to approach this, but I'd guess the same problem will be there with regular em.findAndCount() too, it's not just about the cursor pagination, right? Or generally em.count() will break things.

edit: or maybe its actually very simple - we can simply skip the ordering for the count query, IIRC we are already doing that somewhere

[bakura10]

Exactly, the findAndCount/count will fail as of today. The absence of after/before on the cursor pagination is probably a different issue :)

[simonbrunel]

@B4nan For dynamic expressions returning something other than a QueryBuilder (e.g. a string, a knex query, etc.), the virtual expression is not wrapped in a subquery. In this case, count does not work and there is no way to know (from inside the expression function) when it's called for a count or a find. Additionally, filtering, sorting, pagination, etc. have to be handled manually inside the expression, which I think kinda defeats the purpose of a virtual entity (but I may be making wrong assumptions).

I'm wondering what the rational behind wrapping only QueryBuilder expressions?

[B4nan]

Not following, maybe provide a repro for your problem? A string expression is handled a few lines above that:

https://github.com/mikro-orm/mikro-orm/blob/master/packages/knex/src/AbstractSqlDriver.ts#L150

You are not supposed to be putting a knex qb instance in there, that's why it's not explicitly handled anyhow (not saying it's a bad idea, we can support that too). The expression expects either an ORM qb, a string SQL, or an array (for mongodb aggregation).

[simonbrunel]

A string expression is handled a few lines above that

But in this case it must be a hard-coded string (i.e. not a function returning a string). I would like to rely on both the ORM for typing / field mapping benefits and Knex for the abstraction (and generating more complex SQL).

The expression expects either an ORM qb, a string SQL, or an array...

Looks like that returning a Knex qb also works but maybe not on purpose. There is no strong typing about what should return the expression (object), so I wasn't sure what's possible.

Not following, maybe provide a repro for your problem?

Yes, sorry for recycling a closed issue without even providing more context. I can't easily share a repro and I'm not sure it worth the time to build one since I may actually just mis/overuse virtual entities. Please let me know if the following example is enough to draw some conclusions?

@Entity()
class Book {
  @PrimaryKey() id!: number; 
  @Property() name! string;
  // ...
}

@Entity()
class BookDownload {
  @PrimaryKey() id!: number; 
  @ManyToOne() book! Book;
  @Property() downloadedAt!: Date;
  // ...
}

// VIRTUAL ENTITY
@Entity({
   expression: (em: EntityManager) => {
      const knex = em.qb(Book).select(["id", "name"]).getKnexQuery();

      // ... join with BookDownload, COUNT FILTER on downloadAt, etc.
      // ... and some other advanced SQL that can't be done via the ORM qb
      
      return knex;
   }
})
class BookAnalytic {
  // Fields from Book
  @Property() id!: string;
  @Property() name!: string;

  // Generated fields from BookDownload
  @Property() downloads7Days!: number;
  @Property() downloads30Days!: number;
  @Property() lastDownloadedAt!: Date;
}

The reason I would like to rely on a virtual entity is to benefit from the smart filtering automatically converted to SQL but also working on generated fields (such as downloads7Days, etc.).

em.findAndCount(BookAnalytic, {
   downloads7Days: { $gt: 42 },
}, {
   offset: 10,
   limit: 10,
   orderBy: {
      lastDownloadedAt: "DESC"
   }
}

[B4nan]

I think it's just an omission, we can surely support both string and knex qb in the callback, it just wasn't expected/tested in there (but I do see a value for both).

[B4nan]

Actually, the idea behind the loose implementation was that you are free to return anything from the callback, including the results - you can await the knex qb and just do that.

@Entity({
   expression: (em: EntityManager) => {
      const knex = em.qb(Book).select(["id", "name"]).getKnexQuery();

      // ... join with BookDownload, COUNT FILTER on downloadAt, etc.
      // ... and some other advanced SQL that can't be done via the ORM qb
      
      return await knex; // this should be enough
   }
})

But I will add support for the knex qb so it's less confusing.

[simonbrunel]

Thanks @B4nan for your replies!

return await knex; // this should be enough

That doesn't solve the "count" issue, right? It also doesn't automatically apply query options such as filter, limit, offset, etc.

But I will add support for the knex qb so it's less confusing.

Thank you! I would wrap the result of the expression if it's either an ORM qb, a knex qb or a string (to be consistent with the expression: string form).

[B4nan]

#4740 should help

[simonbrunel]

Thank you @B4nan for acting so quickly on this!