RobinBlomberg / kysely-codegen

Generate Kysely type definitions from your database.
MIT License
807 stars 73 forks source link

Keep column order #48

Closed wirekang closed 1 year ago

wirekang commented 1 year ago

It seems like kysely-codegen sort columns by alphabetically. Why? We have more then 50 columns, and the order is important for readability. Please consider to keep order from INFORMATION_SCHEMA.

wirekang commented 1 year ago

It seems like this line is the cause of problem. https://github.com/koskimas/kysely/blob/23f004d4ebf55352b6d024b3702843dee6b41bb7/src/dialect/mysql/mysql-introspector.ts#L48

wirekang commented 1 year ago

I found an issue https://github.com/koskimas/kysely/issues/146

wirekang commented 1 year ago

Close this issue and I will continue from https://github.com/koskimas/kysely/issues/146.

RobinBlomberg commented 1 year ago

Well found!

However, kysely-codegen also includes code that sorts the tables and columns (and enums I believe) alphabetically. Why? Two reasons:

  1. This makes the internal tests consistent—they won't fail due to insertion order or upserts, as long as the data is the same
  2. The best default for readability (if you don't have your own specific readability rules) is to order them alphabetically

With that said, it may be a good idea to keep the sorting opt-in or opt-out, so this may be reopened in the future.

wirekang commented 1 year ago

Note that orderBy('column_name') and orderBy('ordinal_position') are both consistent, so tests will never failed as long as the data is the same.

RobinBlomberg commented 1 year ago

Of course, if the ordering is part of the tests, they will never fail. My gripe was that the tests kept failing as I were writing and changing them, but as long as the ordering is stable, I guess this may not be a problem.

Regardless, I have not considered column order to matter by design, so I have not taken it into account, but since others may find importance in column order, I should probably make this configurable.