Skip to content

Add "create table" for MySQL #8

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.

Sign up for GitHub

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

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Add "create table" for MySQL #8

Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 33 additions & 3 deletions database.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,7 +45,6 @@ abstract class RegisterState {
class mxDatabase {

private $db = NULL;

/**
* Creates mxDatabase object
* @param config object which has following members:
Expand All @@ -72,7 +71,37 @@ function __construct($config) {
// create database file when not existent yet
$this->db = new PDO($db_input, $user, $password);
$this->db->setAttribute(PDO::ATTR_ERRMODE, PDO::ERRMODE_EXCEPTION);
$this->db->exec("CREATE TABLE IF NOT EXISTS registrations(
if (strpos($db_input, 'mysql:') !== false) {
$this->db->exec("CREATE TABLE IF NOT EXISTS `registrations` (
`id` int(10) UNSIGNED NOT NULL AUTO_INCREMENT,
`state` tinyint(4) NOT NULL DEFAULT '0',
`first_name` varchar(64) DEFAULT NULL,
`last_name` varchar(64) DEFAULT NULL,
`username` varchar(64) NOT NULL,
`password` varchar(64) NOT NULL,
`note` varchar(255) DEFAULT NULL,
`email` varchar(255) NOT NULL,
`verify_token` varchar(64) NOT NULL,
`admin_token` varchar(64) NOT NULL,
`request_date` timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP,
PRIMARY KEY (`id`),
UNIQUE KEY `username` (`username`)
) ENGINE=MyISAM DEFAULT CHARSET=utf8mb4");
Copy link
Owner

@krombel krombel Feb 24, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would not define an ENGINE here - or default to InnoDB as it is transaction-safe. Same below

Copy link
Author

@jasmir jasmir Feb 24, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't know if it is still true, but I learned some time that if you don't need transactions and/or foreign key support use MyISAM because it has the smaller disk-footprint, is faster in most cases (many more reads than writes) and has the better tools for optimizing. There I would prefer MyISAM if there is no need for transactions and/or foreign keys. If there is a need for it, there should InnoDB explicitly selected.

$this->db->exec("CREATE TABLE IF NOT EXISTS `logins` (
`id` int(10) UNSIGNED NOT NULL AUTO_INCREMENT,
`active` tinyint(4) NOT NULL DEFAULT '1',
`first_name` varchar(64) DEFAULT NULL,
`last_name` varchar(64) DEFAULT NULL,
`localpart` varchar(64) NOT NULL,
`password_hash` varchar(64) DEFAULT NULL,
`email` varchar(64) NOT NULL,
`create_date` timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP,
`last_modified` timestamp NOT NULL DEFAULT CURRENT_TIMESTAMP ON UPDATE CURRENT_TIMESTAMP,
PRIMARY KEY (`id`)
) ENGINE=MyISAM DEFAULT CHARSET=utf8mb4");
}
else {
$this->db->exec("CREATE TABLE IF NOT EXISTS registrations(
id SERIAL PRIMARY KEY,
state INT DEFAULT 0,
first_name TEXT,
Expand All @@ -85,7 +114,7 @@ function __construct($config) {
admin_token TEXT,
request_date TIMESTAMP DEFAULT CURRENT_TIMESTAMP
)");
$this->db->exec("CREATE TABLE IF NOT EXISTS logins (
$this->db->exec("CREATE TABLE IF NOT EXISTS logins (
Copy link
Owner

@krombel krombel Feb 24, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This looks like changed indenting. I try to always indent by 4 spaces. I just noticed that I am inconsistent with that as well but I would ask to at least restore the former indentation to reduce diff size

Copy link
Author

@jasmir jasmir Feb 24, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The "$this->db->exec" are a now substructure of the mysql-check (or the else-branch of this check). So, if I understand it correctly, they have to be indenting, but 4 spaces. Is this correct?

id SERIAL PRIMARY KEY,
active INT DEFAULT 1,
first_name TEXT,
Expand All @@ -96,6 +125,7 @@ function __construct($config) {
create_date TIMESTAMP DEFAULT CURRENT_TIMESTAMP,
last_modified TIMESTAMP DEFAULT CURRENT_TIMESTAMP
)");
}
// make sure the bot is allowed to login
if (!$this->userRegistered("register_bot")) {
$password = $this->addUser("Register", "Bot", "register_bot", NULL, $config["register_email"]);
Expand Down