Update some comments now that knex has some partial sqlite3 returning() support

Signed-off-by: Fredrik Adelöw <freben@gmail.com>

packages/backend-tasks/src/tasks/PluginTaskSchedulerJanitor.ts

@@ -53,13 +53,6 @@ export class PluginTaskSchedulerJanitor {
   }
 
   private async runOnce() {
-    // SQLite currently (Oct 1 2021) returns a number for returning()
-    // statements, effectively ignoring them and instead returning the outcome
-    // of the delete() - and knex also emits a warning about that fact, which
-    // is why we avoid that entirely for the sqlite3 family of drivers.
-    // https://github.com/knex/knex/issues/4370
-    // https://github.com/mapbox/node-sqlite3/issues/1453
-
     const dbNull = this.knex.raw('null');
 
     const tasks = await this.knex<DbTasksRow>(DB_TASKS_TABLE)
@@ -71,7 +64,8 @@ export class PluginTaskSchedulerJanitor {
       })
       .returning(['id']);
 
-    // sqlite ignores "returning", returns number of rows changed instead
+    // In rare cases, knex drivers may ignore "returning", and return the number
+    // of rows changed instead
     if (typeof tasks === 'number') {
       if (tasks > 0) {
         this.logger.warn(`${tasks} tasks timed out and were lost`);

plugins/catalog-backend/src/database/DefaultProcessingDatabase.ts

@@ -112,6 +112,7 @@ export class DefaultProcessingDatabase implements ProcessingDatabase {
     });
 
     // Delete old relations
+    // NOTE(freben): knex implemented support for returning() on update queries for sqlite, but at the current time of writing (Sep 2022) not for delete() queries.
     let previousRelationRows: DbRelationsRow[];
     if (
       tx.client.config.client.includes('sqlite3') ||