From 06ed9d7dfc2ff1f79359326da98326381b6b10fe Mon Sep 17 00:00:00 2001 From: Jan Prochazka Date: Mon, 18 Nov 2024 12:40:53 +0100 Subject: [PATCH 1/4] fix --- packages/tools/src/alterPlan.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/tools/src/alterPlan.ts b/packages/tools/src/alterPlan.ts index dbc3d43fe..ee8be2e9c 100644 --- a/packages/tools/src/alterPlan.ts +++ b/packages/tools/src/alterPlan.ts @@ -459,7 +459,7 @@ export class AlterPlan { // console.log('*****************RECREATED NEEDED', op, operationType, isAllowed); // console.log(this.dialect); - if (!this.opts.allowTableRecreate) { + if (this.opts.noDropTable && !this.opts.allowTableRecreate) { // skip this operation, as it cannot be achieved return []; } From 85c482160623f51e53deb16c362f67e829059c4e Mon Sep 17 00:00:00 2001 From: Jan Prochazka Date: Mon, 18 Nov 2024 12:48:34 +0100 Subject: [PATCH 2/4] added deploy test --- .../__tests__/deploy-database.spec.js | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/integration-tests/__tests__/deploy-database.spec.js b/integration-tests/__tests__/deploy-database.spec.js index 61fc3aa6d..aac6aad88 100644 --- a/integration-tests/__tests__/deploy-database.spec.js +++ b/integration-tests/__tests__/deploy-database.spec.js @@ -428,6 +428,18 @@ describe('Deploy database', () => { }, }; + const T2 = { + name: 't2.table.yaml', + json: { + name: 't2', + columns: [ + { name: 'id', type: 'int' }, + { name: 'val', type: 'int' }, + ], + primaryKey: ['id'], + }, + }; + const T1_DELETED = { name: '_deleted_t1.table.yaml', json: { @@ -686,4 +698,15 @@ describe('Deploy database', () => { expect(res5.rows[0].run_count == 2).toBeTruthy(); }) ); + + test.each(engines.map(engine => [engine.label, engine]))( + 'Mark table removed, one remains - %s', + testWrapper(async (conn, driver, engine) => { + await testDatabaseDeploy(engine, conn, driver, [[T1, T2], [T2], [T2]], { + markDeleted: true, + disallowExtraObjects: true, + finalCheckAgainstModel: [T1_DELETED, T2], + }); + }) + ); }); From 1ce8f6bd1f73077ca5e854242316337cce168279 Mon Sep 17 00:00:00 2001 From: Jan Prochazka Date: Mon, 18 Nov 2024 13:41:29 +0100 Subject: [PATCH 3/4] set NOT NULL for column with default value --- .../__tests__/deploy-database.spec.js | 90 ++++++++++++++----- packages/tools/src/SqlDumper.ts | 6 ++ .../src/frontend/MsSqlDumper.js | 1 + .../src/frontend/Dumper.js | 9 +- 4 files changed, 80 insertions(+), 26 deletions(-) diff --git a/integration-tests/__tests__/deploy-database.spec.js b/integration-tests/__tests__/deploy-database.spec.js index aac6aad88..480ba1e09 100644 --- a/integration-tests/__tests__/deploy-database.spec.js +++ b/integration-tests/__tests__/deploy-database.spec.js @@ -82,30 +82,34 @@ async function testDatabaseDeploy(engine, conn, driver, dbModelsYaml, options) { dbdiffOptionsExtra.schemaMode = 'ignore'; for (const loadedDbModel of dbModelsYaml) { - const { sql, isEmpty } = await generateDeploySql({ - systemConnection: conn.isPreparedOnly ? undefined : conn, - connection: conn.isPreparedOnly ? conn : undefined, - driver, - loadedDbModel, - dbdiffOptionsExtra, - }); - console.debug('Generated deploy script:', sql); - if (!allowDropStatements) { - expect(sql.toUpperCase().includes('DROP ')).toBeFalsy(); - } + if (_.isString(loadedDbModel)) { + await driver.script(conn, loadedDbModel); + } else { + const { sql, isEmpty } = await generateDeploySql({ + systemConnection: conn.isPreparedOnly ? undefined : conn, + connection: conn.isPreparedOnly ? conn : undefined, + driver, + loadedDbModel, + dbdiffOptionsExtra, + }); + console.debug('Generated deploy script:', sql); + if (!allowDropStatements) { + expect(sql.toUpperCase().includes('DROP ')).toBeFalsy(); + } - console.log('dbModelsYaml.length', dbModelsYaml.length, index); - if (testEmptyLastScript && index == dbModelsYaml.length - 1) { - expect(isEmpty).toBeTruthy(); - } + console.log('dbModelsYaml.length', dbModelsYaml.length, index); + if (testEmptyLastScript && index == dbModelsYaml.length - 1) { + expect(isEmpty).toBeTruthy(); + } - await deployDb({ - systemConnection: conn.isPreparedOnly ? undefined : conn, - connection: conn.isPreparedOnly ? conn : undefined, - driver, - loadedDbModel, - dbdiffOptionsExtra, - }); + await deployDb({ + systemConnection: conn.isPreparedOnly ? undefined : conn, + connection: conn.isPreparedOnly ? conn : undefined, + driver, + loadedDbModel, + dbdiffOptionsExtra, + }); + } index++; } @@ -416,6 +420,48 @@ describe('Deploy database', () => { }) ); + test.each(engines.map(engine => [engine.label, engine]))( + 'Change column to NOT NULL column with default - %s', + testWrapper(async (conn, driver, engine) => { + await testDatabaseDeploy(engine, conn, driver, [ + [ + { + name: 't1.table.yaml', + json: { + name: 't1', + columns: [ + { name: 'id', type: 'int', notNull: true }, + { name: 'val', type: 'int' }, + ], + + primaryKey: ['id'], + }, + }, + ], + 'insert into t1 (id, val) values (1, 1); insert into t1 (id) values (2)', + [ + { + name: 't1.table.yaml', + json: { + name: 't1', + columns: [ + { name: 'id', type: 'int', notNull: true }, + { name: 'val', type: 'int', notNull: true, default: '20' }, + ], + primaryKey: ['id'], + }, + }, + ], + ]); + + const res1 = await driver.query(conn, `select val from t1 where id = 1`); + expect(res1.rows[0].val).toEqual(1); + + const res2 = await driver.query(conn, `select val from t1 where id = 2`); + expect(res2.rows[0].val).toEqual(20); + }) + ); + const T1 = { name: 't1.table.yaml', json: { diff --git a/packages/tools/src/SqlDumper.ts b/packages/tools/src/SqlDumper.ts index a5f6ffca4..8fa3a83f2 100644 --- a/packages/tools/src/SqlDumper.ts +++ b/packages/tools/src/SqlDumper.ts @@ -731,6 +731,12 @@ export class SqlDumper implements AlterProcessor { this.put(formatString, optionValue); } + fillNewNotNullDefaults(col: ColumnInfo) { + if (col.notNull && col.defaultValue) { + this.putCmd('^update %f ^set %i = %s ^where %i ^is ^null', col, col.columnName, col.defaultValue, col.columnName); + } + } + fillPreloadedRows( table: NamedObjectInfo, oldRows: any[], diff --git a/plugins/dbgate-plugin-mssql/src/frontend/MsSqlDumper.js b/plugins/dbgate-plugin-mssql/src/frontend/MsSqlDumper.js index 68f599535..b87c6b44e 100644 --- a/plugins/dbgate-plugin-mssql/src/frontend/MsSqlDumper.js +++ b/plugins/dbgate-plugin-mssql/src/frontend/MsSqlDumper.js @@ -132,6 +132,7 @@ class MsSqlDumper extends SqlDumper { } else { this.dropDefault(oldcol); if (oldcol.columnName != newcol.columnName) this.renameColumn(oldcol, newcol.columnName); + this.fillNewNotNullDefaults(newcol); this.put('^alter ^table %f ^alter ^column %i ', oldcol, oldcol.columnName, newcol.columnName); this.columnDefinition(newcol, { includeDefault: false }); this.endCommand(); diff --git a/plugins/dbgate-plugin-postgres/src/frontend/Dumper.js b/plugins/dbgate-plugin-postgres/src/frontend/Dumper.js index dd269f590..963fc1dc1 100644 --- a/plugins/dbgate-plugin-postgres/src/frontend/Dumper.js +++ b/plugins/dbgate-plugin-postgres/src/frontend/Dumper.js @@ -76,10 +76,6 @@ class Dumper extends SqlDumper { if (!testEqualTypes(oldcol, newcol)) { this.putCmd('^alter ^table %f ^alter ^column %i ^type %s', oldcol, newcol.columnName, newcol.dataType); } - if (oldcol.notNull != newcol.notNull) { - if (newcol.notNull) this.putCmd('^alter ^table %f ^alter ^column %i ^set ^not ^null', newcol, newcol.columnName); - else this.putCmd('^alter ^table %f ^alter ^column %i ^drop ^not ^null', newcol, newcol.columnName); - } if (oldcol.defaultValue != newcol.defaultValue) { if (newcol.defaultValue == null) { this.putCmd('^alter ^table %f ^alter ^column %i ^drop ^default', newcol, newcol.columnName); @@ -92,6 +88,11 @@ class Dumper extends SqlDumper { ); } } + if (oldcol.notNull != newcol.notNull) { + this.fillNewNotNullDefaults(newcol); + if (newcol.notNull) this.putCmd('^alter ^table %f ^alter ^column %i ^set ^not ^null', newcol, newcol.columnName); + else this.putCmd('^alter ^table %f ^alter ^column %i ^drop ^not ^null', newcol, newcol.columnName); + } } putValue(value) { From 71a9d6c5c0214b41c756e7ca1c1e5a9f1e39c08a Mon Sep 17 00:00:00 2001 From: Jan Prochazka Date: Mon, 18 Nov 2024 13:47:23 +0100 Subject: [PATCH 4/4] fix --- packages/tools/src/SqlDumper.ts | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/packages/tools/src/SqlDumper.ts b/packages/tools/src/SqlDumper.ts index 8fa3a83f2..e9d46b1d3 100644 --- a/packages/tools/src/SqlDumper.ts +++ b/packages/tools/src/SqlDumper.ts @@ -732,7 +732,7 @@ export class SqlDumper implements AlterProcessor { } fillNewNotNullDefaults(col: ColumnInfo) { - if (col.notNull && col.defaultValue) { + if (col.notNull && col.defaultValue != null) { this.putCmd('^update %f ^set %i = %s ^where %i ^is ^null', col, col.columnName, col.defaultValue, col.columnName); } }