Migrate squel to kysely #922
Conversation
| Log.debug('Reading from unsynced table ' + this._tableName) | ||
|
|
||
| const query = db.selectFrom(this._tableName) | ||
| const addFieldSelectionP = this.addFieldSelection.bind(this, i, selectWhereFields ? identificationFields : []) |
There was a problem hiding this comment.
nit: could you add back the comment right before this line to explain what it does:
// only select the fields provided in `i.select` and the ones in `i.where`
| return query.select(fields) | ||
| } | ||
|
|
||
| castBigIntToText(field: string) { |
There was a problem hiding this comment.
Here also, put back the comment that clarifies why this is needed:
/**
* Casts a field to TEXT if it is of type BigInt
* because not all adapters deal well with BigInts
* (e.g. better-sqlite3 requires BigInt support to be enabled
* but then all integers are returned as BigInt...)
* The DAL will convert the string into a BigInt in the `fromSqlite` function from `../conversions/sqlite.ts`.
*/
| const db = new Kysely<any>({ | ||
| dialect: { | ||
| createAdapter() { | ||
| return new PostgresAdapter(); |
There was a problem hiding this comment.
We should use SQLite adapter, introspector and compiler.
The squel version used PG because of a feature of squel that only worked when using the Postgres flavour.
But for Kysely this should not be needed.
…ed them with tests
…method that is yet to be migrated
|
Tests are updated a little bit to match syntax generated by kysely.
|
|
A problem I am currently facing to complete this is migrating the method makeBooleanFilter. With sequel library we were just concatenating the statements but this does not work very easily in kysely. The "OR" statements needs to be add by calling There is another way of using raw SQL which does support parameters automatically however I am unable to implement that. |
|
Hi @ygarg465, what exactly is the problem you're facing with |
|
Hey @kevin-dp, yes that is the code copied from squel implementation and is not working with Kysely. The problem is that all the And there is another way to achieve this which is raw SQL, that is I am unable to understand and implement. |
|
@ygarg465 Do you understand the changes that are needed to make it work with |
|
@kevin-dp I have to look at the implementation again for |
# Conflicts: # clients/typescript/src/client/execution/db.ts # clients/typescript/src/client/execution/transactionalDB.ts
This PR is just a PoC of migrating squel to kysely for now.