Support json_agg(column_ref)

[SimonSimCity]

Fixes #1280.

Wow, my first PR in this repo 🤩

If there was something wrong about what I wrote - e.g. if the types are too narrow or wide, please let me know.

Here's what I know about this:

• Postgresql returns an array if there are elements in the linked table.
• Postgresql returns null otherwise.

According to the docs, this method does only exist for Postgresql, and I therefore didn't look into any other of the options.

[vercel]

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	Jan 17, 2025 4:25pm

[pkg-pr-new]

Open in Stackblitz • kysely_koa_example

npm i https://pkg.pr.new/kysely-org/kysely@1316

commit: b74fc0b

[SimonSimCity]

Since it's likely that people will use this functionality with coalesce, I thought of adding a helper function, similar to the existing jsonArrayFrom. I don't know how you have it in regards to other dialects, because I would only be able to add it for Postgres. Here's an example of how this could be used: #1280 (comment)

[igalklebanov]

Hey 👋

Thanks! 💪

Since it's likely that people will use this functionality with coalesce, I thought of adding a helper function, similar to the existing jsonArrayFrom. I don't know how you have it in regards to other dialects, because I would only be able to add it for Postgres. Here's an example of how this could be used: #1280 (comment)

I'll need to think about the helper. You're right about the usage pattern due to nullability.

1. How would you name such a helper without making stuff confusing (due to the existence of jsonArrayFrom)?
2. Can it be replicated with other dialects? people might get disappointed with other dialects not having something similar.
3. Not that big of a lift compared to other helpers:

(eb) => eb.coalesce(eb.jsonAgg("col"), sql<string[]>`[]`)

vs.

import { someHelper } from 'kysely/helpers/postgres'

(eb) => someHelper(eb.ref("col"))

The existing helpers were born out of a LOT of signals from the community needing something after they ditched some ORM and did not know how to do nested stuff with SQL.

Regardless, this shouldn't be part of this PR.

[igalklebanov]

This could really use a unit test.

[SimonSimCity]

Could you assist me in writing a unit test? What unit do you think should be tested though not to make it an integration test?

[igalklebanov]

Could you assist me in writing a unit test? What unit do you think should be tested though not to make it an integration test?

Sure. We could also pair up on this if you want (DM on Discord).

We have a suite for aggregate functions, probably there.

[SimonSimCity]

@igalklebanov I've just taken some time to look at writing a unit test - it was not as hard as I assumed. Is this how you envisioned it?

I saw that you rebased my PR on the branch 0.28, which had some tests failing - at least at the stage it was back then. Now, as I've rebased it on master, all tests run through.

[igalklebanov]

@igalklebanov I've just taken some time to look at writing a unit test - it was not as hard as I assumed. Is this how you envisioned it?

Yeah, looks good. Most times there's also a sql output check between execution and query, but I see this suite doesn't do these so it's fine.

I saw that you rebased my PR on the branch 0.28, which had some tests failing - at least at the stage it was back then. Now, as I've rebased it on master, all tests run through.

Yeah since this PR introduces new functionality, we wanna keep it out of any documentation so it's parked in the next major release branch.