Skip to content

Commit

Permalink
Fixed bug where number type is too small for unix epoch in some SQL…
Browse files Browse the repository at this point in the history
… variants (#42)

- Fixed bug where `number` type is too small for unix epoch in some SQL
variants
- Fixed bug where only SQLite was tested against `dwn-sdk-js` test suite
for `ReumsableTaskStore` (the reason why the bug was not caught)
- Special handling for PostgreSQL on `bigint`
  • Loading branch information
thehenrytsai authored Jul 1, 2024
1 parent 5b7468b commit 5d1759b
Show file tree
Hide file tree
Showing 4 changed files with 38 additions and 4 deletions.
5 changes: 5 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,9 @@ SQL backed implementations of DWN `MessageStore`, `DataStore`, and `EventLog`.
* MySQL ✔️
* PostgreSQL ✔️

> NOTE:
See [SQL Dialect Variations](sql-dialect-variations) for the list of special handling to support the above SQL variations.

# Installation

```bash
Expand Down Expand Up @@ -171,3 +174,5 @@ Docker is used to spin up a local containerized DBs for testing purposes. Docker
| `npm run test-coverage` | runs tests and includes coverage |
| `npm run lint` | runs linter |
| `npm run lint:fix` | runs linter and fixes auto-fixable problems |

[codeowners-link]: https://github.com/TBD54566975/web5-js/blob/main/sql-dialect-variations.md
19 changes: 19 additions & 0 deletions sql-dialect-variations.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
# SQL Dialect Variations

We use `Kysely` to help us abstract differences between SQL variants in this codebase. Unfortunately many differences are not fully abstracted away. This document outlines the specific handling required in this codebase to support variations between MySQL, PostgreSQL, and SQLite. Its purpose is to help future repository maintainers avoid known pitfalls. Note that this may not be an exhaustive list of all special handling within the codebase.

## MySQL

- Does not support adding a reference column directly, requires adding a foreign key constraint instead.

- When inserting values into a table, the inserted values are returned by default, unlike PostgreSQL and SQLite.


## PostgreSQL

- Uses a special type: `serial` for auto-increment columns.

- Uses a special type: `bytea` for blob columns.

- `bigint` column type gets returned as `string` in `pg` library.

14 changes: 12 additions & 2 deletions src/resumable-task-store-sql.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,7 @@ export class ResumableTaskStoreSql implements ResumableTaskStore {
.ifNotExists()
.addColumn('id', 'varchar(255)', (col) => col.primaryKey())
.addColumn('task', 'text')
.addColumn('timeout', 'integer')
.addColumn('timeout', 'bigint')
.addColumn('retryCount', 'integer');

await table.execute();
Expand Down Expand Up @@ -111,11 +111,21 @@ export class ResumableTaskStoreSql implements ResumableTaskStore {
throw new Error('Connection to database not open. Call `open` before using `read`.');
}

return this.#db
const task = await this.#db
.selectFrom('resumableTasks')
.selectAll()
.where('id', '=', taskId)
.executeTakeFirst();

if (task !== undefined) {
// NOTE: special handling ONLY for PostgreSQL:
// Even though PostgreSQL stores `bigint` as a 64 bit number, the `pg` library we depend on returns it as a string, hence the conversion.
if (typeof task.timeout !== 'number') {
task.timeout = parseInt(task.timeout, 10);
}
}

return task;
}

async extend(taskId: string, timeoutInSeconds: number): Promise<void> {
Expand Down
4 changes: 2 additions & 2 deletions tests/test-suite.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@ describe('SQL Store Test Suite', () => {
messageStore : new MessageStoreSql(testMysqlDialect),
dataStore : new DataStoreSql(testMysqlDialect),
eventLog : new EventLogSql(testMysqlDialect),
resumableTaskStore : new ResumableTaskStoreSql(testSqliteDialect),
resumableTaskStore : new ResumableTaskStoreSql(testMysqlDialect),
});
});

Expand All @@ -26,7 +26,7 @@ describe('SQL Store Test Suite', () => {
messageStore : new MessageStoreSql(testPostgresDialect),
dataStore : new DataStoreSql(testPostgresDialect),
eventLog : new EventLogSql(testPostgresDialect),
resumableTaskStore : new ResumableTaskStoreSql(testSqliteDialect),
resumableTaskStore : new ResumableTaskStoreSql(testPostgresDialect),
});
});

Expand Down

0 comments on commit 5d1759b

Please sign in to comment.