What's going on with the database design?

fiahblade

First off, the project is amazing and I have lots and lots of respect towards all the people who are contributing and are making this possible. So don't shoot me down for simply raising my concerns.

Can nobody agree with me that the database design is a complete mess? This might be done on purpose but why?

No primary/foreign keys, no normalisation, same data stored in multiple tables.

Kulga

fiahblade No primary/foreign keys, no normalisation, same data stored in multiple tables.

Just to help make the issue clearer, can you provide examples of the "messy" design and ideas for how a more efficient design would be? IE, how would you implement fixing this?
Maybe include what would be improved with a more efficient design? Typical users aren't going to care but a extension developer may..

JoshyPHP

You should probably describe the changes you would like to see in concrete terms if you want a constructive discussion.

fiahblade

I'm writing down all my findings, this might take a while. Please bear with me.

fiahblade

@Kulga I think it's beter if i post some examples in another thread, 'Dev' probably. Normalisation is probably the main thing to keep in mind.

I tried to motivate and keep everything as understandable as possible. Ok here we go.

Users

discussions_count / comments_count

The table users looks good, until I saw the columns "discussions_count" and "comments_count", values that can be calculated/looked up shouldn't be stored. Don't be afraid of COUNT(), its performance might surprise you.

preferences

“preferences” is a BLOB type and holds all the users preferences in a json format. If you want to read or edit one of those preferences, the server will need to load this data into the ram, find the correct preference, edit it (or do something else with it) and put it back. Seems like a lot of trouble.

Creating a new table e.g. user_preferences with ‘user_id’ as a key is a great alternative.

normalisation of user_preferences

Getting or updating a field this way is just as easy as executing a simple SELECT, no further serialization needed, here you only come in contact with the data you actually need.

{"notify_discussionRenamed_alert":true,"notify_postLiked_alert":true,"notify_discussionLocked_alert":true,"notify_postMentioned_alert":true,"notify_postMentioned_email":false,"notify_userMentioned_alert":true,"notify_userMentioned_email":false,"notify_newPost_alert":true,"notify_newPost_email":true,"followAfterReply":false,"discloseOnline":true,"indexProfile":true}

↑ This blob of text of 381 bytes becomes 12 bytes (note: per user):
(12 booleans, tinyint takes up 1 byte hence 12 bytes)

We are just talking about a few bytes here, the savings are indeed negligible. But look at it from a performance point of view. If you want to lookup 1 preference, well now you’ll just come in contact with that 1 byte.

notifications_read_time

I’m not sure but I assume “notifications_read_time” has to do with the notification centre. If it does, might using cookies render this column not useless? Then again I’m not sure about this one.
NOTE: This might also be the case with “flags_read_time”, but this is an extension.

read_time

For my own curiosity, what does “read_time” do? The naming is quite vague within the current table.

posts

type

This is a red flag. Duplicate data, it breaks the rules of normalisation. Beter to define the types in a new table and point to it with an id.

discussions

discussions_count / participants_count

Same as before. Don't be afraid of COUNT(), its performance might surprise you.

discussions (in general)

Looking at the big picture, there is something not completely right here. I can see the choice that was made to completely separate every post including ‘the original discussion message’. But the amount of unnecessary information that is stored is, well weird.

All this can be looked up, why is this stored again?
discussions unnecessary information

~Cheers

Toby

fiahblade Don't be afraid of COUNT(), its performance might surprise you.

Please show me evidence that COUNT() is just as fast as reading a cached value on InnoDB tables, where the read:write ratio is significantly more towards reading.

fiahblade preferences

We're aware that the current JSON storage format of preferences is not ideal and have plans to remove it in the next version.

fiahblade notifications_read_time

This value is tied to each user account (not to a single browser as cookies are) and is essential for the notifications system to function properly. Same with flags_read_time.

fiahblade read_time

read_time stores the time that a user last marked all discussions as read. We're also aware that some of the column names could be improved and are planning to implement some more intuitive naming conventions in the next version.

fiahblade type

I'll think about this but it's unlikely to change. Sometimes in the real world (because there are many other factors involved) we can't have perfect normalisation unfortunately.

fiahblade All this can be looked up, why is this stored again?

Again, performance.

fiahblade

Toby Didn't realise that COUNT() is that slow, maybe one day ^.

BillPennock

fiahblade I don't believe your wrong. I have been doing this for 30 years and I have seen only a very few systems with calculated data that is consistent over a long period of time. It's just hard to do. Properly normalized and designed databases don't typically have a need for this sort of thing. I have not insight into the particular database you are discussing and I'm liable to take shots for that but not recognizing that an event is an entity can cause this sort of "requirement" that's not really one. There are way to many REALLY BIG DATABASES that are highly normalized that work very fast. Now if this is unstructured data that's very different.

JoshyPHP

I don't speak for Toby but generally the answer to "why is this complicated db thing not simpler" is "because it'd be slower" and the answer to "why is this simple db thing not faster" is "because it'd be more complicated". So most of the time, decisions related to database things are a compromise between simplicity and performance.

Removing the last_time column would be catastrophic. The queries' performance would drop so badly that it would be unusable on anything but the smallest boards. Imagine this kind of query used to retrieve the 10 most recent discussions:

   SELECT d.*, COUNT(p.id) as comments, MAX(p.time) AS last_time
     FROM discussions d
LEFT JOIN posts p ON ...
 GROUP BY d.id
 ORDER BY last_time DESC
    LIMIT 10

Most databases (read: MySQL) would end up reading both tables in their entirety every time.

Creating a new table e.g. user_preferences with ‘user_id’ as a key is a great alternative.

The other side of that coin is that extensions would have to change the database schema to add preferences. Then it becomes tempting to normalize further and have a preferences (pref_id, pref_name) table and a user_preferences (user_id, pref_id, pref_value) table. The issue with that schema is that you have to read the entire preferences table on every page and then you have to retrieve a bunch of rows from user_preferences for every user you instantiate. At that point, one may want to cache the result of that query inside the users table, which would take us full circle. You get a normalized table, good perf when you use the serialized/cached preferences but you had to pay for it with increased code complexity. Sometimes it makes sense.

JoshyPHP

BillPennock There are way to many REALLY BIG DATABASES that are highly normalized that work very fast.

The issue is that the hardware Flarum runs on bears no resemblance to those. People want to run Flarum on small VPS that don't have a ton of RAM and not that many IOPS. As soon as the size of the indexes exceeds the size of the available memory, the performance takes a hit.