feat: support for CTE expressions - code review remarks

docker-compose.yml

@@ -24,6 +24,17 @@ services:
       MYSQL_PASSWORD: "test"
       MYSQL_DATABASE: "test"
 
+  mysql8:
+    image: "mysql:8.0.28"
+    container_name: "typeorm-mysql8"
+    ports:
+      - "3308:3306"
+    environment:
+      MYSQL_ROOT_PASSWORD: "admin"
+      MYSQL_USER: "test"
+      MYSQL_PASSWORD: "test"
+      MYSQL_DATABASE: "test"
+
   # mariadb
   mariadb:
     image: "mariadb:10.5.13"

ormconfig.json.dist

@@ -10,6 +10,18 @@
     "database": "test",
     "logging": false
   },
+  {
+    "skip": false,
+    "name": "mysql8",
+    "connectorPackage": "mysql2",
+    "type": "mysql",
+    "host": "localhost",
+    "port": 3308,
+    "username": "root",
+    "password": "admin",
+    "database": "test",
+    "logging": false
+  },
   {
     "skip": false,
     "name": "mariadb",

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

@@ -310,7 +310,9 @@ export class AuroraDataApiDriver implements Driver {
      */
     maxAliasLength = 63;
 
-    cteCapabilities: CteCapabilities = {};
+    cteCapabilities: CteCapabilities = {
+        enabled: true,
+    };
 
     // -------------------------------------------------------------------------
     // Constructor

src/driver/cockroachdb/CockroachDriver.ts

@@ -239,7 +239,12 @@ export class CockroachDriver implements Driver {
      */
     maxAliasLength?: number;
 
-    cteCapabilities: CteCapabilities = { writable: true, materializedHint: true, requiresRecursiveHint: true };
+    cteCapabilities: CteCapabilities = {
+        enabled: true,
+        writable: true,
+        materializedHint: true,
+        requiresRecursiveHint: true
+    };
 
     // -------------------------------------------------------------------------
     // Constructor

src/driver/mongodb/MongoDriver.ts

@@ -213,7 +213,9 @@ export class MongoDriver implements Driver {
         "retryWrites"
     ];
 
-    cteCapabilities: CteCapabilities = {};
+    cteCapabilities: CteCapabilities = {
+        enabled: false,
+    };
 
     // -------------------------------------------------------------------------
     // Constructor

src/driver/mysql/MysqlConnectionOptions.ts

@@ -143,4 +143,9 @@ export interface MysqlConnectionOptions extends BaseConnectionOptions, MysqlConn
 
     };
 
+    /**
+     * TypeORM will automatically use package found in your node_modules, prioritizing mysql over mysql2,
+     * but you can specify it manually
+     */
+    readonly connectorPackage?: "mysql" | "mysql2";
 }

src/driver/mysql/MysqlDriver.ts

@@ -321,6 +321,7 @@ export class MysqlDriver implements Driver {
     };
 
     cteCapabilities: CteCapabilities = {
+        enabled: false,
         requiresRecursiveHint: true,
     };
 
@@ -381,17 +382,25 @@ export class MysqlDriver implements Driver {
             await queryRunner.release();
         }
 
-        if (this.options.type === "mariadb") {
-            const result = await this.createQueryRunner("master")
-                .query(`SELECT VERSION() AS \`version\``) as { version: string; }[];
-            const dbVersion = result[0].version;
+        const result = await this.createQueryRunner("master")
+            .query(`SELECT VERSION() AS \`version\``) as { version: string; }[];
+        const dbVersion = result[0].version;
 
+        if (this.options.type === "mariadb") {
             if (VersionUtils.isGreaterOrEqual(dbVersion, "10.0.5")) {
                 this._isReturningSqlSupported.delete = true;
             }
             if (VersionUtils.isGreaterOrEqual(dbVersion, "10.5.0")) {
                 this._isReturningSqlSupported.insert = true;
             }
+            if (VersionUtils.isGreaterOrEqual(dbVersion, "10.2.0")) {
+                this.cteCapabilities.enabled = true;
+            }
+        }
+        if (this.options.type === "mysql") {
+            if(VersionUtils.isGreaterOrEqual(dbVersion, "8.0.0")) {
+                this.cteCapabilities.enabled = true;
+            }
         }
     }
 
@@ -955,9 +964,10 @@ export class MysqlDriver implements Driver {
      * Loads all driver dependencies.
      */
     protected loadDependencies(): void {
+        const defaultPackage = this.options.connectorPackage || "mysql";
         try {
             // try to load first supported package
-            const mysql = this.options.driver || PlatformTools.load("mysql");
+            const mysql = this.options.driver || PlatformTools.load(defaultPackage);
             this.mysql = mysql;
             /*
              * Some frameworks (such as Jest) may mess up Node's require cache and provide garbage for the 'mysql' module

src/driver/oracle/OracleDriver.ts

@@ -1,7 +1,7 @@
 import {Driver} from "../Driver";
 import {ConnectionIsNotSetError} from "../../error/ConnectionIsNotSetError";
 import {DriverPackageNotInstalledError} from "../../error/DriverPackageNotInstalledError";
-import { CteCapabilities } from "../types/CteCapabilities";
+import {CteCapabilities} from "../types/CteCapabilities";
 import {OracleQueryRunner} from "./OracleQueryRunner";
 import {ObjectLiteral} from "../../common/ObjectLiteral";
 import {ColumnMetadata} from "../../metadata/ColumnMetadata";
@@ -223,7 +223,9 @@ export class OracleDriver implements Driver {
      */
     maxAliasLength = 30;
 
-    cteCapabilities: CteCapabilities = {};
+    cteCapabilities: CteCapabilities = {
+        enabled: true,
+    };
 
     // -------------------------------------------------------------------------
     // Constructor

src/driver/postgres/PostgresDriver.ts

@@ -13,7 +13,7 @@ import {DateUtils} from "../../util/DateUtils";
 import {OrmUtils} from "../../util/OrmUtils";
 import {Driver} from "../Driver";
 import {ColumnType} from "../types/ColumnTypes";
-import { CteCapabilities } from "../types/CteCapabilities";
+import {CteCapabilities} from "../types/CteCapabilities";
 import {DataTypeDefaults} from "../types/DataTypeDefaults";
 import {MappedColumnTypes} from "../types/MappedColumnTypes";
 import {ReplicationMode} from "../types/ReplicationMode";
@@ -278,6 +278,7 @@ export class PostgresDriver implements Driver {
     maxAliasLength = 63;
 
     cteCapabilities: CteCapabilities = {
+        enabled: true,
         writable: true,
         requiresRecursiveHint: true,
         materializedHint: true,

src/driver/sap/SapDriver.ts

@@ -16,7 +16,7 @@ import {ApplyValueTransformers} from "../../util/ApplyValueTransformers";
 import {DateUtils} from "../../util/DateUtils";
 import {OrmUtils} from "../../util/OrmUtils";
 import {Driver} from "../Driver";
-import { CteCapabilities } from "../types/CteCapabilities";
+import {CteCapabilities} from "../types/CteCapabilities";
 import {DataTypeDefaults} from "../types/DataTypeDefaults";
 import {MappedColumnTypes} from "../types/MappedColumnTypes";
 import {SapConnectionOptions} from "./SapConnectionOptions";
@@ -212,7 +212,9 @@ export class SapDriver implements Driver {
      */
     maxAliasLength = 128;
 
-    cteCapabilities: CteCapabilities = {};
+    cteCapabilities: CteCapabilities = {
+        enabled: true,
+    };
 
     // -------------------------------------------------------------------------
     // Constructor

src/driver/sqlite-abstract/AbstractSqliteDriver.ts

@@ -4,7 +4,7 @@ import {ColumnMetadata} from "../../metadata/ColumnMetadata";
 import {DateUtils} from "../../util/DateUtils";
 import {Connection} from "../../connection/Connection";
 import {RdbmsSchemaBuilder} from "../../schema-builder/RdbmsSchemaBuilder";
-import { CteCapabilities } from "../types/CteCapabilities";
+import {CteCapabilities} from "../types/CteCapabilities";
 import {MappedColumnTypes} from "../types/MappedColumnTypes";
 import {ColumnType} from "../types/ColumnTypes";
 import {QueryRunner} from "../../query-runner/QueryRunner";
@@ -223,6 +223,7 @@ export abstract class AbstractSqliteDriver implements Driver {
     maxAliasLength?: number;
 
     cteCapabilities: CteCapabilities = {
+        enabled: true,
         requiresRecursiveHint: true,
     };
 

src/driver/sqlserver/SqlServerDriver.ts

@@ -230,6 +230,7 @@ export class SqlServerDriver implements Driver {
     };
 
     cteCapabilities: CteCapabilities = {
+        enabled: true,
         // todo: enable it for SQL Server - it's partially supported, but there are issues with generation of non-standard OUTPUT clause
         writable: false,
     };

src/driver/types/CteCapabilities.ts

@@ -1,4 +1,10 @@
 export interface CteCapabilities {
+    /**
+     * Are CTEs supported at all?
+     */
+    enabled: boolean;
+
+
     /**
      * Are RETURNING clauses supported in CTEs?
      */

test/functional/query-builder/cte/helpers.ts

@@ -0,0 +1,6 @@
+import { Connection } from "../../../../src";
+import { CteCapabilities } from "../../../../src/driver/types/CteCapabilities";
+
+export function filterByCteCapabilities(capability: keyof CteCapabilities, equalsTo: boolean = true): (conn: Connection) => boolean {
+    return conn => conn.driver.cteCapabilities[capability] === equalsTo;
+}

test/functional/query-builder/cte/recursive-cte.ts

@@ -1,6 +1,7 @@
 import { expect } from "chai";
 import { Connection } from "../../../../src";
 import { closeTestingConnections, createTestingConnections, reloadTestingDatabases } from "../../../utils/test-utils";
+import { filterByCteCapabilities } from "./helpers";
 
 describe("query builder > cte > recursive", () => {
     let connections: Connection[];
@@ -19,21 +20,23 @@ describe("query builder > cte > recursive", () => {
     beforeEach(() => reloadTestingDatabases(connections));
     after(() => closeTestingConnections(connections));
 
-    it("should work with simple recursive query", () => Promise.all(connections.map(async connection => {
-        const qb = await connection
-            .createQueryBuilder()
-            .select([])
-            .from("cte", "cte")
-            .addCommonTableExpression(`
-                SELECT 1
-                UNION ALL
-                SELECT cte.foo + 1
-                FROM cte
-                WHERE cte.foo < 10
-            `, "cte", { recursive: true, columnNames: ["foo"] })
-            .addSelect("cte.foo", "foo")
-            .getRawMany<{foo: number}>();
+    it("should work with simple recursive query", () => Promise.all(connections
+        .filter(filterByCteCapabilities("enabled"))
+        .map(async connection => {
+            const qb = await connection
+                .createQueryBuilder()
+                .select([])
+                .from("cte", "cte")
+                .addCommonTableExpression(`
+                    SELECT 1
+                    UNION ALL
+                    SELECT cte.foo + 1
+                    FROM cte
+                    WHERE cte.foo < 10
+                `, "cte", { recursive: true, columnNames: ["foo"] })
+                .addSelect("cte.foo", "foo")
+                .getRawMany<{ foo: number }>();
 
-        expect(qb).to.have.length(10);
-    })));
+            expect(qb).to.have.length(10);
+        })));
 });