fix: Supposed to alter only existing columns (#3120)

* fix: Supposed to alter only existing columns * test: If existing columns are changed
ceramicnetwork · Jan 26, 2024 · b5ee70e · b5ee70e
1 parent 7e7f91f
commit b5ee70e
Show file tree

Hide file tree

Showing 2 changed files with 53 additions and 20 deletions.
diff --git a/packages/indexing/src/__tests__/database-index-api.test.ts b/packages/indexing/src/__tests__/database-index-api.test.ts
@@ -1,4 +1,4 @@
-import { jest } from '@jest/globals'
+import { expect, jest, test } from '@jest/globals'
 import { StreamID } from '@ceramicnetwork/streamid'
 import knex, { Knex } from 'knex'
 import tmp from 'tmp-promise'
@@ -144,6 +144,19 @@ describe('postgres', () => {
         expect(JSON.stringify(columns)).toEqual(JSON.stringify(STRUCTURE.CONFIG_TABLE_MODEL_INDEX))
       })
 
+      test('change only existing columns', async () => {
+        const modelsToIndex = [{ model: StreamID.fromString(STREAM_ID_A) }, { model: Model.MODEL }]
+        const indexApi = new PostgresIndexApi(dbConnection, true, logger, Networks.INMEMORY)
+        indexApi.setSyncQueryApi(new CompleteQueryApi())
+        await indexApi.init()
+        await indexApi.indexModels(modelsToIndex)
+        await expect(
+          indexApi.tablesManager.initMidTables([
+            { model: modelsToIndex[0].model, relations: { foo: { type: 'account' } } },
+          ])
+        ).resolves.not.toThrow()
+      })
+
       test('create new table with indices', async () => {
         const modelToIndex = StreamID.fromString(STREAM_ID_A)
         const indexApi = new PostgresIndexApi(dbConnection, true, logger, Networks.INMEMORY)
@@ -1007,6 +1020,19 @@ describe('sqlite', () => {
         )
       })
 
+      test('change only existing columns', async () => {
+        const modelsToIndex = [{ model: StreamID.fromString(STREAM_ID_A) }, { model: Model.MODEL }]
+        const indexApi = new SqliteIndexApi(dbConnection, true, logger, Networks.INMEMORY)
+        indexApi.setSyncQueryApi(new CompleteQueryApi())
+        await indexApi.init()
+        await indexApi.indexModels(modelsToIndex)
+        await expect(
+          indexApi.tablesManager.initMidTables([
+            { model: modelsToIndex[0].model, relations: { foo: { type: 'account' } } },
+          ])
+        ).resolves.not.toThrow()
+      })
+
       test('create new table with indices', async () => {
         const modelToIndex = StreamID.fromString(STREAM_ID_A)
         const indexApi = new SqliteIndexApi(dbConnection, true, logger, Networks.INMEMORY)

diff --git a/packages/indexing/src/tables-manager.ts b/packages/indexing/src/tables-manager.ts
@@ -1,24 +1,24 @@
 import {
-  DatabaseType,
   ColumnInfo,
   ColumnType,
   createConfigTable,
+  createPostgresIndices,
   createPostgresModelTable,
+  createSqliteIndices,
   createSqliteModelTable,
+  DatabaseType,
   defaultIndices,
-  createSqliteIndices,
-  createPostgresIndices,
   migrateConfigTable,
 } from './migrations/1-create-model-table.js'
 import { asTableName } from './as-table-name.util.js'
 import { Knex } from 'knex'
 import { Model, ModelRelationsDefinition } from '@ceramicnetwork/stream-model'
 import { DiagnosticsLogger, Networks } from '@ceramicnetwork/common'
 import {
+  fieldsIndexName,
   INDEXED_MODEL_CONFIG_TABLE_NAME,
   IndexModelArgs,
   MODEL_IMPLEMENTS_TABLE_NAME,
-  fieldsIndexName,
 } from './database-index-api.js'
 import { STRUCTURES } from './migrations/cdb-schema-verification.js'
 import { CONFIG_TABLE_NAME } from './config.js'
@@ -219,8 +219,7 @@ export class TablesManager {
     if (missingIndices.length > 0) {
       throw new Error(`Schema verification failed for index: ${tableName}. Please make sure latest migrations have been applied.
           Missing Indices=${JSON.stringify(missingIndices)}
-          Actual=${JSON.stringify(actual)}`
-      )
+          Actual=${JSON.stringify(actual)}`)
     }
   }
 }
@@ -283,13 +282,18 @@ export class PostgresTablesManager extends TablesManager {
         await createPostgresIndices(this.dataSource, tableName, modelIndexArgs.indices)
       }
     } else if (relationColumns.length) {
+      const columnNamesToChange: Array<string> = []
+      for (const column of relationColumns) {
+        if (column.type === ColumnType.STRING) {
+          const columnName = addColumnPrefix(column.name)
+          const isColumnPresent = await this.dataSource.schema.hasColumn(tableName, columnName)
+          if (isColumnPresent) columnNamesToChange.push(columnName)
+        }
+      }
       // Make relations columns nullable
       await this.dataSource.schema.alterTable(tableName, (table) => {
-        for (const column of relationColumns) {
-          if (column.type === ColumnType.STRING) {
-            const columnName = addColumnPrefix(column.name)
-            table.string(columnName, 1024).nullable().alter()
-          }
+        for (const columnName of columnNamesToChange) {
+          table.string(columnName, 1024).nullable().alter()
         }
       })
     }
@@ -307,13 +311,11 @@ export class PostgresTablesManager extends TablesManager {
         expectedIndices.push(fieldsIndexName(index, tableName).toLowerCase())
       }
     }
-    const indicesResult = (
-      await this.dataSource.raw<PgIndexResult>(`
+    const indicesResult = await this.dataSource.raw<PgIndexResult>(`
 select *
 from pg_indexes
 where tablename like '${tableName}'
     `)
-    )
     const actualIndices = indicesResult ? indicesResult.rows.map((row) => row.indexname) : []
     this.validateIndices(tableName, expectedIndices, actualIndices)
   }
@@ -367,13 +369,18 @@ export class SqliteTablesManager extends TablesManager {
     const relationColumns = relationsDefinitionsToColumnInfo(modelIndexArgs.relations)
     if (existingTables.includes(tableName)) {
       if (relationColumns.length) {
+        const columnNamesToChange: Array<string> = []
+        for (const column of relationColumns) {
+          if (column.type === ColumnType.STRING) {
+            const columnName = addColumnPrefix(column.name)
+            const isColumnPresent = await this.dataSource.schema.hasColumn(tableName, columnName)
+            if (isColumnPresent) columnNamesToChange.push(columnName)
+          }
+        }
         // Make relations columns nullable
         await this.dataSource.schema.alterTable(tableName, (table) => {
-          for (const column of relationColumns) {
-            if (column.type === ColumnType.STRING) {
-              const columnName = addColumnPrefix(column.name)
-              table.string(columnName, 1024).nullable().alter()
-            }
+          for (const columnName of columnNamesToChange) {
+            table.string(columnName, 1024).nullable().alter()
           }
         })
       }