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

upgrade.php and create.sql different #681

Open
shawniverson opened this issue Mar 22, 2017 · 13 comments
Open

upgrade.php and create.sql different #681

shawniverson opened this issue Mar 22, 2017 · 13 comments
Milestone

Comments

@shawniverson
Copy link
Member

It appears that the initial setup of the databases with create.sql are not the same as those resulting from running upgrade.php.

Should create.sql be brought up to the same level as upgrade.php?

create.sql

CREATE TABLE IF NOT EXISTS `users` (
  `username` varchar(191) COLLATE utf8_unicode_ci NOT NULL DEFAULT '',
  `password` varchar(255) COLLATE utf8_unicode_ci DEFAULT NULL,
  `fullname` varchar(255) COLLATE utf8_unicode_ci NOT NULL DEFAULT '',
  `type` enum('A','D','U','R','H') COLLATE utf8_unicode_ci DEFAULT NULL,
  `quarantine_report` tinyint(1) DEFAULT '0',
  `spamscore` float DEFAULT '0',
  `highspamscore` float DEFAULT '0',
  `noscan` tinyint(1) DEFAULT '0',
  `quarantine_rcpt` varchar(60) COLLATE utf8_unicode_ci DEFAULT NULL,
  `resetid` varchar(32) COLLATE utf8_unicode_ci DEFAULT NULL,
  `resetexpire` bigint(20) COLLATE utf8_unicode_ci DEFAULT NULL,
  `lastreset` bigint(20) COLLATE utf8_unicode_ci DEFAULT NULL,
  PRIMARY KEY (`username`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8 COLLATE=utf8_unicode_ci;

upgrade.php result

| users | CREATE TABLE `users` (
  `username` varchar(191) COLLATE utf8mb4_unicode_ci NOT NULL DEFAULT '',
  `password` varchar(255) COLLATE utf8mb4_unicode_ci DEFAULT NULL,
  `fullname` varchar(255) COLLATE utf8mb4_unicode_ci NOT NULL DEFAULT '',
  `type` enum('A','D','U','R','H') COLLATE utf8mb4_unicode_ci DEFAULT NULL,
  `quarantine_report` tinyint(1) DEFAULT '0',
  `spamscore` float DEFAULT '0',
  `highspamscore` float DEFAULT '0',
  `noscan` tinyint(1) DEFAULT '0',
  `quarantine_rcpt` varchar(60) COLLATE utf8mb4_unicode_ci DEFAULT NULL,
  `resetid` varchar(32) COLLATE utf8mb4_unicode_ci DEFAULT NULL,
  `resetexpire` bigint(20) DEFAULT NULL,
  `lastreset` bigint(20) DEFAULT NULL,
  PRIMARY KEY (`username`)
) ENGINE=InnoDB DEFAULT CHARSET=utf8mb4 COLLATE=utf8mb4_unicode_ci |
@endelwar
Copy link
Member

create.sql is more compatible with older mysql installation with no utf8mb4 support.
upgrade.php enhance the charset and collation on capable mysql systems.
In install procedure there should be a step to run upgrade.php after create.sql

@shawniverson
Copy link
Member Author

Ah, I see. That wasn't clear to me. Makes a lot more sense now.

@Skywalker-11
Copy link
Member

I think its not realy intuitive that you have to run upgrade.php on a fresh install to be able to use utf8mb4.
At least we should mention it in the docs

@remkolodder
Copy link

I agree with @Skywalker-11 . If you do an initial install, it should be the best experience you can get. Everyone expects to run a 'creation' tool so that the proper database structure is created. You would not expect to instantly upgrade (that suggests upgrade.php) the structure to get some additional features. If the database supports utf8mb4, then we should create those tables. Else it should fallback to regular utf8.

@shawniverson shawniverson reopened this Mar 22, 2017
@stefaweb
Copy link

A suggestion.

You can replace create.sql by a small install_database.php script.

You test if utf8mb4 is possible and load the right database version.
SQL schema included in the php. No external file.

@remkolodder
Copy link

Or via a general install.php file which writes conf.php and installs the db etc.

@stefaweb
Copy link

Or via a general install.php file which writes conf.php and installs the db etc.

@Skywalker-11 is working on this but its not yet ready.

https://github.com/mailwatch/mailwatch-install-script

@Skywalker-11
Copy link
Member

We need some people to test the script on different OS. Especially sendmail as MTA is still missing. Heres a list what we have to do : mailwatch/mailwatch-install-script#2

I only have experience with debian/ubuntu and postfix 😞

@stefaweb
Copy link

And this one?

@shawniverson
Copy link
Member Author

I think this one should remain open for further consideration. I think the create.sql and upgrade.php logic could be combined into a unified database preparation script. Not really necessary right now, but perhaps down the road.

@endelwar endelwar added this to the Post-1.2.0 milestone Jun 13, 2017
@asuweb
Copy link
Member

asuweb commented Jun 13, 2017

The simple fix is for upgrade.php to run create.sql if the database doesn't exist or is empty.

@stefaweb
Copy link

stefaweb commented Jul 5, 2017

Does this still active or can be closed ?

@Skywalker-11
Copy link
Member

Skywalker-11 commented Jul 5, 2017

Still not done

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

6 participants