Take the 2-minute tour ×
Code Review Stack Exchange is a question and answer site for peer programmer code reviews. It's 100% free, no registration required.

I'm a student and I've created this MySQL database schema for my Service Order Management academic project. I want to know if I need to improve it and, if so, how I could do that.

/*
 * Table 'brands'
 */
CREATE TABLE `brands` (
    `brand_id` INTEGER NOT NULL,
    `equipment_id` INTEGER NOT NULL,
    `brand` VARCHAR(45) NOT NULL,
    CONSTRAINT `pk_brand_id` PRIMARY KEY (`brand_id`),
    CONSTRAINT `fk_equipment_id` FOREIGN KEY (`equipment_id`) REFERENCES `equipments`
);

/*
 * Table 'cities'
 */
CREATE TABLE `cities` (
    `city_id` INTEGER NOT NULL,
    `state_id` INTEGER NOT NULL,
    `city` VARCHAR(50) NOT NULL,
    CONSTRAINT `pk_city_id` PRIMARY KEY (`city_id`),
    CONSTRAINT `fk_state_id` FOREIGN KEY (`state_id`) REFERENCES `states`
);

/*
 * Table 'customers'
 */
CREATE TABLE `customers` (
    `customer_id` INTEGER NOT NULL,
    `user_id` INTEGER NOT NULL,
    CONSTRAINT `pk_customer_id` PRIMARY KEY (`customer_id`),
    CONSTRAINT `fk_user_id` FOREIGN KEY (`user_id`) REFERENCES `users`
);

/*
 * Table 'employees'
 */
CREATE TABLE `employees` (
    `employee_id` INTEGER NOT NULL,
    `user_id` INTEGER NOT NULL,
    `is_admin` INTEGER NOT NULL,
    CONSTRAINT `pk_employee_id` PRIMARY KEY (`employee_id`),
    CONSTRAINT `fk_user_id` FOREIGN KEY (`user_id`) REFERENCES `users`,
    CONSTRAINT `ck_is_admin` CHECK (`is_admin` IN (0, 1)) -- unsupported by MySQL
);

/*
 * Table 'equipments'
 */
CREATE TABLE `equipments` (
    `equipment_id` INTEGER NOT NULL,
    `equipment` VARCHAR(45) NOT NULL,
    CONSTRAINT `pk_equipment_id` PRIMARY KEY (`equipment_id`)
);

/*
 * Table 'legal_users'
 */
CREATE TABLE `legal_users` (
    `user_id` INTEGER NOT NULL,
    `cnpj` CHAR(14) NOT NULL,
    `company_name` VARCHAR(150) NOT NULL,
    `trade_name` VARCHAR(100) NULL,
    `contact_name` VARCHAR(50) NOT NULL,
    `mobile_phone` CHAR(10) NULL,
    `work_phone` CHAR(10) NULL,
    CONSTRAINT `pk_user_id` PRIMARY KEY (`user_id`),
    CONSTRAINT `fk_user_id` FOREIGN KEY (`user_id`) REFERENCES `users`,
    CONSTRAINT `uc_cnpj` UNIQUE (cnpj)
);

/*
 * Table 'natural_users'
 */
CREATE TABLE `natural_users` (
    `user_id` INTEGER NOT NULL,
    `cpf` CHAR(11) NOT NULL,
    `name` VARCHAR(50) NOT NULL,
    `nickname` VARCHAR(30) NULL,
    `sex` CHAR(1) NOT NULL,
    `birth_date` DATE NOT NULL,
    `home_phone` CHAR(10) NULL,
    `mobile_phone` CHAR(10) NULL,
    `work_phone` CHAR(10) NULL, -- ignored for employees
    CONSTRAINT `pk_user_id` PRIMARY KEY (`user_id`),
    CONSTRAINT `fk_user_id` FOREIGN KEY (`user_id`) REFERENCES `users`,
    CONSTRAINT `uc_cpf` UNIQUE (`cpf`),
    CONSTRAINT `ck_sex` CHECK (`sex` IN ('M', 'F')) -- unsupported by MySQL
);

/*
 * Table 'service_orders'
 */
CREATE TABLE `service_orders` (
    `order_id` INTEGER NOT NULL,
    `employee_id` INTEGER NOT NULL,
    `customer_id` INTEGER NOT NULL,
    `opening_date` TIMESTAMP NOT NULL,
    `status` INTEGER NOT NULL,
    `equipment_id` INTEGER NOT NULL,
    `brand_id` INTEGER NOT NULL,
    `model` VARCHAR(20) NOT NULL,
    `serial_number` VARCHAR(20) NULL,
    `under_warranty` INTEGER NOT NULL,
    `defect` TEXT NOT NULL,
    `diagnosis` TEXT NULL,
    `solution` TEXT NULL,
    `maintenance_cost` DECIMAL(6,2) NULL,
    `closing_date` TIMESTAMP NULL,
    `remarks` TEXT NULL,
    CONSTRAINT `pk_order_id` PRIMARY KEY (`order_id`),
    CONSTRAINT `fk_employee_id` FOREIGN KEY (`employee_id`) REFERENCES `employees`,
    CONSTRAINT `fk_customer_id` FOREIGN KEY (`customer_id`) REFERENCES `customers`,
    CONSTRAINT `ck_status` CHECK (`status` IN (0, 5)), -- unsupported by MySQL
    CONSTRAINT `fk_equipment_id` FOREIGN KEY (`equipment_id`) REFERENCES `equipments`,
    CONSTRAINT `fk_brand_id` FOREIGN KEY (`brand_id`) REFERENCES `brands`,
    CONSTRAINT `ck_under_warranty` CHECK (`under_warranty` IN (0, 1)) -- unsupported by MySQL
);

/*
 * Table 'states'
 */
CREATE TABLE `states` (
    `state_id` INTEGER NOT NULL,
    `state` VARCHAR(20) NOT NULL,
    `code` CHAR(2) NOT NULL,
    CONSTRAINT `pk_state_id` PRIMARY KEY (`state_id`),
    CONSTRAINT `uc_state` UNIQUE (`state`),
    CONSTRAINT `uc_code` UNIQUE (code)
);

/*
 * Table 'users'
 */
CREATE TABLE `users` (
    `user_id` INTEGER NOT NULL,
    `person_type` CHAR(1) NOT NULL,
    `postal_code` CHAR(8) NOT NULL,
    `address` VARCHAR(60) NOT NULL,
    `number` INTEGER NULL,
    `complement` VARCHAR(50) NULL,
    `neighborhood` VARCHAR(30) NOT NULL,
    `city_id` INTEGER NOT NULL,
    `email` VARCHAR(150) NOT NULL,
    `password` CHAR(40) NOT NULL,
    `remarks` TEXT NULL,
    `is_active` INTEGER NOT NULL DEFAULT 1,
    CONSTRAINT `pk_user_id` PRIMARY KEY (`user_id`),
    CONSTRAINT `ck_person_type` CHECK (`person_type` IN ('F', 'J')), -- unsupported by MySQL
    CONSTRAINT `fk_city_id` FOREIGN KEY (`city_id`) REFERENCES `cities`,
    CONSTRAINT `uc_email` UNIQUE (`email`),
    CONSTRAINT `ck_is_active` CHECK (`is_active` IN (0, 1)) -- unsupported by MySQL
);
share|improve this question
add comment

4 Answers 4

It looks fine. Two things to consider:

1, Distinct table for phone numbers:

phone_number
    id
    user_id
    type (home/work/mobile/etc)
    number

It may not worth it, depends on the data.

2, Maybe you want to store some audit information about your data. (Hints: journal table in Oracle, Hibernate Envers)

+1: CHAR(40) for password - it's SHA-1? If you use SHA-256/SHA-512 it will be too short.

share|improve this answer
add comment

You could set the integers that act as an ID to 'unsigned'. It doesn't really improve much, it only gives you a wider range of integer values.

share|improve this answer
add comment

Looks good. Two things to possibly consider, both in regards to self-referencing columns:

  1. Manager column if you ever wanted to establish a hierarchy using self-joins:

    CREATE TABLE `employees` (
        `employee_id` INTEGER NOT NULL,
        `user_id` INTEGER NOT NULL,
        `is_admin` INTEGER NOT NULL,
        `manager_employee_id` INTEGER NULL
    
  2. Kit column in case any of the equipments could be bundled into a kit, for example SELECT * FROM equipments WHERE kit_id = 5; would return all parts of a kit, which would itself be another record in the equipments table.

    CREATE TABLE `equipments` (
        `equipment_id` INTEGER NOT NULL,
        `equipment` VARCHAR(45) NOT NULL,
        `kit_id` INTEGER NULL
    
share|improve this answer
add comment

For states, this makes more sense. Adjust foreign keys to match.

CREATE TABLE `states` (
    `code` CHAR(2) NOT NULL,
    `state` VARCHAR(20) NOT NULL,
    CONSTRAINT `pk_states` PRIMARY KEY (`code`),
    CONSTRAINT `uc_state` UNIQUE (`state`)
);

Using an id number for a surrogate key only makes sense if

  • the thing you're modeling doesn't carry its identity along with it, or
  • its identity is impractically long, like varchar(250).

Both state code and state name are fully identifying. (Within the context of a single country.) Using the state code instead of an integer saves space and usually eliminates a join.

share|improve this answer
2  
this is extremely bad practice. Never ever use data that gets displayed anywhere as a primary key. The assumption that state codes don't change ignores that: States change, which might result in change of codes. What an enterprise stores in a state table often doesn't map to some standards body notion of state, so you might end up with duplicate codes, changing codes and all kinds of weird stuff happening to your codes. All this is pain full in any case, but it will kill you when your code is actually a primary key. –  Jens Schauder Nov 10 '12 at 13:00
    
No, it's not a bad practice. There's no relational principle that says candidate keys must be hidden from the user and immutable. In fact, it's obviously impossible to enforce that requirement on all candidate keys. (Above, "code" and "state" are both candidate keys.) There's no similar SQL restriction, either, unless you use Oracle. (Oracle is the only major dbms that doesn't support ON UPDATE CASCADE.) Both the relational model and SQL allow any column(s) having a UNIQUE constraint to be the target of foreign key references. –  Mike Sherrill 'Cat Recall' Nov 10 '12 at 13:05
    
Yes it is bad practice. I'm not saying that candidate key must be hidden. But Primary Keys should. Have fun with an ON UPDATE CASCADE once you have tables of considerable size referencing a mutating Primary Key. Also ON UPDATE CASCADE solves only one of the mentioned problems. Of course every single problem is solvable in some way. I'm just saying you should just avoid it in the first place when it is so simple. –  Jens Schauder Nov 10 '12 at 13:11
    
A modern dbms handles ON UPDATE CASCADE fine, even with multi-million row tables. (Except Oracle.) This isn't 1985. (Except for Oracle.) There's no theoretical backing for "primary key" at all now--all candidate keys are created equal. Again, this isn't 1985. –  Mike Sherrill 'Cat Recall' Nov 10 '12 at 15:32
    
So we should drop all other IDs as well? Last time I checked pretty much every database had a problem with updating every single row in possibly multiple large tables. –  Jens Schauder Nov 10 '12 at 15:47
show 1 more comment

Your Answer

 
discard

By posting your answer, you agree to the privacy policy and terms of service.

Not the answer you're looking for? Browse other questions tagged or ask your own question.