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 orderBy clause support to aggregate function builder #896

Merged
Merged
Show file tree
Hide file tree
Changes from 13 commits
Commits
Show all changes
20 commits
Select commit Hold shift + click to select a range
2daec93
feat(operation-node): add order by ro aggregate function node
Mar 7, 2024
ab69143
feat(query-compiler): add order by aggregate function query compiler
Mar 7, 2024
f618df9
feat(operation-node): add order by to aggregate function node transfo…
Mar 7, 2024
bf05295
feat(query-builder): add orderBy method to aggregate function builder
Mar 7, 2024
848ff97
test(node): add aggregate function orderBy clause node platform test
Mar 7, 2024
ed959aa
test(typings): add aggregate function orderBy clause typings test
Mar 7, 2024
af31386
refactor: fix esm imports in changed files
Mar 7, 2024
9c83da6
test(node): add separate test for order-sensitive aggregate functions…
Mar 12, 2024
95e3f1f
Merge branch 'master' into 780-add-order-by-support-to-aggregate
ivashog Mar 13, 2024
04520e6
Merge branch 'master' into 780-add-order-by-support-to-aggregate
igalklebanov Mar 15, 2024
5aafcbc
Merge branch 'master' into 780-add-order-by-support-to-aggregate
igalklebanov Mar 24, 2024
7298545
Merge branch 'master' into 780-add-order-by-support-to-aggregate
ivashog Mar 26, 2024
ace9f98
Merge branch 'master' into 780-add-order-by-support-to-aggregate
igalklebanov Apr 21, 2024
fd6ebb1
refactor: apply suggestions from code review
ivashog Apr 25, 2024
4b4c633
Merge branch 'master' into 780-add-order-by-support-to-aggregate
ivashog Apr 25, 2024
5b7927a
Merge branch 'master' into 780-add-order-by-support-to-aggregate
ivashog Apr 30, 2024
2059b4f
Merge branch 'master' into 780-add-order-by-support-to-aggregate
ivashog May 3, 2024
d476c00
test(typings): removed unnecessary typings test
May 3, 2024
b1d987e
Merge branch 'master' into 780-add-order-by-support-to-aggregate
igalklebanov Sep 15, 2024
692b4df
Merge branch 'master' into 780-add-order-by-support-to-aggregate
igalklebanov Sep 25, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 15 additions & 0 deletions src/operation-node/aggregate-function-node.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,12 +2,15 @@ import { freeze } from '../util/object-utils.js'
import { OperationNode } from './operation-node.js'
import { OverNode } from './over-node.js'
import { WhereNode } from './where-node.js'
import { OrderByNode } from './order-by-node.js'
import { OrderByItemNode } from './order-by-item-node.js'

export interface AggregateFunctionNode extends OperationNode {
readonly kind: 'AggregateFunctionNode'
readonly func: string
readonly aggregated: readonly OperationNode[]
readonly distinct?: boolean
readonly orderBy?: OrderByNode
readonly filter?: WhereNode
readonly over?: OverNode
}
Expand Down Expand Up @@ -40,6 +43,18 @@ export const AggregateFunctionNode = freeze({
})
},

cloneWithOrderBy(
aggregateFunctionNode: AggregateFunctionNode,
orderItems: ReadonlyArray<OrderByItemNode>,
): AggregateFunctionNode {
return freeze({
...aggregateFunctionNode,
orderBy: aggregateFunctionNode.orderBy
? OrderByNode.cloneWithItems(aggregateFunctionNode.orderBy, orderItems)
: OrderByNode.create(orderItems),
})
},

cloneWithFilter(
aggregateFunctionNode: AggregateFunctionNode,
filter: OperationNode,
Expand Down
1 change: 1 addition & 0 deletions src/operation-node/operation-node-transformer.ts
Original file line number Diff line number Diff line change
Expand Up @@ -880,6 +880,7 @@ export class OperationNodeTransformer {
kind: 'AggregateFunctionNode',
aggregated: this.transformNodeList(node.aggregated),
distinct: node.distinct,
orderBy: this.transformNode(node.orderBy),
filter: this.transformNode(node.filter),
func: node.func,
over: this.transformNode(node.over),
Expand Down
47 changes: 46 additions & 1 deletion src/query-builder/aggregate-function-builder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,10 @@ import {
AliasedExpression,
Expression,
} from '../expression/expression.js'
import { ReferenceExpression } from '../parser/reference-parser.js'
import {
ReferenceExpression,
StringReference,
} from '../parser/reference-parser.js'
import {
ComparisonOperatorExpression,
OperandValueExpressionOrList,
Expand All @@ -19,6 +22,11 @@ import {
} from '../parser/binary-operation-parser.js'
import { SqlBool } from '../util/type-utils.js'
import { ExpressionOrFactory } from '../parser/expression-parser.js'
import { DynamicReferenceBuilder } from '../dynamic/dynamic-reference-builder.js'
import {
OrderByDirectionExpression,
parseOrderBy,
} from '../parser/order-by-parser.js'

export class AggregateFunctionBuilder<DB, TB extends keyof DB, O = unknown>
implements AliasableExpression<O>
Expand Down Expand Up @@ -95,6 +103,43 @@ export class AggregateFunctionBuilder<DB, TB extends keyof DB, O = unknown>
})
}

/**
* Adds an `order by` clause inside the aggregate function for specify correct result order
* ([see postgres docs](https://www.postgresql.org/docs/9.5/sql-expressions.html#SYNTAX-AGGREGATES))
ivashog marked this conversation as resolved.
Show resolved Hide resolved
*
* ### Examples
*
* ```ts
* const result = await db
* .selectFrom('person')
* .innerJoin('pet', 'pet.owner_id', 'person.id')
* .select((eb) =>
* eb.fn.jsonAgg('pet.name').orderBy('pet.name').as('person_pets')
* )
* .executeTakeFirstOrThrow()
* ```
*
* The generated SQL (PostgreSQL):
*
* ```sql
* select json_agg("pet"."name" order by "pet"."name") as "person_pets"
* from "person"
* inner join "pet" ON "pet"."owner_id" = "person"."id"
* ```
*/
orderBy<OE extends StringReference<DB, TB> | DynamicReferenceBuilder<any>>(
orderBy: OE,
direction?: OrderByDirectionExpression,
): AggregateFunctionBuilder<DB, TB, O> {
return new AggregateFunctionBuilder({
...this.#props,
aggregateFunctionNode: AggregateFunctionNode.cloneWithOrderBy(
this.#props.aggregateFunctionNode,
parseOrderBy([orderBy, direction]),
),
})
}

/**
* Adds a `filter` clause with a nested `where` clause after the function.
*
Expand Down
6 changes: 6 additions & 0 deletions src/query-compiler/default-query-compiler.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1358,6 +1358,12 @@ export class DefaultQueryCompiler
}

this.compileList(node.aggregated)

if (node.orderBy) {
this.append(' ')
this.visitNode(node.orderBy)
}

this.append(')')

if (node.filter) {
Expand Down
51 changes: 51 additions & 0 deletions test/node/src/aggregate-function.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import {
AggregateFunctionBuilder,
ExpressionBuilder,
SimpleReferenceExpression,
ReferenceExpression,
sql,
} from '../../../'
import {
Expand Down Expand Up @@ -1106,6 +1107,56 @@ for (const dialect of DIALECTS) {

await query.execute()
})

describe(`should execute order-sensitive aggregate functions`, () => {
// @todo add test for mssql when implement `WITHIN GROUP ( order_by_clause )` clause
ivashog marked this conversation as resolved.
Show resolved Hide resolved
if (dialect !== 'mssql') {
ivashog marked this conversation as resolved.
Show resolved Hide resolved
const isMySql = dialect === 'mysql'
const funcName = isMySql ? 'group_concat' : 'string_agg'
const funcArgs: Array<ReferenceExpression<Database, 'person'>> = [
'first_name',
]
if (!isMySql) funcArgs.push(sql.lit(','))
ivashog marked this conversation as resolved.
Show resolved Hide resolved

it(`should execute a query with ${funcName}(column order by column) in select clause`, async () => {
const query = ctx.db
.selectFrom('person')
.select((eb) =>
eb.fn
.agg(funcName, funcArgs)
.orderBy('first_name', 'desc')
.as('first_names'),
)

testSql(query, dialect, {
postgres: {
sql: [
`select ${funcName}("first_name", ',' order by "first_name" desc) as "first_names"`,
`from "person"`,
],
parameters: [],
},
mysql: {
sql: [
`select group_concat(\`first_name\` order by \`first_name\` desc) as \`first_names\``,
ivashog marked this conversation as resolved.
Show resolved Hide resolved
`from \`person\``,
],
parameters: [],
},
mssql: NOT_SUPPORTED,
sqlite: {
sql: [
`select string_agg("first_name", ',' order by "first_name" desc) as "first_names"`,
ivashog marked this conversation as resolved.
Show resolved Hide resolved
`from "person"`,
],
parameters: [],
},
})

await query.execute()
})
}
})
})
}

Expand Down
21 changes: 19 additions & 2 deletions test/typings/test-d/aggregate-function.test-d.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,8 +4,8 @@ import {
expectNotAssignable,
expectType,
} from 'tsd'
import { Generated, Kysely, sql } from '..'
import { Database } from '../shared'
import { Generated, Kysely, Selectable, sql } from '..'
import { Database, Pet } from '../shared'

async function testSelectWithoutAs(db: Kysely<Database>) {
const { avg, count, countAll, max, min, sum } = db.fn
Expand Down Expand Up @@ -378,6 +378,23 @@ async function testSelectWithDistinct(db: Kysely<Database>) {
expectAssignable<string | number | bigint>(result.total_age)
}

async function testAggregateWithOrderBy(db: Kysely<Database>) {
const result = await db
.selectFrom('person')
.innerJoin('pet', 'pet.owner_id', 'person.id')
.select((eb) => [
eb.fn
.agg<string[]>('array_agg', ['pet.name'])
.orderBy('pet.name')
.as('person_pet_names'),
eb.fn.jsonAgg('pet').orderBy('pet.id').as('person_pets'),
])
.executeTakeFirstOrThrow()

expectAssignable<string[]>(result.person_pet_names)
expectAssignable<Selectable<Pet>[]>(result.person_pets)
}
ivashog marked this conversation as resolved.
Show resolved Hide resolved

async function testWithFilterWhere(db: Kysely<Database>) {
const { avg, count, max, min, sum } = db.fn

Expand Down