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

Add Ord instance on schema, enables sorted table names in IDE #584

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

nghamilton
Copy link
Contributor

How do we feel about this?

@mpscholten
Copy link
Member

When reordering the full Schema.sql we'll have edge cases that causes errors, e.g. a enum type always has to be declared before it can be used inside a table DDL statement. The user could also be using a text editor to edit the Schema.sql and then the reorder could be a bit annoying.

But maybe we could sort the tables inside the render function of the table list of the GUI?

@nghamilton
Copy link
Contributor Author

I wondered if that might be the case; I am happy to change it. An other option is to write the Ord instance to order statements in a manner that is appropriate for the order of the SQL declarations, which might avoid other statement ordering issues in the future? (I have noticed that the generation of the foreign key constraints appears to not be deterministic; I believe the order of the type parameters can change depending on the order of the constraint statements).

@mpscholten
Copy link
Member

I have noticed that the generation of the foreign key constraints appears to not be deterministic; I believe the order of the type parameters can change depending on the order of the constraint statements

Very good point 👍

Let's give this a try with custom Ord instances. Order should be defined like this:

CREATE EXTENSION
CREATE TYPE
CREATE TABLE
ALTER TABLE .. -- FKs

Then there's still an open edge case: When the CREATE TABLE statement has a column with an inline foreign key constraint:

CREATE TABLE projects (
    id UUID,
    user_id UUID,
    PRIMARY KEY (id),
    CONSTRAINT fk_user_id FOREIGN KEY (user_id) REFERENCES users
)

Then this table needs to be placed after the users table.

To avoid this issue the schema designer always uses ALTER TABLE statements to place it's own constraints. But the user might have e.g. copied an existing db schema when migrating a legacy app to IHP.

@nghamilton
Copy link
Contributor Author

I've added the Ord instance for Statement, but this change still does not address (a) the situation of a user editing the schema manually and having schema order change underneath them, and (b) any inline foreign key constraints needing to be sorted after the table definition. Perhaps (b) could have an Ord instance of CreateTable to sort any in-line constraint tables to appear last? The IDE display of table names would be quirky under this scenario.

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

Successfully merging this pull request may close these issues.

2 participants