From 745c673de4916d4758ed81c7080b1e392e561e95 Mon Sep 17 00:00:00 2001 From: mertmit Date: Mon, 15 Jan 2024 18:27:15 +0000 Subject: [PATCH 1/2] fix: mm table deletion --- .../src/db/sql-client/lib/pg/PgClient.ts | 21 +++---------------- .../nocodb/src/services/columns.service.ts | 5 +++-- .../nocodb/src/services/tables.service.ts | 3 ++- 3 files changed, 8 insertions(+), 21 deletions(-) diff --git a/packages/nocodb/src/db/sql-client/lib/pg/PgClient.ts b/packages/nocodb/src/db/sql-client/lib/pg/PgClient.ts index ef94e4e07d..39481e6d44 100644 --- a/packages/nocodb/src/db/sql-client/lib/pg/PgClient.ts +++ b/packages/nocodb/src/db/sql-client/lib/pg/PgClient.ts @@ -965,28 +965,13 @@ class PGClient extends KnexClient { f.attnotnull as rqd, p.contype as cst, p.conname as cstn, + ix.indisprimary as primarykey, + not ix.indisunique as non_unique_original, + not ix.indisunique as non_unique, CASE WHEN i.oid<>0 THEN true ELSE false END AS is_index, - CASE - WHEN p.contype = 'p' THEN true - ELSE false - END AS primarykey, - CASE - WHEN p.contype = 'u' THEN 0 - WHEN p.contype = 'p' THEN 0 - ELSE 1 - END AS non_unique_original, - CASE - WHEN p.contype = 'p' THEN true - ELSE false - END AS primarykey, - CASE - WHEN p.contype = 'u' THEN 0 - WHEN p.contype = 'p' THEN 0 - ELSE 1 - END AS non_unique, CASE WHEN f.atthasdef = 't' THEN pg_get_expr(d.adbin, d.adrelid) END AS default FROM pg_attribute f diff --git a/packages/nocodb/src/services/columns.service.ts b/packages/nocodb/src/services/columns.service.ts index db75073e82..34d7655944 100644 --- a/packages/nocodb/src/services/columns.service.ts +++ b/packages/nocodb/src/services/columns.service.ts @@ -2059,6 +2059,7 @@ export class ColumnsService { req?: any; columnId: string; user: UserType; + forceDeleteSystem?: boolean; reuse?: ReusableParams; }, ncMeta = this.metaService, @@ -2067,7 +2068,7 @@ export class ColumnsService { const column = await Column.get({ colId: param.columnId }, ncMeta); - if (column.system) { + if (column.system && !param.forceDeleteSystem) { NcError.badRequest( `The column '${ column.title || column.column_name @@ -2424,7 +2425,7 @@ export class ColumnsService { ...index, tn: cTable.table_name, columns: [childColumn.column_name], - indexName: index.index_name, + indexName: index.key_name, }); } } diff --git a/packages/nocodb/src/services/tables.service.ts b/packages/nocodb/src/services/tables.service.ts index f0a6552c22..40e7e190ef 100644 --- a/packages/nocodb/src/services/tables.service.ts +++ b/packages/nocodb/src/services/tables.service.ts @@ -202,7 +202,7 @@ export class TablesService { // delete all relations for (const c of relationColumns) { // skip if column is hasmany relation to mm table - if (c.system) { + if (c.system && !table.mm) { continue; } @@ -216,6 +216,7 @@ export class TablesService { req: param.req, columnId: c.id, user: param.user, + forceDeleteSystem: true, }, ncMeta, ); From 438a9f554b4b5fef363d279afe2ffbcba33b1eb5 Mon Sep 17 00:00:00 2001 From: mertmit Date: Mon, 15 Jan 2024 18:27:15 +0000 Subject: [PATCH 2/2] fix: allow deletion of link columns with missing mm table --- .../nocodb/src/services/columns.service.ts | 120 ++++++++++-------- 1 file changed, 66 insertions(+), 54 deletions(-) diff --git a/packages/nocodb/src/services/columns.service.ts b/packages/nocodb/src/services/columns.service.ts index 34d7655944..bd34aa95ce 100644 --- a/packages/nocodb/src/services/columns.service.ts +++ b/packages/nocodb/src/services/columns.service.ts @@ -2179,9 +2179,11 @@ export class ColumnsService { colOpt.type === 'mm' && colOpt.fk_parent_column_id === childColumn.id && colOpt.fk_child_column_id === parentColumn.id && - colOpt.fk_mm_model_id === mmTable.id && - colOpt.fk_mm_parent_column_id === mmChildCol.id && - colOpt.fk_mm_child_column_id === mmParentCol.id + colOpt.fk_mm_model_id === relationColOpt.fk_mm_model_id && + colOpt.fk_mm_parent_column_id === + relationColOpt.fk_mm_child_column_id && + colOpt.fk_mm_child_column_id === + relationColOpt.fk_mm_parent_column_id ) { await Column.delete(c.id, ncMeta); break; @@ -2190,14 +2192,16 @@ export class ColumnsService { await Column.delete(relationColOpt.fk_column_id, ncMeta); - // delete bt columns in m2m table - await mmTable.getColumns(ncMeta); - for (const c of mmTable.columns) { - if (!isLinksOrLTAR(c.uidt)) continue; - const colOpt = - await c.getColOptions(ncMeta); - if (colOpt.type === 'bt') { - await Column.delete(c.id, ncMeta); + if (mmTable) { + // delete bt columns in m2m table + await mmTable.getColumns(ncMeta); + for (const c of mmTable.columns) { + if (!isLinksOrLTAR(c.uidt)) continue; + const colOpt = + await c.getColOptions(ncMeta); + if (colOpt.type === 'bt') { + await Column.delete(c.id, ncMeta); + } } } @@ -2207,7 +2211,9 @@ export class ColumnsService { if (!isLinksOrLTAR(c.uidt)) continue; const colOpt = await c.getColOptions(ncMeta); - if (colOpt.fk_related_model_id === mmTable.id) { + if ( + colOpt.fk_related_model_id === relationColOpt.fk_mm_model_id + ) { await Column.delete(c.id, ncMeta); } } @@ -2218,20 +2224,24 @@ export class ColumnsService { if (!isLinksOrLTAR(c.uidt)) continue; const colOpt = await c.getColOptions(ncMeta); - if (colOpt.fk_related_model_id === mmTable.id) { + if ( + colOpt.fk_related_model_id === relationColOpt.fk_mm_model_id + ) { await Column.delete(c.id, ncMeta); } } - // retrieve columns in m2m table again - await mmTable.getColumns(ncMeta); + if (mmTable) { + // retrieve columns in m2m table again + await mmTable.getColumns(ncMeta); - // ignore deleting table if it has more than 2 columns - // the expected 2 columns would be table1_id & table2_id - if (mmTable.columns.length === 2) { - (mmTable as any).tn = mmTable.table_name; - await sqlMgr.sqlOpPlus(source, 'tableDelete', mmTable); - await mmTable.delete(ncMeta); + // ignore deleting table if it has more than 2 columns + // the expected 2 columns would be table1_id & table2_id + if (mmTable.columns.length === 2) { + (mmTable as any).tn = mmTable.table_name; + await sqlMgr.sqlOpPlus(source, 'tableDelete', mmTable); + await mmTable.delete(ncMeta); + } } } break; @@ -2341,42 +2351,44 @@ export class ColumnsService { }, ignoreFkDelete = false, ) => { - let foreignKeyName; - - // if relationColOpt is not provided, extract it from child table - // and get the foreign key name for dropping the foreign key - if (!relationColOpt) { - foreignKeyName = ( - ( - await childTable.getColumns(ncMeta).then(async (cols) => { - for (const col of cols) { - if (col.uidt === UITypes.LinkToAnotherRecord) { - const colOptions = - await col.getColOptions(ncMeta); - if (colOptions.fk_related_model_id === parentTable.id) { - return { colOptions }; + if (childTable) { + let foreignKeyName; + + // if relationColOpt is not provided, extract it from child table + // and get the foreign key name for dropping the foreign key + if (!relationColOpt) { + foreignKeyName = ( + ( + await childTable.getColumns(ncMeta).then(async (cols) => { + for (const col of cols) { + if (col.uidt === UITypes.LinkToAnotherRecord) { + const colOptions = + await col.getColOptions(ncMeta); + if (colOptions.fk_related_model_id === parentTable.id) { + return { colOptions }; + } } } - } - }) - )?.colOptions as LinkToAnotherRecordType - ).fk_index_name; - } else { - foreignKeyName = relationColOpt.fk_index_name; - } + }) + )?.colOptions as LinkToAnotherRecordType + ).fk_index_name; + } else { + foreignKeyName = relationColOpt.fk_index_name; + } - if (!relationColOpt?.virtual && !virtual) { - // todo: handle relation delete exception - try { - await sqlMgr.sqlOpPlus(source, 'relationDelete', { - childColumn: childColumn.column_name, - childTable: childTable.table_name, - parentTable: parentTable.table_name, - parentColumn: parentColumn.column_name, - foreignKeyName, - }); - } catch (e) { - console.log(e.message); + if (!relationColOpt?.virtual && !virtual) { + // todo: handle relation delete exception + try { + await sqlMgr.sqlOpPlus(source, 'relationDelete', { + childColumn: childColumn.column_name, + childTable: childTable.table_name, + parentTable: parentTable.table_name, + parentColumn: parentColumn.column_name, + foreignKeyName, + }); + } catch (e) { + console.log(e.message); + } } }