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

CASE...WHEN...THEN...END Support #214

Closed
sampbarrow opened this issue Nov 1, 2022 · 16 comments · Fixed by #404
Closed

CASE...WHEN...THEN...END Support #214

sampbarrow opened this issue Nov 1, 2022 · 16 comments · Fixed by #404
Assignees
Labels
api Related to library's API enhancement New feature or request mysql Related to MySQL postgres Related to PostgreSQL sqlite Related to sqlite

Comments

@sampbarrow
Copy link

sampbarrow commented Nov 1, 2022

This is a bigger one, but, any plans on implementing CASE support?

https://modern-sql.com/feature/case

Looks like it is supported by: BigQuery, Db2, MariaDB, MySQL, Oracle DB, PostgreSQL, SQL Server, SQLite, so basically all major engines.

Syntax:

CASE
    WHEN condition1 THEN result1
    WHEN condition2 THEN result2
    WHEN conditionN THEN resultN
    ELSE result
END;

Probably would fit well as another method on ExpressionBuilder, ie:

db.selectFrom("table")
 .select(eb => {
  return eb.case(cb => {
   return cb.when("expression1", "expression2")
    .when("expression3", "expression4")
    .else("expression5")
  })
 })
@igalklebanov
Copy link
Member

igalklebanov commented Nov 1, 2022

I've been thinking about this recently. The urgency is there for sure.

Its a difficult one to get right, as far as API goes.

CASE <ref> WHEN <value> THEN <another_value> END requires limiting <value> to <ref>'s SelectType. Have a strong feeling this should be .caseRef(ref).whenThen(when, then).else(then).

CASE WHEN <expression> THEN <value> END requires a where-like API, but with a then. More than 3 arguments is a smell.
Maybe something like.. .case().whenThen(wb => wb.when(...).when(...).orWhen(...).then(then)).else(then).

Not sure if it should be bound to ExpressionBuilder. A good example is that it can be wrapped with aggregate functions.

@igalklebanov igalklebanov added enhancement New feature or request api Related to library's API labels Nov 1, 2022
@igalklebanov igalklebanov changed the title SQL CASE Support CASE...WHEN...THEN...END Support Nov 1, 2022
@sampbarrow
Copy link
Author

sampbarrow commented Nov 1, 2022

Yeah, it's a tricky one.

As far as ExpressionBuilder, might make more sense to go in the other direction and look at expanding what fn.count (and other agg funcs) can use, allowing an ExpressionBuilder or some kind of subset of it there, rather than trying to treat CASE as a special case? No pun intended.

Not sure about other engines and I'm not an SQL expert but for sqlite, the expr syntax https://www.sqlite.org/syntax/expr.html is reusable across almost everything (aggregate functions, select columns, order by clauses etc) - I figured this was roughly analogous to ExpressionBuilder. I'm a little confused about subqueries though, since you can use those in SELECT clauses but I don't think you can pass them to COUNT.

.case().whenThen(wb => wb.when(...).when(...).orWhen(...).then(then)).else(then)

This is roughly what I was thinking. "then" could also just be a second argument to whenThen - since you'll never want multiple thens anyway. That would allow wb to be identical to the regular WhereBuilder.

One random thing to keep in mind with types, if you don't specify an else, then the statement might return NULL. So by default, .whenThen would make the result nullable, but then calling .else() might remove that. I'm not sure if that will cause issues with types - what if the then type was already nullable? Then the result is nullable even after an .else() is chained.

@koskimas
Copy link
Member

koskimas commented Nov 2, 2022

If we go down the road of adding builders for all kinds of expressions, we need to really think it through and create a cohesive expression builder instead of adding different things ad-hoc. I'm also not sure we should do it at all. The code quickly becomes unreadable and it's better just to write raw SQL and pay the small type-unsafety penalty.

Let's think about the case builder for example. The syntax could indeed be something like this

eb.case(maybeExpr)
  .when(expr).then(expr)
  .when(expr).then(expr)
  .else(expr)

The expressions can be anything but let's say they are simple comparison operations. We'd have two options:

  1. Use raw SQL for each subexpression
  2. Use the expression builder

Since we are on the road of full type-safety, we'd of course want to use the expression builder. We'd get something like this:

eb.case()
  .when(eb.bin('first_name', '=', 'foo')).then(eb.val(1))
  .when(eb.bin('first_name', 'in', ['bar', 'baz'])).then(eb.val(2))
  .else(eb.val(3))

In this example I used two non-existent expression builder methods bin and val. That's not too bad yet, but compared to the raw SQL it's quite a bit of boilerplate

sql<number>`
  case
    when first_name = 'foo' then 1
    when first_name in ('bar', 'baz') then 2
    else 3
  end
`

@sampbarrow
Copy link
Author

sampbarrow commented Nov 2, 2022

Just my two cents, I like the idea of an ExpressionBuilder that does pretty much everything, with type safety, and can be used anywhere. Could be useful in other places as well (orderBy, etc). But I'm very personally biased towards type safety everywhere, even at the cost of extra verbosity.

But why not both that as a fallback, with an overload or two for the most common use? I figure CASE is similar to WHERE, in which the majority of use will be a simple IF X OP Y, THEN LITERAL. This is basically how orderBy/select already works.

eb.case().when("first_name", "=", "foo").then("x") <-- most uses
eb.case().when(eb.op(eb.fn.aggFunc("x"), ">", eb.randomThing())).then(eb.ref("z")) <-- edge cases

Or just have the overloads be idential to qb.where - but then you lose some flexibility, unless falling back to raw. I feel like this could get a bit messy too. Do you need whenRef too? whenNotExists? Etc etc. Not the biggest fan for those reasons.

eb.case().when("first_name", "=", "foo")
eb.case().when(wb => wb.where("first_name", "=", "foo").orWhere(...))
eb.case().when(sql.literal(true))

Ironically, if you did add to ExpressionBuilder, it that would make CASE no longer necessary for the use case that prompted my initial post on this (sorting a query by putting a set of items at the top based on an equality condition), because I could just use the operator in qb.orderBy using the EB and not have a case statement at all.

I guess the downside is just that ExpressionBuilder may start to become pretty complicated, but a lot of that could go towards covering future use cases that no one is thinking of now, and for simple queries, you don't need to use it anyway.

@igalklebanov
Copy link
Member

igalklebanov commented Nov 3, 2022

@midwesterntechnology eb.case() should return a CaseBuilder, so this only adds 1-2 methods to ExpressionBuilder.

@igalklebanov
Copy link
Member

igalklebanov commented Nov 3, 2022

@koskimas I think I get eb.val (string ref vs. value), but what's behind having eb.bin, instead of a (subset of) WhereInterface-like API (argument-wise)?

@koskimas
Copy link
Member

koskimas commented Nov 3, 2022

So we'd have orWhen, orWhenExists etc? That's an option and it would take care of the case statement but it's not reusable. I'm talking about a builder that could be used to build any expression. For example

select(eb => eb.bin('first_name', '=', 'Sami').as('is_sami'))

@thelinuxlich
Copy link
Contributor

This is ambitious, would help a lot creating db-exclusive helpers

@igalklebanov igalklebanov added postgres Related to PostgreSQL mysql Related to MySQL sqlite Related to sqlite labels Dec 9, 2022
@igalklebanov igalklebanov added blocked Blocked waiting for another feature and removed blocked Blocked waiting for another feature labels Mar 21, 2023
@igalklebanov igalklebanov self-assigned this Mar 31, 2023
@noppa
Copy link

noppa commented Apr 2, 2023

Note that with the then method, CaseThenBuilder instances can't be returned from Promise.then or async functions, as they are "thenables" themselves. I don't know what the actual use case for returning them from async functions would be so maybe it's not an issue in practice, but figured it was worth pointing out. Depending on the implementation, it could be that the user wouldn't get an actual error for it and the program just wouldn't quite work correctly, so it could become a hard to debug edge case footgun. Does TypeScript give an error if you try to do that?

@thelinuxlich
Copy link
Contributor

Well since it will be on Typescript if the return is not a Promise probably it's okay

@twiddler
Copy link
Contributor

Can we use case().when(…).then(…) with reduce? I am trying to find a way of writing things like

const rows = await kysely
  .updateTable("user")
  .set((eb) => ({
    first_name: eb
      .case()
      .when('first_name', '=', 'foo')
      .then("found")
      .when('first_name', '=', 'bar')
      .then("found")
      .else("not found")
      // any maybe more …
      .end()
  })
).execute()

more compact like

const names = ["foo", "bar"]

const rows = await kysely
  .updateTable("user")
  .set((eb) => ({
    first_name: (names.reduce((prev, curr) => prev.when("first_name", "=", curr).then("found"), eb.case())).else("not found").end()
  })
).execute()

but TypeScript sees the isolated eb.case() call as CaseBuilder<DB, "user", unknown, never> and then (rightfully) about the conflict with string:

No overload matches this call.
  Overload 1 of 3, '(callbackfn: (previousValue: string, currentValue: string, currentIndex: number, array: string[]) => string, initialValue: string): string', gave the following error.
    Type 'CaseWhenBuilder<DB, "user", unknown, string>' is not assignable to type 'string'.
  Overload 2 of 3, '(callbackfn: (previousValue: CaseBuilder<DB, "user", unknown, never>, currentValue: string, currentIndex: number, array: string[]) => CaseBuilder<DB, "user", unknown, never>, initialValue: CaseBuilder<...>): CaseBuilder<...>', gave the following error.
    Type 'CaseWhenBuilder<DB, "user", unknown, string>' is not assignable to type 'CaseBuilder<DB, "user", unknown, never>'.
      Property '#private' in type 'CaseWhenBuilder' refers to a different member that cannot be accessed from within type 'CaseBuilder'.(2769)

I can get around it with an explicit cast

import { DB, Member } from './db'
import { CaseWhenBuilder } from 'kysely'

const names = ["foo", "bar"]

const rows = await kysely
  .updateTable("user")
  .set((eb) => ({
    first_name: (names.reduce((prev, curr) => prev.when("first_name", "=", curr).then("found"), eb.case() as unknown as CaseWhenBuilder<DB, 'UserTable', unknown, UserTable['first_name']>)).else("not found").end()
  })
).execute()

but I'd rather not.

Arguably, that reduce call also isn't as readable as I'd like it to be, so I am open to approaching this differently.

Appreciate any help and insight. 🙏

@thelinuxlich
Copy link
Contributor

Doesnt'this solves it?

const names = ["foo", "bar"] as const

@twiddler
Copy link
Contributor

Nope.

@igalklebanov
Copy link
Member

igalklebanov commented Sep 22, 2023

@twiddler native Array.reduce types are not great, even without Kysely.

It won't work with the case chain, because each step in the chain returns a different type, with accumulated generics, and TypeScript sticks with the type of the initial state for all iterations.

I think what @thelinuxlich suggested can work, if you explicitly write the return type.

@twiddler
Copy link
Contributor

twiddler commented Sep 22, 2023

Ye, that's what I thought it probably would boil down to. Thanks.

I do not know what you mean by "explicitely write the return type". I pasted this

const names = ["foo", "bar"] as const

const rows = await kysely
  .updateTable("user")
  .set((eb) => ({
    first_name: (names.reduce((prev, curr) => prev.when("first_name", "=", curr).then("found"), eb.case())).else("not found").end()
  })
).execute()

into https://kyse.link/ and it throws "No overload matches this call.".

@MattyD-Pave
Copy link

MattyD-Pave commented Nov 1, 2024

Would anybody have any tips to here for building a case statement via any other pattern so i can accomplish this without an any? for example...

the update dictionary isn't constant, so i cant use the tip above

    const caseStatement = _.entries(someDataByUserEmail).reduce(
      (
        caseBuilder,
        [userEmail, data],
      ) => {
        return caseBuilder.when('email', '=', ).then(data);
      },
      // I have to cast the acc to any to avoid type errors. 
      kysely.case() as any,
    );

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
api Related to library's API enhancement New feature or request mysql Related to MySQL postgres Related to PostgreSQL sqlite Related to sqlite
Projects
None yet
Development

Successfully merging a pull request may close this issue.

7 participants