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

introspection.getTables sorts column array alphabetically instead of by ordinal position #146

Closed
jaylmiller opened this issue Aug 22, 2022 · 2 comments
Labels
enhancement New feature or request wontfix This will not be worked on

Comments

@jaylmiller
Copy link
Contributor

Is this intentional? It is a bit counter intuitive to me and makes the introspection feature slightly less useful

would be happy to do a PR if you think this change would make sense.

@koskimas
Copy link
Member

koskimas commented Aug 23, 2022

Why is it counter intuitive? And what is the ordinal position anyway in case of columns? And you know the order is intentional, since you've looked at the code. No need to be passive aggressive

@igalklebanov igalklebanov added enhancement New feature or request wontfix This will not be worked on labels Oct 11, 2022
@wirekang
Copy link
Contributor

This issue should be reopen. Columns have a clear order regardless of dialect.

Mysql

information_schema.columns.ordinal_position

Postgres

inforamtion_schema.columns.ordinal_position or pg_catalog.pg_attribute.attnum

Sqlite

pragma_table_info(table).cid

Suggestion

Reverting part of f6588f8 could solve this problem. However, since there is no criterion for ordering the tables, sorting the tables by name seems necessary.

This line
https://github.com/koskimas/kysely/blob/71507c4102ca1ae86fa18940c53310aa6e15ac1b/src/dialect/mysql/mysql-introspector.ts#L49

should be orderBy('ordinal_position')

This line
https://github.com/koskimas/kysely/blob/71507c4102ca1ae86fa18940c53310aa6e15ac1b/src/dialect/postgres/postgres-introspector.ts#L80

should be orderBy('a.attnum')

This line
https://github.com/koskimas/kysely/blob/71507c4102ca1ae86fa18940c53310aa6e15ac1b/src/dialect/sqlite/sqlite-introspector.ts#L85

should be orderBy('cid')

wirekang added a commit to wirekang/kysely that referenced this issue Dec 29, 2022
koskimas pushed a commit to wirekang/kysely that referenced this issue Dec 29, 2022
koskimas pushed a commit that referenced this issue Dec 29, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request wontfix This will not be worked on
Projects
None yet
Development

No branches or pull requests

4 participants