kysely-org / kysely

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

Ability to use kysely as standalone sql builder #185

Closed ilijaNL closed 2 years ago

ilijaNL commented 2 years ago

Currently Kysely requires to have some QueryExecutor defined which means that it is not stateless. However I wonder how much work it is to use kysely as a sql builder without an execution implementation, which means it only produces a CompiledQuery. To make it typesafe the CompiledQuery interface could have Result generic:

export interface CompiledQuery<Result> {
  readonly query: RootOperationNode
  readonly sql: string
  readonly parameters: ReadonlyArray<unknown>
  __result?: Result;
}

The CompiledQuery<Result> then can be executed by any driver and still provide the benefit of typesafety, e.g:

import { Pool } from "pg"
import {
  RootOperationNode
} from 'kysely'

export interface CompiledQuery<Result> {
  readonly query: RootOperationNode
  readonly sql: string
  readonly parameters: ReadonlyArray<any>
  __result?: Result;
}

const pool = new Pool()

async function executeQuery<TResult>(query: CompiledQuery<TResult>) {
  return await pool.query<TResult>(query.sql, query.parameters).then(d => d.rows);
}

This has a benefit that Kysely could be used as a stateless query builder and not requiring any connection to any database and managing the state. And easily be used on frontend as well.

What are your thoughts?

igalklebanov commented 2 years ago

You can instantiate a dialect with DummyDriver and compiler of choice and use .compile() instead of .execute().

koskimas commented 2 years ago

You can create an instance just for query compilation like this

  const db = new Kysely<DB>({
    dialect: {
      createAdapter: () => new PostgresAdapter(),
      createDriver: () => new DummyDriver(),
      createIntrospector: (db) => new PostgresIntrospector(db),
      createQueryCompiler: () => new PostgresQueryCompiler(),
    },
  })

The only stateful thing in the whole Kysely instance in that case is the query executor, but absolutely nothing else but the result of DummyDriver.init() is stored in the state, which in this case is a no-op async function. And actually, since you'll never be calling execute on anything, even that state isn't created. The executor is never called.

There is no way to fully separate the QueryExecutor from the rest of the code, but there really is no need to do it.

koskimas commented 2 years ago

None of the classes used in the above example have any references to the actual db driver (pg). The driver is the only place that deals with the underlying database driver, and in this case we've replaced it with a no-op DummyDriver.

ilijaNL commented 2 years ago

Thanks for the quick response, I was also thinking about this implementation. Only necessary thing left is to have the result type information in the compiledquery

koskimas commented 2 years ago

We could consider adding the type information. I'll have to see if it makes sense internally.

igalklebanov commented 2 years ago

Another possible (?) approach, doesn't require compilation..

import { infer } from 'kysely'

const query = db.selectFrom(...).where(...).select(...)

type Result = infer<typeof query>

Basically extract O from query builder.

koskimas commented 2 years ago

I don't think that syntax works? At least not in my tests. And how would that infer the output type and not something else?

edit: Oh sorry, you were thinking about an internal Kysely helper. I didn' see the import.

You can infer it like this though:

interface TypedCompiledQuery<T> extends CompiledQuery {
  // I'm not 100% sure if this is needed unless you want to prevent
  // assignability between different kinds of `TypedCompiledQuery` instances.
  readonly __result: T | undefined
}

function compile<O>(query: SelectQueryBuilder<any, any, O>): TypedCompiledQuery<O> {
  return {
    ...query.compile(),
    __result: undefined,
  }
}

or using a type helper like this:

type GetOutputType<QB> = QB extends SelectQueryBuilder<any, any, infer O> ? O : never
ilijaNL commented 2 years ago

I made a PR: #186

koskimas commented 2 years ago

The change is too invasive and also a breaking change. Since there are multiple ways to get the type information already, I don't think we should merge the PR.

ilijaNL commented 2 years ago

That is fair, what about adding a new method to SelectQueryBuilder, DeleteQueryBuilder, UpdateQueryBuilder, InsertQueryBuilder, called something like compileTyped.

Current workaround is

export interface TypedCompiledQuery<Result> extends CompiledQuery {
  __result?: Result;
}

function compileTyped<O>(q: SelectQueryBuilder<any, any, O> | DeleteQueryBuilder<any, any, O> | UpdateQueryBuilder<any, any, any, O> | InsertQueryBuilder<any, any, O>): TypedCompiledQuery<O> {
  return q.compile();
}
igalklebanov commented 2 years ago

Don't see a good enough reason to have both compile(...) & compileTyped(...) (the name itself is weird, it suggests there's runtime difference between the two).

Also, I don't understand why coupling result type to compilation is so important.. when result type could be easily inferred (we should expose a helper type) from builder.

edit: at best, it removes the need to assign builder to a constant before compiling it and assigning compiled query to another constant. You could simply wrap compilation and execution in a function with return type as generic, and let invoker control it (has context of which query was passed and its return type).

edit2: the use case in general is odd, why not simply wrap your client in a dialect and pass it to Kysely? there are 3 examples of 3rd party dialects in the readme. Some should work in the browser..

ilijaNL commented 2 years ago

Don't see a good enough reason to have both compile(...) & compileTyped(...) (the name itself is weird, it suggests there's runtime difference between the two).

You are right, it implies different runtime behaviour, so probably not good idea.

For my purposes I just want to use kysely as a sql builder without any execution, e.g. only compile method. The type information is needed to make possible to have results (executed by any executor) typed. The idea is similar to https://github.com/dotansimha/graphql-typed-document-node, only for graphql.

the use case in general is odd, why not simply wrap your client in a dialect and pass it to Kysely? there are 3 examples of 3rd party dialects in the readme. Some should work in the browser..

The query builder (e.g. SelectQueryBuilder) still will expose the runtime methods (e.g. execute), which gives other developers a wrong impression that they can directly execute queries, which actually is only catched runtime as far as I know.

Edit: it is probably better for me to fork this repository and remove all the runtime methods from the query builders.

igalklebanov commented 2 years ago

For my purposes I just want to use kysely as a sql builder without any execution, e.g. only compile method. The type information is needed to make possible to have results (executed by any executor) typed. The idea is similar to dotansimha/graphql-typed-document-node, only for graphql.

Not just any executor, an executor that speaks a specific sql dialect (e.g. only postgres)..

Is it an attempt to abstract away query building and make it easily swappable? Or is it an attempt to future-proof the driver side?

edit: Sounds like over-engineering to me, as query building is rarely replaced in a project, and database client world is very stale.

The query builder (e.g. SelectQueryBuilder) still will expose the runtime methods (e.g. execute), which gives other developers a wrong expression that they can directly execute queries, which actually is only catched runtime as far as I know.

So these "other developers" are building queries using Kysely, but are forced to compile and then pass output to an executor, and then have to handle its results? That's very inconvenient..

Edit: it is probably better for me to fork this repository and remove all the runtime methods from the query builders.

You could also use something like patch-package instead of forking.

ilijaNL commented 2 years ago

Is it an attempt to abstract away query building and make it easily swappable? Or is it an attempt to future-proof the driver side?

Not really, I actually don't care about the executor that much but it can be used for that as well. A benefit of that is that other tools and drives can take care of that (they only need to implement the execution of sql dialect, and for postgres, there are a lot open source packages available). Additionally tools like http://vitaly-t.github.io/pg-promise/helpers.html can be used to do some transformations on the generated sql.

However my use cases is more like this: Consider you have a mutation route with some side-effects to the database. Lets consider /create-user creates 2 side effects to the database:

  1. insert user into user table
  2. insert welcome email job into job table

They both should succeed or fail together. The most common solution is to wrap it into a transaction, execute both statements and commit. This however has few drawbacks. First I need to wrap this inside a transaction which makes it harder to test, secondly the connection for that transaction is longer open since it executes per statement and waits for response per statement. This can be really slow when working with edge computing or whenever the database is far from the api server.

Another approach is to generated all sqls with side effects as pure statements, concat the statements (postgres supports this) and send it to the database as a single statement. Big advantages of this is that it can be easily tested. Also all "mutations" are easy composable, you only need to return the side-effect statements and your route handler can execute all returned side-effect statements. You can read my article for more in depth: https://itnext.io/how-to-write-ddd-scalable-and-type-safe-nodejs-backends-e0711403a755. Currently you can achieve this with kysely, which I definetly will use for that, however I personally prefer to split the query-builder side from the execution side. Nevertheless I understand why it is made like this and I really appreciate what you have done.

For further discussion we can connect on discord.

igalklebanov commented 2 years ago

Thank you for sharing! Was really curious about the motivation behind this.. will definitely read that article.