cortezaproject / corteza

Low-code platform
https://cortezaproject.org
Apache License 2.0
1.61k stars 371 forks source link

Migrations need to specify a primary key on table creation #104

Closed requaos closed 3 years ago

requaos commented 4 years ago

Describe the bug Unable to initialize a fresh DB when sql_require_primary_key is set in MySQL

To Reproduce Steps to reproduce the behavior: Create a fresh DB, set sql_require_primary_key and run the stack, allowing migrations to run.

Expected behavior Expected the migration scripts to be compatible with this MySQL best practice.

Additional context Currently using DigitalOcean for managed MySQL services:

MySQL databases containing tables without a primary key and which contain more than 5000 rows may experience replication issues. To prevent this, DigitalOcean now requires you to add a primary key for each new table you create in any managed MySQL database created after 8 April 2020. We strongly recommend that you also add primary keys in existing databases to avoid replication issues.

I would be happy to open a PR with adjusted migration scripts if you can link the source files and statik asset bundling script.

requaos commented 4 years ago

DEBUG   system.database.migrations      db/migrate.go:81        migration error         {"filename": "/20200508070000.actionlog.up.sql", "error": "exec query failed: Error 3750: Unable to create or change a table without a primary key, when the system variable 'sql_require_primary_key' is set. Add a primary key to the table or unset this variable to avoid this message. Note that tables without a primary key can cause performance problems in row-based replication, so please consult your DBA before changing this setting.", "errorVerbose": "Error 3750: Unable to create or change a table without a primary key, when the system variable 'sql_require_primary_key' is set. Add a primary key to the table or unset this variable to avoid this message. Note that tables without a primary key can cause performance problems in row-based replication, so please consult your DBA before changing this setting.\nexec query failed\ngithub.com/titpetric/factory.(*DB).Exec\n\t/drone/src/vendor/github.com/titpetric/factory/database.go:390\ngithub.com/cortezaproject/corteza-server/system/db.Migrate.func2.1\n\t/drone/src/system/db/migrate.go:80\ngithub.com/titpetric/factory.(*DB).Transaction\n\t/drone/src/vendor/github.com/titpetric/factory/database.go:252\ngithub.com/cortezaproject/corteza-server/system/db.Migrate.func2\n\t/drone/src/system/db/migrate.go:91\ngithub.com/cortezaproject/corteza-server/system/db.Migrate\n\t/drone/src/system/db/migrate.go:111\ngithub.com/cortezaproject/corteza-server/system.(*App).Upgrade\n\t/drone/src/system/app.go:54\ngithub.com/cortezaproject/corteza-server/pkg/app.RunUpgrade\n\t/drone/src/pkg/app/helpers.go:46\ngithub.com/cortezaproject/corteza-server/monolith.(*App).Upgrade\n\t/drone/src/monolith/app.go:69\ngithub.com/cortezaproject/corteza-server/pkg/app.RunUpgrade\n\t/drone/src/pkg/app/helpers.go:46\ngithub.com/cortezaproject/corteza-server/pkg/app.(*runner).Upgrade\n\t/drone/src/pkg/app/runner.go:155\ngithub.com/cortezaproject/corteza-server/pkg/app.(*runner).Activate\n\t/drone/src/pkg/app/runner.go:170\ngithub.com/cortezaproject/corteza-server/pkg/app.(*runner).Provision\n\t/drone/src/pkg/app/runner.go:187\ngithub.com/cortezaproject/corteza-server/pkg/app.(*runner).serve\n\t/drone/src/pkg/app/runner.go:236\ngithub.com/cortezaproject/corteza-server/pkg/app.(*runner).Run.func2\n\t/drone/src/pkg/app/runner.go:276\ngithub.com/cortezaproject/corteza-server/pkg/cli.ServeCommand.func1\n\t/drone/src/pkg/cli/commands.go:91\ngithub.com/spf13/cobra.(*Command).execute\n\t/drone/src/vendor/github.com/spf13/cobra/command.go:762\ngithub.com/spf13/cobra.(*Command).ExecuteC\n\t/drone/src/vendor/github.com/spf13/cobra/command.go:852\ngithub.com/spf13/cobra.(*Command).Execute\n\t/drone/src/vendor/github.com/spf13/cobra/command.go:800\ngithub.com/cortezaproject/corteza-server/pkg/app.(*runner).Run\n\t/drone/src/pkg/app/runner.go:321\ngithub.com/cortezaproject/corteza-server/pkg/app.Run\n\t/drone/src/pkg/app/runner.go:333\nmain.main\n\t/drone/src/cmd/monolith/main.go:16\nruntime.main\n\t/usr/local/go/src/runtime/proc.go:203\nruntime.goexit\n\t/usr/local/go/src/runtime/asm_amd64.s:1373"}
Error: exec query failed: Error 3750: Unable to create or change a table without a primary key, when the system variable 'sql_require_primary_key' is set. Add a primary key to the table or unset this variable to avoid this message. Note that tables without a primary key can cause performance problems in row-based replication, so please consult your DBA before changing this setting.
requaos commented 4 years ago

@darh, In this commit: Add db migration for actionlog Which key should be set as primary?

requaos commented 4 years ago

Also, since this migration is already committed, which is the best strategy for replacing this file, to include a primary key, but also adding the primary key for those that have migrated past this point without a primary key specified on this table?

requaos commented 4 years ago

@darh, I see that you have completely rewritten the database schema management system and have moved away from the migration files. However, I still see this will be an issue HERE in the 2020.9.x branch.

requaos commented 4 years ago

Workaround:

 CREATE TABLE sys_actionlog ( id bigint unsigned NOT NULL, ts datetime NOT NULL DEFAULT CURRENT_TIMESTAMP, actor_ip_addr varchar(15) NOT NULL, actor_id bigint unsigned DEFAULT NULL, requ                             est_origin varchar(32) NOT NULL, request_id varchar(64) NOT NULL, resource varchar(128) NOT NULL, action varchar(64) NOT NULL, error varchar(64) NOT NULL, severity smallint NOT NULL, descripti                             on text, meta json DEFAULT NULL, PRIMARY KEY (id), KEY ts (ts DESC), KEY request_origin (request_origin), KEY actor_id (actor_id), KEY resource (resource), KEY action (action) );

 UPDATE migrations SET status='ok' WHERE filename='/20200508070000.actionlog.up.sql';
darh commented 4 years ago

Wow @requaos thank you.

One of our users just warned me about this so I'll make sure this is properly addressed in 2020.9.x branch! For now, I would just stay with this manual fix.

darh commented 3 years ago

We've reimplemented db schema migrations. This should not be an issue anymore.