Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Keep column order #48

Closed
wirekang opened this issue Dec 14, 2022 · 6 comments
Closed

Keep column order #48

wirekang opened this issue Dec 14, 2022 · 6 comments

Comments

@wirekang
Copy link

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
Copy link
Author

@wirekang
Copy link
Author

I found an issue kysely-org/kysely#146

@wirekang
Copy link
Author

Close this issue and I will continue from kysely-org/kysely#146.

@RobinBlomberg
Copy link
Owner

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
Copy link
Author

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
Copy link
Owner

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants