feat: support for Common Table Expressions

[Ginden]

Resolves #1116 #5149 #5484

Description of change

This pull request adds support for common table expressions in QueryBuilder.

Pull-Request Checklist

• Code is up-to-date with the master branch
• npm run lint passes with this change
• npm run test passes with this change
• This pull request links relevant issues as Fixes #0000
• There are new or updated unit tests validating the change
• Documentation has been updated to reflect this change
• The new commits follow conventions explained in CONTRIBUTING.md

[pleerock]

Why does it marked for 0.3.0? If there are no breaking changes we can introduce it in 0.2.x.

[Ginden]

Why does it marked for 0.3.0? If there are no breaking changes we can introduce it in 0.2.x.

I marked it as 0.3.0 because original issue #1116 was marked as 0.3.0.

I can't see any breaking changes.

[pleerock]

ah okay, then we can unmark it and release in 0.2.x. Also we probably need to specify in the documentation what databases support this feature. And do you have in plans add other databases support?

[Ginden]

Also we probably need to specify in the documentation what databases support this feature. And do you have in plans add other databases support?

Common table expressions are supported in all major databases... But MySQL supports them since 8.0, but TypeORM tests against MySQL 5.7. MySQL 5.7 goes EOL in 2023.

DML in CTE is supported only in Postgres I think, I updated docs.

[pleerock]

Common table expressions are supported in all major databases... But MySQL supports them since 8.0, but TypeORM tests against MySQL 5.7. MySQL 5.7 goes EOL in 2023.

we can:

• drop mysql 5.7 from tests
  or
• add another mysql 8.0 to test, add check in the test to skip test execution in 5.7

[Ginden]

@pleerock Overall, I think it's reasonable to provide some kind of version check into Driver. Many features are not supported in older versions of databases and users should get early error instead of syntax errors.

[pleerock]

yes, would be good to have.

[AlexMesser]

@Ginden we already have a version checks like this, but used internally in QueryRunner. We can make getVersion() function part of the QueryRunner public API. We can also add a version property to all drivers and set it before the schema is synchronized.

[Ginden]

I will try to have a look at this. Maybe it makes sense to do simple

connection.query(`WITH cte_test AS (SELECT 1 AS bar) SELECT * FROM cte_test`);

instead of hard-coding checks.

Or disable support for CTEs by default for certain (only MySQL?) database drivers and specify forceEnableCte in MySqlConnectionOptions?

@pleerock Which approach should I choose?

@AlexMesser Linked check made me think that TypeORM should explicitly say which database versions it supports. I think support for EOL databases should be explicitly dropped.

[pleerock]

@Ginden why can't we go with version checks as @AlexMesser suggested?

[Ginden]

@pleerock Done!

[pleerock]

Tests are failing...

[Ginden]

Hm, these tests fail only if MySQL 8.0 is enabled in tests. I added mysql8 to docker-compose.yml (with forced mysql2 driver due to this unresolved issue in mysql package).

After disabling mysql8, tests pass locally.

I will have a look at this, whether it's issue with TypeORM or mysql2 or MySQL 8.0.

EDIT:

• I was able to pinpoint one issue - legacySpatialSupport wasn't disabled for MySQL 8.0.
• tinyint don't have width on MySQL 8.0

[Ginden]

Full list of issues with enabling MySQL 8.0 in tests:

• legacySpatialSupport wasn't disabled for MySQL 8.0.
• tinyint don't have width on MySQL 8.0
• geometrycollection is returned as geomcollection (GeomCollection() and GeometryCollection() are synonymous, with GeomCollection() the preferred function.

Also:

1) ConnectionOptionsReader
   properly loads config from yaml:
 Error: EEXIST: file already exists, mkdir '/typeorm/build/compiled/test/functional/connection-options-reader/configs/yaml'


2) database schema > column width
   all types should be created with correct width:
 AssertionError: expected undefined to equal 10
  at /typeorm/test/functional/database-schema/column-width/mysql/column-width.ts:25:61

3) database schema > column width
   should update data type display width:
 AssertionError: expected undefined to equal 5
  at /typeorm/test/functional/database-schema/column-width/mysql/column-width.ts:48:61

4) entity-schema > columns > mysql
   should create columns with different options:

  AssertionError: expected 'concat(`FirstName`,_utf8mb4\\\' \\\',`LastName`)' to equal 'concat(`FirstName`,\' \',`LastName`)'
  + expected - actual

  -concat(`FirstName`,_utf8mb4\' \',`LastName`)
  +concat(`FirstName`,' ',`LastName`)
  
  at /typeorm/test/functional/entity-schema/columns/mysql/columns-mysql.ts:26:77

5) transaction > method wrapped into transaction decorator
   should execute all operations in the method in a transaction:
 AssertionError: expected undefined not to be undefined
  at /typeorm/test/functional/transaction/transaction-decorator/transaction-decorator.ts:35:37

6) transaction > method wrapped into transaction decorator
   should inject repository and custom repository into method decorated with @Transaction:
 AssertionError: expected undefined not to be undefined
  at /typeorm/test/functional/transaction/transaction-decorator/transaction-decorator.ts:135:37

7) transaction > method wrapped into transaction decorator
   should execute all operations in the method in a transaction with a specified isolation:
 AssertionError: expected undefined not to be undefined
  at /typeorm/test/functional/transaction/transaction-decorator/transaction-decorator.ts:156:37

8) github issues > #1377 Add support for `GENERATED ALWAYS AS` in MySQL
   should correctly create table with generated columns:

  AssertionError: expected 'concat(`firstName`,_utf8mb4\\\' \\\',`lastName`)' to equal 'concat(`firstName`,\' \',`lastName`)'
  + expected - actual

  -concat(`firstName`,_utf8mb4\' \',`lastName`)
  +concat(`firstName`,' ',`lastName`)
  
  at /typeorm/test/github-issues/1377/issue-1377.ts:22:77

9) github issues > #3702 MySQL Spatial Type Support : GeomFromText function is not supported
   when legacySpatialSupport: true
     should use GeomFromText:
 AssertionError: expected 'INSERT INTO `letter_box`(`id`, `coord`) VALUES (DEFAULT, ST_GeomFromText(?, 4326))' to not include 'ST_GeomFromText'
  at /typeorm/test/github-issues/3702/issue-3702.ts:31:29

10) github issues > #3702 MySQL Spatial Type Support : GeomFromText function is not supported
   when legacySpatialSupport: true
     should use AsText:
 AssertionError: expected 'SELECT `letterBox`.`id` AS `letterBox_id`, ST_AsText(`letterBox`.`coord`) AS `letterBox_coord` FROM `letter_box` `letterBox`' to not include 'ST_AsText'
  at /typeorm/test/github-issues/3702/issue-3702.ts:53:29

11) github issues > #6266 Many identical selects after insert bunch of items
   should execute a single SELECT to get inserted default and generated values of multiple entities:
 TypeError: Attempted to wrap select which is already wrapped
  at checkWrappedMethod (node_modules/sinon/lib/sinon/util/core/wrap-method.js:40:21)
  at wrapMethod (node_modules/sinon/lib/sinon/util/core/wrap-method.js:88:13)
  at Function.spy (node_modules/sinon/lib/sinon/spy.js:156:16)
  at Sandbox.spy (node_modules/sinon/lib/sinon/sandbox.js:328:35)
  at /typeorm/test/github-issues/6266/issue-6266.ts:43:41
  at step (node_modules/tslib/tslib.js:143:27)
  at Object.next (node_modules/tslib/tslib.js:124:57)
  at /typeorm/node_modules/tslib/tslib.js:117:75
  at new Promise (<anonymous>)
  at __awaiter (node_modules/tslib/tslib.js:113:16)
  at /typeorm/test/github-issues/6266/issue-6266.ts:42:46
  at Array.map (<anonymous>)
  at Context.<anonymous> (test/github-issues/6266/issue-6266.ts:42:25)
  at runMicrotasks (<anonymous>)
  at processTicksAndRejections (node:internal/process/task_queues:96:5)
--------------
Error: Stack Trace for original
  at wrapMethod (node_modules/sinon/lib/sinon/util/core/wrap-method.js:116:26)
  at Function.spy (node_modules/sinon/lib/sinon/spy.js:156:16)
  at Sandbox.spy (node_modules/sinon/lib/sinon/sandbox.js:328:35)
  at /typeorm/test/github-issues/6266/issue-6266.ts:43:41
  at step (node_modules/tslib/tslib.js:143:27)
  at Object.next (node_modules/tslib/tslib.js:124:57)
  at /typeorm/node_modules/tslib/tslib.js:117:75
  at new Promise (<anonymous>)
  at __awaiter (node_modules/tslib/tslib.js:113:16)
  at /typeorm/test/github-issues/6266/issue-6266.ts:42:46
  at Array.map (<anonymous>)
  at Context.<anonymous> (test/github-issues/6266/issue-6266.ts:42:25)
  at runMicrotasks (<anonymous>)
  at processTicksAndRejections (node:internal/process/task_queues:96:5)

I created #8713 to track these.

[Ginden]

I'm not sure why Postgres 12 test fails with:

1. github issues > @CavidM @pleerock This seems to cause an error for connection configs with replication. Please note the lack of the database property on the root object. #4753 MySQL Replication Config broken

Oracle needs fixing though.

[Ginden]

@pleerock I need assistance with this one - tests pass for "query builder > cte", but they fail when attempting to test MySQL replication.

Tests pass locally on my machine connected to Docker, so I suppose it's something related to CircleCI specific configuration.

[pleerock]

oups it got automatically closed

[pleerock]

replace it with InstanceChecker.isSelectQueryBuilder. From 0.3.0 we avoid using instanceof because it doesn't play well in the monorepo projects with multiple instances of TypeORM across different node_modules.

[Ginden]

Done.

[pleerock]

looks good 👍

[pleerock]

thank you a lot