Kysely•2y ago

Insert expression type safe

Is there a way to make .expression calls while inserting to a table to return an "InsertObject" or an array of column name / value as when using .values method so to have type safety in the query? Example right now:

type Table = { a: string, b: number }
const query = database.insertInto("table").columns(['a']).expression(qb => qb.selectFrom('other_table').select('a'))
// this works but will crash at runtime, cause you need both a nd b columns

type Table = { a: string, b: number }
const query = database.insertInto("table").columns(['a']).expression(qb => qb.selectFrom('other_table').select('a'))
// this works but will crash at runtime, cause you need both a nd b columns

Example wanted:

type Table = { a: string, b: number }
const query1 = database.insertInto("table").columns(['a'] /* type error*/).expression(qb => qb.selectFrom('other_table').select('a'))

const query2 = database.insertInto("table").expression(qb => qb.selectFrom('other_table').select(['a', 'b as bChanged']).mapping({
  a: "a",
  b: "bChanged"
}))
// this or something to link values to columns so that order is not important but it's then decided by the query builder collecting key/values pairs into an array of columns and then array of select in the expression as in:
// insert into table(a, b) select a, b from other_table

type Table = { a: string, b: number }
const query1 = database.insertInto("table").columns(['a'] /* type error*/).expression(qb => qb.selectFrom('other_table').select('a'))

const query2 = database.insertInto("table").expression(qb => qb.selectFrom('other_table').select(['a', 'b as bChanged']).mapping({
  a: "a",
  b: "bChanged"
}))
// this or something to link values to columns so that order is not important but it's then decided by the query builder collecting key/values pairs into an array of columns and then array of select in the expression as in:
// insert into table(a, b) select a, b from other_table

10 Replies

Kristian NotariOP•2y ago

Use case: I have a "history" table that I manually insert to when needed due to some business logic and since I need to add columns to it over time (business rules!) and the history table also manages a "archived_at" column I can't just blindly insert into the history table selecting all the columns from the main table respecting order, cause the "archived_at" colum would be somewhere in between other columns when new columns are added. Do you know how to "move" columns? Cause that seems something which is not supported in sql and postgres in my case, so more type safety would be great here

Igal•2y ago

Hey 👋🏻 Did you manage to find a workaround? I'm thinking about possible edge cases where this wouldn't end up being an InsertResult.. interesting.

koskimas•2y ago

Good point! We should make columns error if you don't list all required columns. Why would you need the mapping function? Why not just give the columns the correct aliases?

Kristian NotariOP•2y ago

Mapping was something to link the expression result to the columns without using the columns method directly. Not needed, really, it was done to give a consistent “ending” data structure for insert expression subquery builders, as when you use OnConflictBuilder and end up using doNothing or DoUpdate etc, whatever you did before Not really as of now, neither I tried honestly. Wanted to know if this was something kysely could take over or not before messing up with types and helpers This and also, the returning type should be somehow linked to those columns you listed. That’s why I was proposing doing everything inside expression directly, returning an InsertObject or something similar And removing columns method altogether (don’t know about other edge cases though)

koskimas•2y ago

The mapping method is too far from SQL. It would also require us to add the mapping method to the SelectQueryBuilder and it would only work in this context. We'd also need to add it to ExpressionBuilder by the way. And in RawBuilder.

Kristian NotariOP•17mo ago

yeah I agree, it was just a starting point to see what constraints would be great to have Btw what i’ve done in userland right now is to have the select query inside the expression return an array of values where the position matters and the type is matched on the insert object types for columns as stated in che columns method for insert. So the array given as the columns is generic and used as reference to check the return value of the select query inside expression But that’s done through a satisfies cause I’m in userland and haven’t played with the kysely typings directly I can share the type as soon as I’m at the pc nevermind, I noticed now this only works when building the actual array of select columns inside the expression, but for it (the check) to happen at the expression method you probably want to look at the returning select query builder. Not sure how's that done though 🙂

koskimas•17mo ago

I took a quick look at this. The columns method: What we need is a way to tell typescript "the user can provide a list of string literals, but the list must contain these items. I don't know if that's possible. We can maybe get that done using tuple types, but I wasn't able to make it work quickly. The expression method First of all, I'd like to rename this to selectFrom. It'd take two arguments 1. The table(s) to select from 2. A callback that gets passed a SelectQueryBuilder. I don't think there are any other "expressions" that can be built there except select from and select. We could require the return value of the callback to extend Insertable<DB[T]> easily, but that's not the correct fix. In reality we need a new generic for InsertQueryBuilder that holds the column name literals passed to the columns method.

Kristian NotariOP•17mo ago

maybe a simpler way to join both constraints would be to allow for any tuple in columns and then check the return type of the selectFrom, filter it for the columns in the tuple given at columns and only THEN check for it being assignable to Insertable<DB[T]> ? obviously you don't get an error by specifying less columns in the columns method, but you will later on using the selectFrom. or maybe you could accept a mapped type with true/false values as columns, forcing true to all the required properties. Order of the columns is then to be handled, depending on how the selectFrom would return those cause there's both the columns type problem and the order problem as far as I know

koskimas•17mo ago

{ foo: true, bar: true } is just too ugly and unlike anything else in the API. We could store the types of the list items passed to columns but if we error in expression the error message will be completely incomrehensible. People won't know what's wrong and that they should fix it in columns.

Kristian NotariOP•17mo ago

You might try giving unique symbol typed error messages like: Actual extends Expected ? Actual : { ErrorId: unique symbol, message: “cannot insert columns… } But yeah I agree that would be a little strange to have that after, even with proper message

Gaming

Programming

Insert expression type safe