From 5d1759b40be5881b3bcf860f2481dfe558d60180 Mon Sep 17 00:00:00 2001
From: Henry Tsai <henrytsai@outlook.com>
Date: Mon, 1 Jul 2024 14:24:31 -0700
Subject: [PATCH] Fixed bug where `number` type is too small for unix epoch in
 some SQL 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`
---
 README.md                       |  5 +++++
 sql-dialect-variations.md       | 19 +++++++++++++++++++
 src/resumable-task-store-sql.ts | 14 ++++++++++++--
 tests/test-suite.spec.ts        |  4 ++--
 4 files changed, 38 insertions(+), 4 deletions(-)
 create mode 100644 sql-dialect-variations.md

diff --git a/README.md b/README.md
index 89fbb5d..037aca0 100644
--- a/README.md
+++ b/README.md
@@ -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
@@ -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
diff --git a/sql-dialect-variations.md b/sql-dialect-variations.md
new file mode 100644
index 0000000..911eb49
--- /dev/null
+++ b/sql-dialect-variations.md
@@ -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.
+
diff --git a/src/resumable-task-store-sql.ts b/src/resumable-task-store-sql.ts
index 0b84dcf..385ebb2 100644
--- a/src/resumable-task-store-sql.ts
+++ b/src/resumable-task-store-sql.ts
@@ -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();
@@ -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> {
diff --git a/tests/test-suite.spec.ts b/tests/test-suite.spec.ts
index fcb809b..d211832 100644
--- a/tests/test-suite.spec.ts
+++ b/tests/test-suite.spec.ts
@@ -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),
     });
   });
 
@@ -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),
     });
   });