kysely icon indicating copy to clipboard operation
kysely copied to clipboard

`jsonArrayFrom` support outside of `select`.

Open weeksie opened this issue 11 months ago • 7 comments

First off, great library. I can't believe how few corner cases I run into. Really just awesome stuff.

Here's the relevant section of a query where I've run into this.

        eb
          .selectFrom("connectedDivisions")
          .leftJoinLateral(
            ({ selectFrom }) =>
              // throwing a bare jsonArrayFrom over this query 
              // doesn't let you add an alias for the column so 
              // you end up with `coalesce` as the column name 
              // b/c under the hood the query is
              //    SELECT coalesce(json_agg(...
              jsonArrayFrom(
                selectFrom("connectionDivisionFulfillmentRules")
                  .innerJoin(
                    "fulfillmentRules",
                    "fulfillmentRules.id",
                    "connectionDivisionFulfillmentRules.fulfillmentRuleId"
                  )
                  .whereRef(
                    "connectionDivisionFulfillmentRules.connectionDivisionId",
                    "=",
                    "connectedDivisions.id"
                  )
                  .selectAll("fulfillmentRules")
                  .select(({ ref }) => [
                    jsonObjectFrom(joinAddress(ref("fulfillmentRules.addressId"))).as("address"),
                    jsonObjectFrom(joinSchedule(ref("fulfillmentRules.scheduleId")))
                      .$notNull()
                      .as("schedule"),
                  ])
              ).as("rules"),
            (join) => join.onTrue()
          )
          .selectAll(["connectedDivisions"])
          // see left lateral join for why `rules.coalesce`
          .select(() => [sql`rules.coalesce`.as("rules")])
          .whereRef("connectedDivisions.connectionId", "=", "filteredConnections.id")

The reason I assume it's bug behavior is because the select clause can't pick up the rules.coalesce column so I have to dip into sql to get to it.

at the mechanical level it's clear that the coalesce function that's created by jsonArrayFrom isn't getting an alias so it's using the function name.

weeksie avatar Dec 21 '24 19:12 weeksie

Hey 👋

Can you provide a kyse.link that reproduces this?

igalklebanov avatar Dec 21 '24 21:12 igalklebanov

Is there a reason you're using lateral join instead of selecting the rules?

koskimas avatar Dec 22 '24 07:12 koskimas

@igalklebanov ah soz, I should have led with that. https://kyse.link/Zrxmu

@koskimas this is part of a larger query and even though you wouldn't necessarily think it from the query plan, the left lateral performs significantly better than a subselect in practice (in this case)

weeksie avatar Dec 23 '24 14:12 weeksie

Agree, this would be an amazing addition if it worked with the correct types. The correlated subquery from the docs performs much worse also for me. IMO LATERAL JOIN should be the default approach for ONE to MANY joins.

FYI LATERAL JOIN and jsonArrayFrom is also how drizzle implements their relations feature. I'm thinking of switching but this would be needed to match performance.

johanneskares avatar Jan 30 '25 11:01 johanneskares

Here's a workaround:

import { Expression, RawBuilder, Simplify, sql } from "kysely";

const rows = await db
  .selectFrom("person")
  .leftJoinLateral(
    (eb) =>
      jsonArrayFromForJoins(
        eb
          .selectFrom("pet")
          .selectAll("pet")
          .whereRef("owner_id", "=", "person.id"),
      ).as("pets"),
    (join) => join.onTrue(),
  )
  .where("first_name", "=", sql.lit("Jennifer"))
  .select(["first_name", "pets.items"])
  .execute();

export function jsonArrayFromForJoins<O>(
  expr: Expression<O>,
): RawBuilder<{ items: Simplify<O>[] }> {
  return sql`(select coalesce(json_agg(agg), '[]') as items from ${expr} as agg)`;
}

https://kyse.link/kbqOR

igalklebanov avatar Feb 01 '25 15:02 igalklebanov

Renamed this.

It's not a bug, you're holding it wrong - jsonArrayFrom was meant to be used in selects only.

There is an enhancement opportunity here:

  1. adding an alias to the coalesce statement in jsonArrayFrom doesn't break it for current select usage.
  2. adding the workaround as another official helper could be it - need to check if other dialects need something too. maybe deprecate the old one and tell everyone to use $asScalar with the new one when in select?
  3. adding a special type T<SelectType, FromType> and unwrapping it in select and in xFrom and xJoinx could be it too - how would compilation performance look if we added something like this? we could benchmark and experiment. generally, something that allows us to remove the need for $asScalar in some contexts.

igalklebanov avatar Feb 01 '25 16:02 igalklebanov

Amazing, thanks for the workaround. I would adapt it slightly to this, so if there's multiple lateral joins, I have proper names for items. Use selectAll("pets") to select the pets it and it has the correct name in the output.

import {
  Expression,
  RawBuilder,
  AliasedRawBuilder,
  Simplify,
  sql,
} from "kysely";

const rows = await db
  .selectFrom("person")
  .leftJoinLateral(
    (eb) =>
      jsonArrayFromForJoins(
        eb
          .selectFrom("pet")
          .selectAll("pet")
          .whereRef("owner_id", "=", "person.id"),
        "pets",
      ),
    (join) => join.onTrue(),
  )
  .where("first_name", "=", sql.lit("Jennifer"))
  .selectAll("pets")
  .select(["first_name"])
  .execute();

export function jsonArrayFromForJoins<O, T extends string>(
  expr: Expression<O>,
  as: T,
): AliasedRawBuilder<Simplify<Record<T, O[]>>, T> {
  const sqlExpression: RawBuilder<Simplify<Record<T, O[]>>> =
    sql`(select coalesce(json_agg(agg), '[]') as ${sql.raw(as)} from ${expr} as agg)`;
  return sqlExpression.as(as);
}

https://kyse.link/UIGQ7

johanneskares avatar Feb 04 '25 16:02 johanneskares