Skip to content

Commit

Permalink
fix(client): Fix incorrect field transformation in queries including …
Browse files Browse the repository at this point in the history
…related tables (#1107)

Fixes #1095

All operations must remain synchronous and use callbacks and
continuations due to some driver limitations, which led to the passing
around of `DB` instances that had field transformation and validation
incorporated for a specific table.

This was not an issue for most cases as either related tables won't have
shared column names, or if they do they are likely to also share the
type and thus the field transformation worked.

In cases where a related table might have a column with the same name as
the source table but a different type, the `DB` instance would try to
validate/transform the related table column with the assumption that it
is of the type of the source table column, leading to either 1)
incorrect transformations (like stringified jsons) or 2) errors in
attempts to transform (like strings being parsed as jsons)

I've added a few tests for this case and a workaround by allowing the DB
instances to be cloned with a different table schema. I'd prefer a
cleaner solution but it might require significant refactoring so I think
this will do the trick for now - the bug is serious enough that I think
we should take care of it ASAP.
  • Loading branch information
msfstef authored Apr 3, 2024
1 parent b7e99c8 commit da1b6f6
Show file tree
Hide file tree
Showing 10 changed files with 359 additions and 138 deletions.
5 changes: 5 additions & 0 deletions .changeset/quick-cooks-accept.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"electric-sql": patch
---

Fixed incorrect field transformation for queries with included relations
7 changes: 7 additions & 0 deletions clients/typescript/src/client/execution/db.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { RunResult } from '../../electric/adapter'
import { QueryBuilder } from 'squel'
import * as z from 'zod'
import { Row, Statement } from '../../util'
import { Fields } from '../model/schema'

/**
* Interface that must be implemented by DB implementations.
Expand All @@ -28,4 +29,10 @@ export interface DB {
successCallback: (tx: DB, res: Row[]) => void,
errorCallback?: (error: any) => void
): void

/**
* Get instance with provided table schema for field
* transformation and validation purposes
*/
withTableSchema(fields: Fields): DB
}
4 changes: 4 additions & 0 deletions clients/typescript/src/client/execution/nonTransactionalDB.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,10 @@ import { Fields } from '../model/schema'
export class NonTransactionalDB implements DB {
constructor(private _adapter: DatabaseAdapter, private _fields: Fields) {}

withTableSchema(fields: Fields) {
return new NonTransactionalDB(this._adapter, fields)
}

run(
statement: QueryBuilder,
successCallback?: (db: DB, res: RunResult) => void,
Expand Down
4 changes: 4 additions & 0 deletions clients/typescript/src/client/execution/transactionalDB.ts
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,10 @@ import { Transformation, transformFields } from '../conversions/input'

export class TransactionalDB implements DB {
constructor(private _tx: Transaction, private _fields: Fields) {}

withTableSchema(fields: Fields) {
return new TransactionalDB(this._tx, fields)
}
run(
statement: QueryBuilder,
successCallback?: (db: DB, res: RunResult) => void,
Expand Down
12 changes: 6 additions & 6 deletions clients/typescript/src/client/model/table.ts
Original file line number Diff line number Diff line change
Expand Up @@ -450,7 +450,7 @@ export class Table<
const relatedTbl = this._tables.get(relatedTable)!
relatedTbl._create(
{ data: relatedObject },
db,
db.withTableSchema(relatedTbl._fields),
(createdRelatedObject) => {
delete data[relationField] // remove the relation field
data[fromField] = createdRelatedObject[toField] // fill in the FK
Expand Down Expand Up @@ -502,7 +502,7 @@ export class Table<
relatedObject[fromField] = obj[toField] // fill in FK
relatedTbl._create(
{ data: relatedObject },
db,
db.withTableSchema(relatedTbl._fields),
() => {
oldMakeRelatedObjects(obj, cont)
},
Expand Down Expand Up @@ -731,7 +731,7 @@ export class Table<
},
},
},
db,
db.withTableSchema(otherTable._fields),
(relatedRows: object[]) => {
// Now, join the original `rows` with the `relatedRows`
// where `row.fromField == relatedRow.toField`
Expand Down Expand Up @@ -966,7 +966,7 @@ export class Table<
data: obj.data,
where: whereArg,
},
db,
db.withTableSchema(relatedTbl._fields),
cont,
onError
)
Expand All @@ -981,7 +981,7 @@ export class Table<
[toField]: fromFieldValue,
},
},
db,
db.withTableSchema(relatedTbl._fields),
cont,
onError
)
Expand Down Expand Up @@ -1049,7 +1049,7 @@ export class Table<
[fromField]: originalObject[toField],
},
},
db,
db.withTableSchema(relatedTable._fields),
cont,
onError
)
Expand Down
Loading

0 comments on commit da1b6f6

Please sign in to comment.