search

k3s-io / kine

Run Kubernetes on MySQL, Postgres, sqlite, dqlite, not etcd.
Apache License 2.0
1.65k stars 237 forks source link

Feat: Skips ddl stmt upon setting up the database #64

Closed yue9944882 closed 1 year ago

yue9944882 commented 3 years ago

our production environment doesn't allow the application executes DDL statement at application runtime, the CREATE/ALTER statements are required to be submitted/reviewed in other places. this pull allows developers to opt-out from the DDL commands

brandond commented 3 years ago

This is an interesting request, I will have to talk it over with the team.

I would suggest that we only support this for postgres/mysql databases - so drop the changes to the sqlite driver. I do not think it is reasonable to have someone externally manage the DDL for an embedded database engine.

Also, if we're not going to attempt to create the tables and indexes on startup, I think we need some fallback code to ensure their existence.

yue9944882 commented 3 years ago

Also, if we're not going to attempt to create the tables and indexes on startup, I think we need some fallback code to ensure their existence.

thanks, checking the table existence SGTM, plz let me know what's your team thought. will update as follow-up if that makes sense..

ibuildthecloud commented 3 years ago

This idea was that all the statements should be "create if not exists" allowing them to be created beforehand. What I don't think I thought about is that you will be rejected because you don't have to create privileges even if the object exists and you don't need to create it.

If we can just check first if something exist before creating it, that's would be preferred.

brandond commented 3 years ago

@ibuildthecloud I thought that the 'create table if not exists' that we're doing would be safe enough but I think you're right that these would probably result in permissions errors even if the table exists. For one of the engines we aren't doing if-not-exists; we're just running the create and ignoring 'already exists' errors.

Explicitly checking for each table and index before creating it seems like a fair bit more code to maintain, but ultimately I think it'll be easier to support than having a flag to completely externalize DDL maintenance - since we really want to validate the existing schema if we're not blindly ensuring that it exists.

yue9944882 commented 3 years ago

@brandond @ibuildthecloud i updated the pull to probe the existence of schema before executing create table statements, this will be more reasonable than simply providing a skip option.

dereknola commented 1 year ago

Is this still relevant?