drawdb-io / drawdb

Free, simple, and intuitive online database diagram editor and SQL generator.
https://drawdb.app
GNU Affero General Public License v3.0
22.7k stars 1.58k forks source link

feat: add databases switcher in control panel header #257

Open belyas opened 1 month ago

belyas commented 1 month ago

This feature adds a database switcher on the top left beside the chosen database to give current user ability to switch the databases easily as we had such situation and we had to redo everything from scratch just the chosen database was not the right one. Here how it looks like when it's open: Screenshot 2024-10-10 104824

Here are example of different outputs based on chosen database: Screenshot 2024-10-10 104614 Screenshot 2024-10-10 104738

vercel[bot] commented 1 month ago

@belyas is attempting to deploy a commit to the dottle's projects Team on Vercel.

A member of the Team first needs to authorize it.

1ilit commented 1 month ago

Since non-generic diagrams each use data types supported by their corresponding database engines, just changing database won't cut it. In order for the output sql to be correct we need to convert the types. For example, if you have an enum column in mysql switching to postgres will result in an incorrect output

belyas commented 1 month ago

Since non-generic diagrams each use data types supported by their corresponding database engines, just changing database won't cut it. In order for the output sql to be correct we need to convert the types. For example, if you have an enum column in mysql switching to postgres will result in an incorrect output

Hi @1ilit, so the underlining export to sql won't properly do its work? because these changes have nothing to do with what the final sql would look like, I thought this use case would be as generic case, when the export to sql was clicked with the chosen database, it will trigger proper toDatabase function? what's your take on this, please?

For enum use case, I see that the current implementation doesn't properly convert enums, I got this sql for Mysql:

CREATE TABLE "table_1" (
    "id" INTEGER NOT NULL UNIQUE GENERATED BY DEFAULT AS IDENTITY,
    "response" ENUM DEFAULT yes,
    PRIMARY KEY("id")
);

It's exactly what I get when I created a fresh diagram and set same enum. I think we can go on with this change as a feature, then we fix the underlining functions that export enums improperly, I can help with this

belyas commented 1 month ago

Actually the Enum fix for Mysql is done on #252

belyas commented 1 month ago

@1ilit can we merge this please, if you don't have any comments on the implementation? as this change will boost the UX. And regarding the underlining specific database engines' implementations have nothing to do with this feature and be done gradually in future PRs

1ilit commented 1 month ago

@belyas sorry but this wont be merged anytime soon, it doesn't even pass the workflows. Regarding the enum issue in mysql there's already pr for it.

The issue isn't only about enums in mysql and postgres. There will be a need to provide conversions from all engines to all engines.

Feel free to deploy your fork and use that instead.

belyas commented 1 month ago

for the workflows isn't an issue, I can fix the eslint error, but I didn't get why this won't be merged as it improves the UX and ease choosing the db engines, I also don't see how this work has to do with conversions from/to engines, this has nothing to do with that, or am I wrong and I got things differently? I'd like to have a proper reply @1ilit

belyas commented 1 month ago

@1ilit I have added conversions to db engines: PostreSQL, MySQL and Sqlite and restricted the db switcher to display only these engines for now, I can add others if this PR can be merged afterwards. Let me know what you think now

Screenshot 2024-10-13 141425 Screenshot 2024-10-13 141513