fix: unnecessary migrations for unsigned numeric types (#6632)

* fix: Unnecessary migrations for unsigned numeric types

In MariaDB, unsigned numeric types (``<tiny,small,medium>int`) without an explicit `width` property set would generate migrations on every run. This is due to an error in setting the default width for unsigned types.

Fixes #2943

* test: Enable all tests

* refactor: Move isDefaultColumnWidth() method out of BaseQueryRunner

See #6632 (review) for discussion as to why.

* fix: Correct unsigned int behaviour for MySQL 5.7

* fix: Correct position of zerofill check

Plus stylistic change based on code review

Author: michaelbromley

src/driver/aurora-data-api/AuroraDataApiQueryRunner.ts

@@ -1623,4 +1623,26 @@ export class AuroraDataApiQueryRunner extends BaseQueryRunner implements QueryRu
         return c;
     }
 
+    /**
+     * Checks if column display width is by default.
+     */
+    protected isDefaultColumnWidth(table: Table, column: TableColumn, width: number): boolean {
+        // if table have metadata, we check if length is specified in column metadata
+        if (this.connection.hasMetadata(table.name)) {
+            const metadata = this.connection.getMetadata(table.name);
+            const columnMetadata = metadata.findColumnWithDatabaseName(column.name);
+            if (columnMetadata && columnMetadata.width)
+                return false;
+        }
+
+        const defaultWidthForType = this.connection.driver.dataTypeDefaults
+            && this.connection.driver.dataTypeDefaults[column.type]
+            && this.connection.driver.dataTypeDefaults[column.type].width;
+
+        if (defaultWidthForType) {
+            return defaultWidthForType === width;
+        }
+        return false;
+    }
+
 }

src/driver/mysql/MysqlQueryRunner.ts

@@ -1296,6 +1296,8 @@ export class MysqlQueryRunner extends BaseQueryRunner implements QueryRunner {
                     tableColumn.name = dbColumn["COLUMN_NAME"];
                     tableColumn.type = dbColumn["DATA_TYPE"].toLowerCase();
 
+                    tableColumn.zerofill = dbColumn["COLUMN_TYPE"].indexOf("zerofill") !== -1;
+                    tableColumn.unsigned = tableColumn.zerofill ? true : dbColumn["COLUMN_TYPE"].indexOf("unsigned") !== -1;
                     if (this.driver.withWidthColumnTypes.indexOf(tableColumn.type as ColumnType) !== -1) {
                         const width = dbColumn["COLUMN_TYPE"].substring(dbColumn["COLUMN_TYPE"].indexOf("(") + 1, dbColumn["COLUMN_TYPE"].indexOf(")"));
                         tableColumn.width = width && !this.isDefaultColumnWidth(table, tableColumn, parseInt(width)) ? parseInt(width) : undefined;
@@ -1332,8 +1334,6 @@ export class MysqlQueryRunner extends BaseQueryRunner implements QueryRunner {
                     tableColumn.isPrimary = dbPrimaryKeys.some(dbPrimaryKey => {
                         return this.driver.buildTableName(dbPrimaryKey["TABLE_NAME"], undefined, dbPrimaryKey["TABLE_SCHEMA"]) === tableFullName && dbPrimaryKey["COLUMN_NAME"] === tableColumn.name;
                     });
-                    tableColumn.zerofill = dbColumn["COLUMN_TYPE"].indexOf("zerofill") !== -1;
-                    tableColumn.unsigned = tableColumn.zerofill ? true : dbColumn["COLUMN_TYPE"].indexOf("unsigned") !== -1;
                     tableColumn.isGenerated = dbColumn["EXTRA"].indexOf("auto_increment") !== -1;
                     if (tableColumn.isGenerated)
                         tableColumn.generationStrategy = "increment";
@@ -1694,4 +1694,35 @@ export class MysqlQueryRunner extends BaseQueryRunner implements QueryRunner {
         return result[0]["version"];
     }
 
+    /**
+     * Checks if column display width is by default.
+     */
+    protected isDefaultColumnWidth(table: Table, column: TableColumn, width: number): boolean {
+        // if table have metadata, we check if length is specified in column metadata
+        if (this.connection.hasMetadata(table.name)) {
+            const metadata = this.connection.getMetadata(table.name);
+            const columnMetadata = metadata.findColumnWithDatabaseName(column.name);
+            if (columnMetadata && columnMetadata.width)
+                return false;
+        }
+
+        const defaultWidthForType = this.connection.driver.dataTypeDefaults
+            && this.connection.driver.dataTypeDefaults[column.type]
+            && this.connection.driver.dataTypeDefaults[column.type].width;
+
+        if (defaultWidthForType) {
+            // In MariaDB & MySQL 5.7, the default widths of certain numeric types are 1 less than
+            // the usual defaults when the column is unsigned.
+            const typesWithReducedUnsignedDefault = ["int", "tinyint", "smallint", "mediumint"];
+            const needsAdjustment = typesWithReducedUnsignedDefault.indexOf(column.type) !== -1;
+            if (column.unsigned && needsAdjustment) {
+                return (defaultWidthForType - 1) === width;
+            } else {
+                return defaultWidthForType === width;
+            }
+        }
+
+        return false;
+    }
+
 }

src/query-runner/BaseQueryRunner.ts

@@ -312,27 +312,6 @@ export abstract class BaseQueryRunner {
         return false;
     }
 
-    /**
-     * Checks if column display width is by default. Used only for MySQL.
-     */
-    protected isDefaultColumnWidth(table: Table, column: TableColumn, width: number): boolean {
-        // if table have metadata, we check if length is specified in column metadata
-        if (this.connection.hasMetadata(table.name)) {
-            const metadata = this.connection.getMetadata(table.name);
-            const columnMetadata = metadata.findColumnWithDatabaseName(column.name);
-            if (columnMetadata && columnMetadata.width)
-                return false;
-        }
-
-        if (this.connection.driver.dataTypeDefaults
-            && this.connection.driver.dataTypeDefaults[column.type]
-            && this.connection.driver.dataTypeDefaults[column.type].width) {
-            return this.connection.driver.dataTypeDefaults[column.type].width === width;
-        }
-
-        return false;
-    }
-
     /**
      * Checks if column precision is by default.
      */

test/github-issues/2943/entity/Test.ts

@@ -0,0 +1,22 @@
+import { Column, Entity, PrimaryColumn } from "../../../../src";
+
+@Entity()
+export class Test {
+    @PrimaryColumn()
+    id: number;
+
+    @Column({ type: 'int', unsigned: true})
+    uInt: number;
+
+    @Column({ type: 'tinyint', unsigned: true})
+    uTinyInt: number;
+
+    @Column({ type: 'smallint', unsigned: true})
+    uSmallInt: number;
+
+    @Column({ type: 'mediumint', unsigned: true})
+    uMediumInt: number;
+
+    @Column({ type: 'bigint', unsigned: true})
+    uBigInt: number;
+}

test/github-issues/2943/issue-2943.ts

@@ -0,0 +1,29 @@
+import "reflect-metadata";
+import { expect } from "chai";
+import { Connection } from "../../../src";
+import { closeTestingConnections, createTestingConnections, reloadTestingDatabases } from "../../utils/test-utils";
+import { Test } from "./entity/Test";
+
+describe("github issues > #2943 Inappropriate migration generated", () => {
+
+    let connections: Connection[];
+
+    before(async () => {
+        connections = await createTestingConnections({
+            enabledDrivers: ['mariadb', 'mysql'],
+            entities: [Test],
+            schemaCreate: true,
+            dropSchema: true
+        });
+    });
+    beforeEach(() => reloadTestingDatabases(connections));
+    after(() => closeTestingConnections(connections));
+
+    it("should not create migrations for unsigned numeric types with no specified width", () =>
+        Promise.all(connections.map(async (connection) => {
+            const sqlInMemory = await connection.driver.createSchemaBuilder().log();
+
+            expect(sqlInMemory.upQueries).to.eql([]);
+            expect(sqlInMemory.downQueries).to.eql([]);
+        })));
+});