kysely-org / kysely

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

fn.agg doesn't type-check #1046

Closed mb21 closed 3 months ago

mb21 commented 3 months ago

The following query (playground Link):

const result = await db.selectFrom('person')
  .select(({ fn }) => fn.agg<string[]>('array_agg', 'first_name').as('imageNames'))
  .execute()

executes fine in PostgreSQL, but TypeScript complains on 'first_name' with:

Argument of type 'string' is not assignable to parameter of type 'readonly ReferenceExpression<DB, "person">[]'.

Not exactly sure whether I'm doing something wrong or it's just a bug?

igalklebanov commented 3 months ago

Hey 👋

The 2nd argument is expected to be an array.

const result = await db.selectFrom('person')
  .select(({ fn }) => fn.agg<string[]>('array_agg', ['first_name']).as('imageNames'))
  .execute()

https://old.kyse.link/?p=s&i=WN4Oic19AnoKH8L81Cik

mb21 commented 3 months ago

Thanks for the quick reply! Ah okay, confusing that the generated JavaScript actually still executes correctly.

Now, in my actual code the example is a bit more contrived. Here's a minimal example that still doesn't type-check even with an array. Playground Link

const result = await db
    .selectFrom('person')
    .leftJoin(
      ({ eb, fn }) => eb.selectFrom('pet')
        .select(['owner_id', fn.agg<string[]>('array_agg', ['name']).as('names')])
        .groupBy('owner_id')
        .as('pets'),
      join => join.onRef('id', '=', 'owner_id'),
    )
    .execute()
igalklebanov commented 3 months ago

The fn you're using doesn't have "pet" in context.

const result = await db
  .selectFrom("person")
  .leftJoin(
    (x) =>
      x
        .selectFrom("pet")
        .select([
          "owner_id",
          (x2) => x2.fn.agg<string[]>("array_agg", ["name"]).as("names"),
        ])
        .groupBy("owner_id")
        .as("pets"),
    (join) => join.onRef("id", "=", "owner_id"),
  )
  .execute()

https://old.kyse.link/?p=s&i=2lDvcgcbos21HqwleQwJ

mb21 commented 3 months ago

Thanks a lot, that did the trick!

Not sure whether you've considered this approach already: since the TypeScript errors often get unwieldly, perhaps it would make sense to additionally have hints emitted during runtime? Like here: "Got string, expected array in [stack trace]"

igalklebanov commented 3 months ago

Adding anything excessive at runtime would slow everyone down.

We could try adding KyselyError<'blah blah blah'> in more places to try and make TypeScript errors friendlier.

mb21 commented 3 months ago

KyselyTypeError ? Ah yes, that sounds like a good idea.