better postgres uuid column handling

[salzhrani]

Description of change

passing {type: "uuid"} as a parameter to PrimaryGeneratedColumn is valid given the TS types. but strategy is set to increment instead of uuid
this change fixes it. I think it would be worthwhile to tighten the parameters type definition instead of having a large union
Fixes #6039

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

[nebkat]

The type there actually refers only to the type of the column, not the generation strategy. If anything "uuid" should be removed as a valid option for type as it doesn't make sense to use it with the increment strategy. If we were to apply this change the strategy could be set to any of the values here:

typeorm/src/driver/types/ColumnTypes.ts

Lines 1 to 19 in af2c5c4

	/**
	* Column types used for @PrimaryGeneratedColumn() decorator.
	*/
	export type PrimaryGeneratedColumnType = "int" // mysql, mssql, oracle, sqlite, sap
	|"int2" // postgres, sqlite, cockroachdb
	|"int4" // postgres, cockroachdb
	|"int8" // postgres, sqlite, cockroachdb
	|"integer" // postgres, oracle, sqlite, mysql, cockroachdb, sap
	|"tinyint" // mysql, mssql, sqlite, sap
	|"smallint" // mysql, postgres, mssql, oracle, sqlite, cockroachdb, sap
	|"mediumint" // mysql, sqlite
	|"bigint" // mysql, postgres, mssql, sqlite, cockroachdb, sap
	|"dec" // oracle, mssql, sap
	|"decimal" // mysql, postgres, mssql, sqlite, sap
	|"smalldecimal" // sap
	|"fixed" // mysql
	|"numeric" // postgres, mssql, sqlite
	|"number" // oracle
	|"uuid"; // postgres

[salzhrani]

do you prefer to fix the parameter type definition? would that be a breaking change ?

[nebkat]

It is technically a breaking change but unless someone has an example of how @PrimaryGeneratedColumn("increment", {type: "uuid"}) works it looks like a bug and should be removed.

[salzhrani]

uuid is a valid column type in postgres.
how about we check for the cases where uuid is the first parameter and when it is passed as type with a different strategy. and maybe force having an object as a single parameter for the decorator with the next breaking change?

[nebkat]

uuid is a valid column type but how would an auto incrementing column work with it? If the values are sequential then it defeats the purpose of using a UUID.

The correct way to use the uuid is just @PrimaryGeneratedColumn("uuid"). The decorator assumes if the first parameter is an object that the user wanted an "increment" generated column, so it treats it the same as passing @PrimaryGeneratedColumn("increment", options).

[salzhrani]

Whats your recommendation on solving this issue?. just remove uuid from PrimaryGeneratedColumnType?

[salzhrani]

Any other changes would you like me to add?

[nebkat]

That's all, just squash your commits into one and make sure you follow the commit message format in CONTRIBUTING.md 🎉

[pleerock]

It took me quite a while to figure out this small single-line change.

I can't understand why we did this #4043 change (liked by 3 people) two years ago.

[nebkat]

This looks like an easy mistake to overlook, the type is PrimaryGeneratedColumnType rather than PrimaryGeneratedIncrementColumnType so it is confusing.