kysely-org / kysely

A type-safe typescript SQL query builder
https://kysely.dev
MIT License
9.81k stars 249 forks source link

Add `within group` clause support for aggregate function builder #1024

Open ivashog opened 1 month ago

ivashog commented 1 month ago

closes #781

vercel[bot] commented 1 month ago

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
kysely ✅ Ready (Inspect) Visit Preview 💬 Add feedback Jun 18, 2024 9:45am
ivashog commented 1 month ago

@igalklebanov, I want to consult about the API of the future within group () functionality. I see the following possible varaints:

  1. Extended - fn.agg('mode').withinGroup(wg => wg.orderBy('field')): + : looks like SQL - : too verbose; need to create separate WithingGroupBuilder
  2. Simplified - fn.agg('mode').withinGroup('field'), because within group () use only order by expression
  3. Also I can try to implement both variants in one method
igalklebanov commented 3 weeks ago

@igalklebanov, I want to consult about the API of the future within group () functionality. I see the following possible varaints:

  1. Extended - fn.agg('mode').withinGroup(wg => wg.orderBy('field')): + : looks like SQL - : too verbose; need to create separate WithingGroupBuilder
  2. Simplified - fn.agg('mode').withinGroup('field'), because within group () use only order by expression
  3. Also I can try to implement both variants in one method

Hey 👋

We had a similar situation with filter (where ...). All current SQL specs only support where clause there, so we decided, for simplicity, to just go with filterWhere(...).

So I'd go with withinGroupOrderBy(...) for now. WDYT?

ivashog commented 3 weeks ago

Hey 👋

We had a similar situation with filter (where ...). All current SQL specs only support where clause there, so we decided, for simplicity, to just go with filterWhere(...).

So I'd go with withinGroupOrderBy(...) for now. WDYT?

OK! We think in the same direction)

https://github.com/kysely-org/kysely/pull/1024/commits/c81d766336bb8bee0d276894621833d3df52b8f6#diff-0e4fd806a08c99c93c0331a3d0e75d4c726e4face483d2e21a9febe03e944593R167