fix: fixed all known enum issues #7419

AlexMesser · 2021-02-24T14:57:15Z

Description of change

enum issues:

Fixes #2233
Fixes #4833
Fixes #4350
Fixes #4897
Fixes #5000
Fixes #5275
Fixes #5371
Fixes #5478
Fixes #5648
Fixes #5729
Fixes #5871
Fixes #6047
Fixes #6115
Fixes #6376
Fixes #6471
Fixes #6772
Fixes #7217
Fixes #7283

related issues:

Fixes #5882

covered pull-requests:

Closes #4005
Closes #5276
Closes #5479
Closes #6126
Closes #7221

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
[N/A] Documentation has been updated to reflect this change
The new commits follow conventions explained in CONTRIBUTING.md

fix: `enumName` changes not handled; fix: `enumName` does not handle table schema;

nebkat · 2021-02-26T04:45:23Z

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

@@ -534,6 +534,10 @@ export class AuroraDataApiDriver implements Driver {
    normalizeDefault(columnMetadata: ColumnMetadata): string | undefined {
        const defaultValue = columnMetadata.default;

+        if (defaultValue === null) {
+            return undefined


Should we maybe be differentiating here between null and undefined? When the user sets the column default explicitly to null perhaps they would want the column generated to also explicitly specify DEFAULT NULL, even if that is probably unnecessary.

It is also already dealt with in places like this anyway:

typeorm/src/driver/mysql/MysqlQueryRunner.ts

Lines 1877 to 1878 in 2fa6231

if (column.default !== undefined && column.default !== null)

c += ` DEFAULT ${column.default}`;

@nebkat thank you for review!

I've checked MySQL and Postgres databases and they are storing both null and undefined defaults like (NULL) value, therefore there is no difference on database level between null and undefined.

So, we can't differentiate these values during synchronization process that leads to unnecessary update queries.

This problem was also brought up somewhat in #7229 (comment) but now we have TypeScript 4.2 with support for the ?? operator. Maybe we could start using ?? null when comparing all the fields to avoid these situations?

Now that I think about it more it might actually make sense to have such a mapping in normalizeDefault but I think it should be if (defaultValue === undefined) return null rather than vice-versa. It's a small change but I think it might save some headaches in a few months when someone else is looking at it: it makes more sense that we are saying "if the column default wasn't defined, we default to null" rather than "if the column was explicitly set to null we change it back to not being defined (and then assume that elsewhere the code will treat undefined as null)".

updated `postgres-enum` test;

AlexMesser · 2021-02-26T12:54:20Z

src/driver/postgres/PostgresDriver.ts

+                    // replace double escaped backslash to single escaped e.g. \\\\ -> \\
+                    val = val.replace(/(\\\\)/g, "\\")
+                    // replace escaped double quotes to non-escaped e.g. \"asd\" -> "asd"
+                    return val.replace(/(\\")/g, '"')


I am not so good in regex and here can be more elegant solution

nebkat · 2021-02-26T16:29:15Z

src/driver/postgres/PostgresDriver.ts

+                    // replace double quotes from the beginning and from the end
+                    val = val.replace(/(^")|("$)/g, "")
+                    // replace double escaped backslash to single escaped e.g. \\\\ -> \\
+                    val = val.replace(/(\\\\)/g, "\\")
+                    // replace escaped double quotes to non-escaped e.g. \"asd\" -> "asd"
+                    return val.replace(/(\\")/g, '"')


Could be faster? Probably slightly easier to read. Safer for the start/end quotes because it won't fix a broken "string with missing start/end quote.

Suggested change

// replace double quotes from the beginning and from the end

val = val.replace(/(^")|("$)/g, "")

// replace double escaped backslash to single escaped e.g. \\\\ -> \\

val = val.replace(/(\\\\)/g, "\\")

// replace escaped double quotes to non-escaped e.g. \"asd\" -> "asd"

return val.replace(/(\\")/g, '"')

// replace double quotes from the beginning and from the end

if (val.startsWith(`"`) && val.endsWith(`"`)) val = val.slice(1, -1);

// replace double escaped backslash to single escaped e.g. \\ -> \

val = val.replaceAll("\\\\", "\\");

// replace escaped double quotes to non-escaped e.g. \"asd\" -> "asd"

return val.replaceAll(`\\"`, `"`);

carlocorradini · 2021-03-05T13:12:33Z

@AlexMesser When this changes will be available?

Thanks a lot, I was waiting this fixes for quite some time. 🤯🥳🥳🥳

* docs: fix small typo on package.json script example (typeorm#7408) Add missing colon in JSON property at `package.json` `"script"` example * feat: output Javascript Migrations instead of TypeScript (typeorm#7294) * docs / test: Added tests and documentation for Feature 7253 - Migrations Javascript output * Change in the test * test: Re-arranged the tests to move them to the core tests directory * tests: Adjusted Tests a bit * tests - renamed tests to follow the other functional tests naming * tests - renamed tests to follow the other functional tests naming * tests - Fixed issues with the test connections setup * tests - Removed unnecesary restore * fix: improve EntityManager.save() return type (typeorm#7391) This brings it in line with the equivalent method in Repository. * fix: resolve issue building tree entities with embeded primary column (typeorm#7416) Closes: typeorm#7415 * Adjust mongodb driver options & connect driver to support replica set (typeorm#7402) - Dupplicate buildDriverOptions for mongodb especially - Add hostReplicaSet to MongoConnectionOptions properties for collect host replica list - Adjust buildConnectionUrl to build replica set connection url * fix: performance issues of `RelationId`. (typeorm#7318) * test: relationId is too slow * perf: RelationId is too slow When we join a lot of relations we can get 1k+ records from db even it is only 10 entities. Then when relationId are loaded the query contains too many duplciates for the same ids. The solution is to check that the relationId query fetches only unique values. Closes: typeorm#5691 * fix: Array type default value should not generate SQL commands without change (typeorm#7409) * fix(1532) Array type default value should not generate SQL commands without change * Update PostgresDriver.ts * removed `arrayCast` from `normalizeDefault` since casting for default value is already removed in `PostgresQueryRunner.loadTables()` method; * removed support for function definition in `default` because function syntax suppose to support raw sql, we don't have to confuse things by applying custom modifications. * Update User.ts removed incorrect `default` definition with functions Co-authored-by: AlexMesser <dmzt08@gmail.com> * feat: add check and dryrun to migration generate (typeorm#7275) Adds support for “check” and “drynrun” modes to the migration generate command. Fixes typeorm#3037 Refs typeorm#6978 * chore: typescript version upgrade (typeorm#7422) * chore: dependencies update (typeorm#7424) * typescript version upgrade * fixing linting * fixing mongo query runner issues * fixing linting * updated all dependencies * fixes typeorm#7418 * fixes typeorm#7418 * adding missing ILike operator docs (took from next branch) * fix: mongodb connectionurl parse options (#1) * fix: mongodb connectionurl parse options - Loop every options in mongodb connection url and turn it as object to merge with connection url object before return of method "parseMongoDBConnectionUrl" - unit test of mongodb replicaset parse connectionurl of typeorm#7401 - unit test of mongodb options parse connectionurl of typeorm#7437 * fix: add semicolon by lint suggestion /home/circleci/typeorm/src/driver/DriverUtils.ts 192:39 error Missing semicolon @typescript-eslint/semi * chore: @beamdev package scope (#2) * chore: update master (typeorm#3) * fix: fixed all known enum issues (typeorm#7419) * fix typeorm#5371 * fix typeorm#6471; fix: `enumName` changes not handled; fix: `enumName` does not handle table schema; * fixed falling test; * added test for typeorm#7217 * fix typeorm#6047, typeorm#7283; * fix typeorm#5871 * added support for `enumName` in `joinColumns` (typeorm#5729) * fix typeorm#5478 * fixed falling test; updated `postgres-enum` test; * added column `array` property change detection (typeorm#5882); updated `postgres-enum` test; * fix typeorm#5275 * added validation for `enum` property (typeorm#2233) * fix typeorm#5648 * improved missing "enum" or "enumName" properties validation; * fix typeorm#4897, typeorm#6376 * lint fix; * fixed falling tests; * fixed falling tests; * removed .only * fix typeorm#6115 * refactor: improve README.md and DEVLOPER.md code examples formatting (typeorm#7436) * fix: correctly get referenceColumn value in `getEntityValueMap` (typeorm#7005) * test: add test case (typeorm#7002) * fix: correctly get referenceColumn value in `getEntityValueMap` * test: reproduction for issue typeorm#3246 (typeorm#3247) * Add reproduction for issue 3246 * Update test/github-issues/3246/issue-3246.ts Co-authored-by: Json Choi <1890mah@gmail.com> Co-authored-by: Dan Imbrogno <dan.imbrogno@gmail.com> Co-authored-by: AlexMesser <dmzt08@gmail.com> Co-authored-by: Json Choi <1890mah@gmail.com> * code refactoring in test; * added test for typeorm#2758 * feat: allow to pass the given table name as string in RelationDecorators (typeorm#7448) * feat(RelationDecorators): allow to pass the given table name as string * Update EntityMetadataBuilder.ts added parentheses; Co-authored-by: Emily Marigold Klassen <forivall@users.noreply.github.com> * feat: add option for installing package using CLI (typeorm#6889) * init cli: add options for installing package * yarg choice, add await, revert formatter changes * init flag - set default to npm Co-authored-by: AlexMesser <dmzt08@gmail.com> Co-authored-by: Henry Boisdequin <boisdequinhenry19@gmail.com> Co-authored-by: Json Choi <1890mah@gmail.com> Co-authored-by: Dan Imbrogno <41128441+danimbrogno-pml@users.noreply.github.com> Co-authored-by: Dan Imbrogno <dan.imbrogno@gmail.com> Co-authored-by: Emily Marigold Klassen <forivall@gmail.com> Co-authored-by: Emily Marigold Klassen <forivall@users.noreply.github.com> Co-authored-by: Gaurav Sharma <gtpan77@gmail.com> * chore: update master typeorm/typeorm (typeorm#5) * fix: fixed all known enum issues (typeorm#7419) * fix typeorm#5371 * fix typeorm#6471; fix: `enumName` changes not handled; fix: `enumName` does not handle table schema; * fixed falling test; * added test for typeorm#7217 * fix typeorm#6047, typeorm#7283; * fix typeorm#5871 * added support for `enumName` in `joinColumns` (typeorm#5729) * fix typeorm#5478 * fixed falling test; updated `postgres-enum` test; * added column `array` property change detection (typeorm#5882); updated `postgres-enum` test; * fix typeorm#5275 * added validation for `enum` property (typeorm#2233) * fix typeorm#5648 * improved missing "enum" or "enumName" properties validation; * fix typeorm#4897, typeorm#6376 * lint fix; * fixed falling tests; * fixed falling tests; * removed .only * fix typeorm#6115 * refactor: improve README.md and DEVLOPER.md code examples formatting (typeorm#7436) * fix: correctly get referenceColumn value in `getEntityValueMap` (typeorm#7005) * test: add test case (typeorm#7002) * fix: correctly get referenceColumn value in `getEntityValueMap` * test: reproduction for issue typeorm#3246 (typeorm#3247) * Add reproduction for issue 3246 * Update test/github-issues/3246/issue-3246.ts Co-authored-by: Json Choi <1890mah@gmail.com> Co-authored-by: Dan Imbrogno <dan.imbrogno@gmail.com> Co-authored-by: AlexMesser <dmzt08@gmail.com> Co-authored-by: Json Choi <1890mah@gmail.com> * code refactoring in test; * added test for typeorm#2758 * feat: allow to pass the given table name as string in RelationDecorators (typeorm#7448) * feat(RelationDecorators): allow to pass the given table name as string * Update EntityMetadataBuilder.ts added parentheses; Co-authored-by: Emily Marigold Klassen <forivall@users.noreply.github.com> * feat: add option for installing package using CLI (typeorm#6889) * init cli: add options for installing package * yarg choice, add await, revert formatter changes * init flag - set default to npm Co-authored-by: AlexMesser <dmzt08@gmail.com> Co-authored-by: Henry Boisdequin <boisdequinhenry19@gmail.com> Co-authored-by: Json Choi <1890mah@gmail.com> Co-authored-by: Dan Imbrogno <41128441+danimbrogno-pml@users.noreply.github.com> Co-authored-by: Dan Imbrogno <dan.imbrogno@gmail.com> Co-authored-by: Emily Marigold Klassen <forivall@gmail.com> Co-authored-by: Emily Marigold Klassen <forivall@users.noreply.github.com> Co-authored-by: Gaurav Sharma <gtpan77@gmail.com> Co-authored-by: rccursach <rccursach@gmail.com> Co-authored-by: Jorge Luis Vargas <Jorge.Vargas@albelli.com> Co-authored-by: Anthony Rosequist <arosequist@users.noreply.github.com> Co-authored-by: Tomas Zaluckij <mrtomaszal@gmail.com> Co-authored-by: MG <m@sudo.eu> Co-authored-by: Ed Mitchell <edeesis@gmail.com> Co-authored-by: AlexMesser <dmzt08@gmail.com> Co-authored-by: Christian Holm <cho@cubitech.dk> Co-authored-by: Umed Khudoiberdiev <pleerock.me@gmail.com> Co-authored-by: Henry Boisdequin <boisdequinhenry19@gmail.com> Co-authored-by: Json Choi <1890mah@gmail.com> Co-authored-by: Dan Imbrogno <41128441+danimbrogno-pml@users.noreply.github.com> Co-authored-by: Dan Imbrogno <dan.imbrogno@gmail.com> Co-authored-by: Emily Marigold Klassen <forivall@gmail.com> Co-authored-by: Emily Marigold Klassen <forivall@users.noreply.github.com> Co-authored-by: Gaurav Sharma <gtpan77@gmail.com>

AlexMesser · 2021-03-31T10:41:19Z

@carlocorradini these changes are now available in version 0.2.32

duvanmonsa · 2021-05-12T22:47:06Z

@AlexMesser I still see the issue when using enumName the migration always re-creates the column . Note: using 0.2.32

AlexMesser · 2021-05-16T12:45:16Z

@duvanmonsa #7661 hurrying to the rescue

AsyncLegs · 2022-11-10T15:38:37Z

Hello folks!
I really appreciate your work with those issues!
Do you have any plans to merge these fixes into 0.3.* version somehow?
Cuz 0.3.10 still has some issues with enum create type duplications

AlexMesser added 2 commits February 24, 2021 19:52

fix #5371

5fe13b1

fix #6471;

734bd2b

fix: `enumName` changes not handled; fix: `enumName` does not handle table schema;

AlexMesser changed the title ~~fix: bunch of enum issues~~ [WIP] bunch of enum issues Feb 25, 2021

AlexMesser added 6 commits February 25, 2021 15:52

fixed falling test;

48f1740

added test for #7217

3c5f946

fix #6047, #7283;

009fd39

fix #5871

cc46ecd

added support for enumName in joinColumns (#5729)

6796563

fix #5478

4613f31

nebkat reviewed Feb 26, 2021

View reviewed changes

AlexMesser added 3 commits February 26, 2021 15:08

fixed falling test;

94ab29d

updated `postgres-enum` test;

added column array property change detection (#5882);

bcdc33d

updated `postgres-enum` test;

fix #5275

798763d

AlexMesser commented Feb 26, 2021

View reviewed changes

AlexMesser added 3 commits February 26, 2021 20:15

added validation for enum property (#2233)

2c2def2

fix #5648

a481012

improved missing "enum" or "enumName" properties validation;

be6faa2

nebkat reviewed Feb 26, 2021

View reviewed changes

AlexMesser added 6 commits February 26, 2021 22:20

fix #4897, #6376

db03ca2

lint fix;

b1e6b74

fixed falling tests;

b1666ef

fixed falling tests;

ccfad1e

removed .only

621732f

fix #6115

233b7ad

AlexMesser changed the title ~~[WIP] bunch of enum issues~~ fix: fixed all known enum issues Mar 5, 2021

AlexMesser merged commit 724d80b into master Mar 5, 2021

AlexMesser deleted the enum-fixes branch March 5, 2021 12:24

AlexMesser mentioned this pull request Mar 27, 2021

Enum with enumName in two tables fails on second synchronize #7501

Closed

21 tasks

AlexMesser mentioned this pull request Apr 1, 2021

Do not create duplicate CREATE TYPE migration query when same 'enumName's are exists #7523

Open

21 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

fix: fixed all known enum issues #7419

fix: fixed all known enum issues #7419

AlexMesser commented Feb 24, 2021 •

edited

Loading

nebkat Feb 26, 2021

AlexMesser Feb 26, 2021 •

edited

Loading

nebkat Feb 26, 2021

AlexMesser Feb 26, 2021

nebkat Feb 26, 2021

carlocorradini commented Mar 5, 2021

AlexMesser commented Mar 31, 2021

duvanmonsa commented May 12, 2021 •

edited

Loading

AlexMesser commented May 16, 2021

AsyncLegs commented Nov 10, 2022 •

edited

Loading

	if (column.default !== undefined && column.default !== null)
	c += ` DEFAULT ${column.default}`;

fix: fixed all known enum issues #7419

fix: fixed all known enum issues #7419

Conversation

AlexMesser commented Feb 24, 2021 • edited Loading

Description of change

enum issues:

related issues:

covered pull-requests:

Pull-Request Checklist

nebkat Feb 26, 2021

Choose a reason for hiding this comment

AlexMesser Feb 26, 2021 • edited Loading

Choose a reason for hiding this comment

nebkat Feb 26, 2021

Choose a reason for hiding this comment

AlexMesser Feb 26, 2021

Choose a reason for hiding this comment

nebkat Feb 26, 2021

Choose a reason for hiding this comment

carlocorradini commented Mar 5, 2021

AlexMesser commented Mar 31, 2021

duvanmonsa commented May 12, 2021 • edited Loading

AlexMesser commented May 16, 2021

AsyncLegs commented Nov 10, 2022 • edited Loading

AlexMesser commented Feb 24, 2021 •

edited

Loading

AlexMesser Feb 26, 2021 •

edited

Loading

duvanmonsa commented May 12, 2021 •

edited

Loading

AsyncLegs commented Nov 10, 2022 •

edited

Loading