Skip to content

Commit

Permalink
feat (client): client-side support for int8 (#665)
Browse files Browse the repository at this point in the history
This PR adds client-side support for int8 aka bigint.
Supporting BigInts is tricky because of inconsistencies in the sqlite
drivers.
For example, better-sqlite3 requires BigInts to be explicitly enabled
but then *all* integers are returned as BigInts because SQLite does not
distinguish between them (they are all just 64bit integers). On the
other hand, wa-sqlite returns a regular JS number if the integer fits,
otherwise it returns a BigInt.

To avoid these problems, we always cast BigInt to text when reading from
the DB and then let the DAL turn it into a BigInt;
i.e., when the Postgres type of a column "foo" is bigint, we read as
follows: `SELECT cast(foo as TEXT) AS foo FROM table`. Similarly, we
cast returned values: `INSERT INTO table VALUES (9223372036854775807)
RETURNING cast(foo as TEXT) AS foo`.

A similar problem occurs in the oplog triggers. These triggers insert an
oplog entry for every insert/update/delete. This oplog entry contains
the old and new rows as JSON objects. However, JSON does not support
BigInts:
```sql
sqlite> SELECT json_valid('{"a": 123n}');
0
```
We also can't write it as a regular number because when parsing the JSON
it is read as a number and rounding errors occur:
```js
js> JSON.parse('{"a": 9223372036854775807}')
{ a: 9223372036854776000 }
```

Hence, the triggers also need to cast BigInts to text and the
deserialisation function for oplog entries (`deserialiseRow`) needs to
correctly deserialise them as BigInts (when the column type is
int8/bigint).

---------

Co-authored-by: David Martos <[email protected]>
  • Loading branch information
kevin-dp and davidmartos96 authored Nov 28, 2023
1 parent 4fca4bd commit f9f4b47
Show file tree
Hide file tree
Showing 39 changed files with 10,373 additions and 6,270 deletions.
3 changes: 1 addition & 2 deletions clients/typescript/src/client/conversions/input.ts
Original file line number Diff line number Diff line change
Expand Up @@ -331,8 +331,7 @@ function isObject(v: any): boolean {
}

function isFilterObject(value: any): boolean {
// if it is an object it can only be a timestamp or a filter object
// because those are the only objects we support in where clauses
// if it is an object it can only be a data object or a filter object
return isObject(value) && !isDataObject(value)
}

Expand Down
5 changes: 5 additions & 0 deletions clients/typescript/src/client/conversions/sqlite.ts
Original file line number Diff line number Diff line change
Expand Up @@ -50,6 +50,11 @@ export function fromSqlite(v: any, pgType: PgType): any {
) {
// it's a serialised NaN
return NaN
} else if (pgType === PgBasicType.PG_INT8) {
// always return BigInts for PG_INT8 values
// because some drivers (e.g. wa-sqlite) return a regular JS number if the value fits into a JS number
// but we know that it should be a BigInt based on the column type
return BigInt(v)
} else {
return v
}
Expand Down
43 changes: 42 additions & 1 deletion clients/typescript/src/client/model/builder.ts
Original file line number Diff line number Diff line change
Expand Up @@ -13,16 +13,34 @@ import { InvalidArgumentError } from '../validation/errors/invalidArgumentError'
import * as z from 'zod'
import { IShapeManager } from './shapes'
import Log from 'loglevel'
import { ExtendedTableSchema } from './schema'
import { PgBasicType } from '../conversions/types'
import { HKT } from '../util/hkt'

const squelPostgres = squel.useFlavour('postgres')
squelPostgres.registerValueHandler('bigint', function (bigint) {
return bigint.toString()
})

type AnyFindInput = FindInput<any, any, any, any, any>

export class Builder {
constructor(
private _tableName: string,
private _fields: string[],
private shapeManager: IShapeManager
private shapeManager: IShapeManager,
private _tableDescription: ExtendedTableSchema<
any,
any,
any,
any,
any,
any,
any,
any,
any,
HKT
>
) {}

create(i: CreateInput<any, any, any>): QueryBuilder {
Expand Down Expand Up @@ -196,10 +214,26 @@ export class Builder {
const fields = identificationFields
.filter((f) => this._fields.includes(f))
.concat(selectedFields)
.map((f) => this.castBigIntToText(f))

return q.fields(fields)
}

/**
* 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`.
*/
private castBigIntToText(field: string) {
const pgType = this._tableDescription.fields.get(field)
if (pgType === PgBasicType.PG_INT8) {
return `cast(${field} as TEXT) AS ${field}`
}
return field
}

private addOrderBy(i: AnyFindInput, q: PostgresSelect): PostgresSelect {
if (typeof i.orderBy === 'undefined') return q
const orderByArray = Array.isArray(i.orderBy) ? i.orderBy : [i.orderBy]
Expand Down Expand Up @@ -243,6 +277,13 @@ export class Builder {
query: T
): T {
return this._fields.reduce((query, field) => {
// if field is of type BigInt cast the result to TEXT
// because not all adapters deal well with BigInts
// the DAL will convert the string into a BigInt in the `fromSqlite` function from `../conversions/sqlite.ts`.
const pgType = this._tableDescription.fields.get(field)
if (pgType === PgBasicType.PG_INT8) {
return query.returning(`cast(${field} as TEXT) AS ${field}`)
}
return query.returning(field)
}, query)
}
Expand Down
9 changes: 7 additions & 2 deletions clients/typescript/src/client/model/table.ts
Original file line number Diff line number Diff line change
Expand Up @@ -107,12 +107,17 @@ export class Table<
) {
this._fields = this._dbDescription.getFields(tableName)
const fieldNames = this._dbDescription.getFieldNames(tableName)
this._builder = new Builder(tableName, fieldNames, shapeManager)
const tableDescription = this._dbDescription.getTableDescription(tableName)
this._builder = new Builder(
tableName,
fieldNames,
shapeManager,
tableDescription
)
this._executor = new Executor(adapter, notifier, this._fields)
this._shapeManager = shapeManager
this._qualifiedTableName = new QualifiedTablename('main', tableName)
this._tables = new Map()
const tableDescription = this._dbDescription.getTableDescription(tableName)
this._schema = tableDescription.modelSchema
this.createSchema = //transformCreateSchema(
omitCountFromSelectAndIncludeSchema(tableDescription.createSchema) //, fields)
Expand Down
13 changes: 11 additions & 2 deletions clients/typescript/src/migrators/triggers.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,12 @@ type ForeignKey = {

type ColumnName = string
type SQLiteType = string
type ColumnTypes = Record<ColumnName, SQLiteType>
type PgType = string
type ColumnType = {
sqliteType: SQLiteType
pgType: PgType
}
type ColumnTypes = Record<ColumnName, ColumnType>

export type Table = {
tableName: string
Expand Down Expand Up @@ -220,6 +225,7 @@ export function generateTriggers(tables: Tables): Statement[] {
* Joins the column names and values into a string of pairs of the form `'col1', val1, 'col2', val2, ...`
* that can be used to build a JSON object in a SQLite `json_object` function call.
* Values of type REAL are cast to text to avoid a bug in SQLite's `json_object` function (see below).
* Similarly, values of type INT8 (i.e. BigInts) are cast to text because JSON does not support BigInts.
*
* NOTE: There is a bug with SQLite's `json_object` function up to version 3.41.2
* that causes it to return an invalid JSON object if some value is +Infinity or -Infinity.
Expand Down Expand Up @@ -268,7 +274,10 @@ function joinColsForJSON(
// casts the value to TEXT if it is of type REAL
// to work around the bug in SQLite's `json_object` function
const castIfNeeded = (col: string, targettedCol: string) => {
if (colTypes[col] === 'REAL') {
const tpes = colTypes[col]
const sqliteType = tpes.sqliteType
const pgType = tpes.pgType
if (sqliteType === 'REAL' || pgType === 'INT8' || pgType === 'BIGINT') {
return `cast(${targettedCol} as TEXT)`
} else {
return targettedCol
Expand Down
2 changes: 1 addition & 1 deletion clients/typescript/src/satellite/client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1130,6 +1130,7 @@ function deserializeColumnData(
switch (columnType) {
case PgBasicType.PG_CHAR:
case PgDateType.PG_DATE:
case PgBasicType.PG_INT8:
case PgBasicType.PG_TEXT:
case PgDateType.PG_TIME:
case PgDateType.PG_TIMESTAMP:
Expand All @@ -1142,7 +1143,6 @@ function deserializeColumnData(
case PgBasicType.PG_INT:
case PgBasicType.PG_INT2:
case PgBasicType.PG_INT4:
case PgBasicType.PG_INT8:
case PgBasicType.PG_INTEGER:
return Number(typeDecoder.text(column))
case PgBasicType.PG_FLOAT4:
Expand Down
3 changes: 3 additions & 0 deletions clients/typescript/src/satellite/oplog.ts
Original file line number Diff line number Diff line change
Expand Up @@ -338,6 +338,9 @@ function deserialiseRow(str: string, rel: Pick<Relation, 'columns'>): Rec {
return Number(value)
}
}
if (columnType === 'INT8' || columnType === 'BIGINT') {
return BigInt(value)
}
return value
})
}
Expand Down
8 changes: 7 additions & 1 deletion clients/typescript/src/satellite/process.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1471,7 +1471,13 @@ export function generateTriggersForTable(tbl: MigrationTable): Statement[] {
}
}),
columnTypes: Object.fromEntries(
tbl.columns.map((col) => [col.name, col.sqliteType.toUpperCase()])
tbl.columns.map((col) => [
col.name,
{
sqliteType: col.sqliteType.toUpperCase(),
pgType: col.pgType!.name.toUpperCase(),
},
])
),
}
const fullTableName = table.namespace + '.' + table.tableName
Expand Down
8 changes: 6 additions & 2 deletions clients/typescript/src/util/types.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
import type Long from 'long'
import {
SatOpMigrate_Column,
SatOpMigrate_PgColumnType,
SatOpMigrate_Table,
SatOpMigrate_Type,
SatRelation_RelationType,
Expand Down Expand Up @@ -130,9 +131,12 @@ export type DataChange = {
tags: Tag[]
}

// The properties are omitted from columns because they are not currently used.
export type SatOpMigrate_Col = Omit<SatOpMigrate_Column, '$type' | 'pgType'> & {
pgType: Omit<SatOpMigrate_PgColumnType, '$type'> | undefined
}

export type MigrationTable = Omit<SatOpMigrate_Table, '$type' | 'columns'> & {
columns: Omit<SatOpMigrate_Column, '$type' | 'pgType'>[]
columns: SatOpMigrate_Col[]
}

export type SchemaChange = {
Expand Down
3 changes: 2 additions & 1 deletion clients/typescript/test/client/conversions/input.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -31,7 +31,7 @@ await tbl.sync()
function setupDB() {
db.exec('DROP TABLE IF EXISTS DataTypes')
db.exec(
"CREATE TABLE DataTypes('id' int PRIMARY KEY, 'date' varchar, 'time' varchar, 'timetz' varchar, 'timestamp' varchar, 'timestamptz' varchar, 'bool' int, 'uuid' varchar, 'int2' int2, 'int4' int4, 'float8' real, 'relatedId' int);"
"CREATE TABLE DataTypes('id' int PRIMARY KEY, 'date' varchar, 'time' varchar, 'timetz' varchar, 'timestamp' varchar, 'timestamptz' varchar, 'bool' int, 'uuid' varchar, 'int2' int2, 'int4' int4, 'int8' int8, 'float8' real, 'relatedId' int);"
)

db.exec('DROP TABLE IF EXISTS Dummy')
Expand Down Expand Up @@ -232,6 +232,7 @@ const dateNulls = {
bool: null,
int2: null,
int4: null,
int8: null,
float8: null,
uuid: null,
}
Expand Down
23 changes: 22 additions & 1 deletion clients/typescript/test/client/conversions/sqlite.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ await tbl.sync()
function setupDB() {
db.exec('DROP TABLE IF EXISTS DataTypes')
db.exec(
"CREATE TABLE DataTypes('id' int PRIMARY KEY, 'date' varchar, 'time' varchar, 'timetz' varchar, 'timestamp' varchar, 'timestamptz' varchar, 'bool' int, 'uuid' varchar, 'int2' int2, 'int4' int4, 'float8' real, 'relatedId' int);"
"CREATE TABLE DataTypes('id' int PRIMARY KEY, 'date' varchar, 'time' varchar, 'timetz' varchar, 'timestamp' varchar, 'timestamptz' varchar, 'bool' int, 'uuid' varchar, 'int2' int2, 'int4' int4, 'int8' int8, 'float8' real, 'relatedId' int);"
)
}

Expand Down Expand Up @@ -211,3 +211,24 @@ test.serial('floats are converted correctly to SQLite', async (t) => {
{ id: 4, float8: -Infinity },
])
})

test.serial('BigInts are converted correctly to SQLite', async (t) => {
//db.defaultSafeIntegers(true) // enables BigInt support
const bigInt = 9_223_372_036_854_775_807n
await tbl.create({
data: {
id: 1,
int8: bigInt,
},
})

const rawRes = await electric.db.raw({
sql: 'SELECT id, cast(int8 as TEXT) AS int8 FROM DataTypes WHERE id = ?',
args: [1],
})
// because we are executing a raw query,
// the returned BigInt for the `id`
// is not converted into a regular number
t.deepEqual(rawRes, [{ id: 1, int8: bigInt.toString() }])
//db.defaultSafeIntegers(false) // disables BigInt support
})
Loading

0 comments on commit f9f4b47

Please sign in to comment.