romeerez / orchid-orm

Orchid ORM
https://orchid-orm.netlify.app/
MIT License
495 stars 14 forks source link

Order by grouping field #283

Closed IlyaSemenov closed 4 months ago

IlyaSemenov commented 4 months ago

Ordering by the field which is used for grouping doesn't work if the aggregated result column name field is named the same as the original table field. It works fine with the newly invented name though.

Example:

import process from "node:process"

import { createBaseTable, orchidORM, testTransaction } from "orchid-orm"

const BaseTable = createBaseTable()

class VisitTable extends BaseTable {
  readonly table = "visit"

  columns = this.setColumns(t => ({
    id: t.serial(),
    page: t.string(),
    total: t.integer(),
  }))
}

const db = orchidORM(
  { databaseURL: process.env.DATABASE_URL, log: true },
  {
    visit: VisitTable,
  },
)

await testTransaction.start(db)

await db.$query`
  create table "visit" (
    id serial primary key,
    page text not null,
    total integer
  );
`

await db.visit.select("page", { total: q => q.sum("total") }).group("page").order({ total: "DESC" })
// SELECT "visit"."page", sum("visit"."total") "total" FROM "visit" GROUP BY "visit"."page" ORDER BY "visit"."total" DESC
// Error: column "visit.total" must appear in the GROUP BY clause or be used in an aggregate function

await db.visit.select("page", { total1: q => q.sum("total") }).group("page").order({ total1: "DESC" })
// SELECT "visit"."page", sum("visit"."total") "total1" FROM "visit" GROUP BY "visit"."page" ORDER BY "total1" DESC

await testTransaction.close(db)

I know I can use orderSql`total desc` but shouldn't ORM recognize the grouping field and exclude the table prefix automatically? It's arguably a more frequent case. Even if there is a case when the current behaviour makes sense, it could be always restored with order({ "visit.total": "DESC" }).

romeerez commented 4 months ago

Good point, I published update for this