SQL query creating tables needed by a simple PHP CMS

Question

I have written recently the SQL query to MariaDB that I want to move into PHP code. Below is my test.sql file. What do you think about my SQL code?

CREATE TABLE IF NOT EXISTS user_roles (
    id int(11) NOT NULL auto_increment,
    name VARCHAR(255) NOT NULL default '',
    PRIMARY KEY (id)
);

CREATE TABLE IF NOT EXISTS users (
    id int(11) NOT NULL auto_increment,
    username varchar(255) NOT NULL default '',
    passwd varchar(255) NOT NULL default '',
    email varchar(255) NOT NULL default '',
    created_at datetime NOT NULL,
    role_id int(11) NOT NULL default 0,
    PRIMARY KEY (id),
    FOREIGN KEY (role_id) REFERENCES user_roles (id)
);

CREATE TABLE IF NOT EXISTS posts (
    id int(11) NOT NULL auto_increment,
    title varchar(255) NOT NULL default '',
    content text NOT NULL,
    author_id int(11) NOT NULL default 0,
    created_at datetime NOT NULL,
    updated_at datetime NOT NULL,
    PRIMARY KEY (id),
    FOREIGN KEY (author_id) REFERENCES users (id)
)

Just a note: A user can have zero or one user_role. Typically, a user would have 0..N roles, implemented with a jump table. That's not a problem in your SQL though, just something with the overall design. BTW: Don't write SQL in PHP. Use a query builder (e.g. cakephp/database), use a migration tool (e.g. robmorgan/phinx) or even a full-blows ORM. — uli
– uli, Commented Jan 16, 2024 at 21:16
In the context of my application user must have only one role — whiteman808
– whiteman808, Commented Jan 18, 2024 at 5:07
Please edit your question so that the title describes the purpose of the code, rather than its mechanism. We really need to understand the motivational context to give good reviews. It's best to describe what value this code provides to its user. — Toby Speight
– Toby Speight, Commented Jan 18, 2024 at 16:32

J_H · Accepted Answer · 2024-01-13 17:56:15Z

It mostly looks nice.

There are lots of NOT NULL constraints, which is refreshing to see and which I feel is a Good Thing.

let things default

The repeated use of int(11) is visually distracting. Prefer to spell it INT, or INTEGER. It will mean the same thing.

plural collection

There are two ways to open a boiled egg: at the little end or at the big end.

There are two types of noun we could name a table with: singular and plural.

Reasonable people will disagree. Me? I fall in the "singular!" camp, which in my experience is more prevalent in production systems. Certainly one can find examples of both.

CREATE TABLE IF NOT EXISTS user_roles (

I recommend naming it the user_role table, and similarly for the post table.

Now, there is a certain bug-a-boo that tends to steer folks down a particular path. Think of the DDL ALTER USER .... We wouldn't want to create a user table and run afoul of that reserved keyword, nor get into quoting nonsense. So in some production systems the users table is the plural exception to the usual singular rule, or we create a foo_user or bar_user table. But in your system I argue (below) there's a better name for it.

primary key

    id int(11) NOT NULL auto_increment,

Each and every relation (table) should have a PK. If it lacks one, then don't bother creating the table, just throw those rows into an ordinary text file. So I always look for it, typically up on that initial line.

You later mention PRIMARY KEY (id), which is equivalent in every way, good job. As a convenience to the reader, I have a preference for putting it up top, after the NOT NULL (unless we have a compound PK). No biggie.

defaulting an FK

CREATE TABLE IF NOT EXISTS users (
    ...
    role_id int(11) NOT NULL default 0,

The NOT NULL is very nice.

I am skeptical that defaulting to 0 is helpful. It imposes a documentation burden on the user role table, explaining that 0 is a "special" role, something that might be disturbed if we e.g. delete then re-create that role.

If someone tries to INSERT and neglects to specify the role ID, I would much rather see the transaction fail with a helpful diagnostic. That tends to make any source code which interacts with the table quickly step into line with expectations.

hashed credentials

    passwd varchar(255) NOT NULL default '',

Letting username or password silently default to empty string is deeply upsetting. I cannot imagine a production system where either of those behaviors would be desirable. Maybe you could find a developer who favors that? But their tune would change once they're meeting with their manager and the Information Security Officer, and the topic of "secure by default" arises.

As a separate matter, I don't see "hash" or similar term in that passwd column name. You should be applying the argon2id hash of user's cleartext password, and storing that. This mitigates several attacks, including downstream exposure to other sites after attacker successfully downloads SELECT * FROM users; rows.

I note in passing that you don't track UPDATE times. So if someone rings up Help Desk with trouble about their password working, or email going to the right place, you won't have as much diagnostic information to help explain the situation as you might have.

simple conventional FK reference

CREATE TABLE IF NOT EXISTS posts (
    ...
    title varchar(255) NOT NULL default '',

I am skeptical that silently defaulting to empty string is something your Product Manager would approve of, but this one is far less significant than the matter of defaulting credential fields.

    author_id int(11) NOT NULL default 0,

Again, we seem to be suggesting that author zero sits at the Editor's desk or in some other way is special. So there would be a documentation burden on that table. Recommend you not default this.

    FOREIGN KEY (author_id) REFERENCES users (id)

Software engineering offers many, many opportunities to be creative. But in lots of situations we actually prefer to be dirt simple boring, doing things the conventional way, with conventional spelling. It makes maintenance easier when someone revisits this a few months down the road.

Here, I was expecting to read

    FOREIGN KEY (author_id) REFERENCES author (id)

Which suggests renaming a particular relation so we will CREATE TABLE author.

This DBA support script achieves many of its design goals.

I would be willing to delegate or accept maintenance tasks on it.

A good review and I have nothing substantial to add. One thing though, you object int(11), but remained silent on VARCHAR(255), both of which are kind-of like Cargo Cult Programming to me. What is your opinion on that? — uli
– uli, Commented Jan 16, 2024 at 21:00
@uli, yeah, I agree they’re not the ideal way to reveal Author’s intent. I actually did squint a bit at the 255 during review, but neglected to mention it when writing it up. Imposing a length restriction probably wasn’t the principle intent, so just writing VARCHAR, or better TEXT, would have been nicer. I disagree about “cargo cult”, though. I believe this is just copy-n-paste from SHOW CREATE TABLE output. Good starting point, and then I prefer to refine it. — J_H
– J_H, Commented Jan 16, 2024 at 21:22
Let me be specific about the Cargo Cult part: 255 is the largest value for an 8-bit unsigned integer or 2 to the power of 8 minus one (NUL byte?). Using sizes that are somehow related to powers of two for e.g. buffers like here is Cargo Cult Programming. — uli
– uli, Commented Jan 17, 2024 at 8:21

Stack Exchange Network

SQL query creating tables needed by a simple PHP CMS

1 Answer 1

let things default

plural collection

primary key

defaulting an FK

hashed credentials

simple conventional FK reference

You must log in to answer this question.

Hot Network Questions

SQL query creating tables needed by a simple PHP CMS

1 Answer 1

let things default

plural collection

primary key

defaulting an FK

hashed credentials

simple conventional FK reference

You must log in to answer this question.

Related

Hot Network Questions