Support for Drupal 7 is ending on 5 January 2025—it’s time to migrate to Drupal 10! Learn about the many benefits of Drupal 10 and find migration tools in our resource center.
Hi,
I'm no DB expert by any means, but I've noticed a few inconsistencies with the current schema (in MySQL):
- From what I've read, the aim is to be as ANSI compliant as possible, implying that there should not be any use of AUTO_INCREMENT at all.
- Fixing (removing) the AUTO_INCREMENTs breaks Drupal as a number of modules (watchdog, locale) seem to rely on it (rather than using db_next_id(), which is not good.
- The sequences table indicates this quite plainly as well. My current table has:
aggregator_category_cid
comments_cid
files_fid
menu_mid
node_nid
node_revisions_vid
term_data_tid
users_uid
vocabulary_vidIDs from watchdog, access_log, url_aliases etc. are all missing..
- There is a lot of inconsistency with the use of UNSIGNED fields. For e.g. {user}_uid is set to be UNSIGNED. But when used as a FK elsewhere, it is not set to be UNSIGNED.. While, in real world situations we won't see an issue until we hit the record no. 2^31, it is nevertheless an error. And it seems to be happening with smaller types as well..
- Naming conventions: There are occurences of duplicate field names that can be very confusing. For e.g. there is {vocabulary}_vid which indicates the vocabulary id, and then there's {forum}_vid which indicates a "version id"... There are multiple instances of such duplicates..
I'm sure a lot of people have wasted a lot of their time because of these issues. The sooner they are fixed, the better. I'm happy to start rolling out patches where I can, but I'd appreciate some feedback on this before I dig in.
Thanks
Karthik.
Comment | File | Size | Author |
---|---|---|---|
#12 | unsigned_update.sql | 2.56 KB | Zen |
Comments
Comment #1
Zen CreditAttribution: Zen commentedThe following queries remove all auto_increments and add in UNSIGNED attributes where necessary:
Updating the DB leads to the first of possibly many AUTO_INCREMENT related errors:
Typical example from locale.inc of code that relies on the AUTO_INCREMENT attribute:
Cheers
Karthik.
Comment #2
Zen CreditAttribution: Zen commentedJust added two patches to the locale and watchdog modules [as above]..
Thanks
Karthik.
Comment #3
killes@www.drop.org CreditAttribution: killes@www.drop.org commentedWhile we try to be ANSI compliant in the queries, the database definition files are permitted to use db specific features where possible. auto_increment is actually much faster than using db_next_id to get a new unique ID, so it has its uses especially where we do not need to know the ID in advance (watchdig, accesslog, etc).
Comment #4
Zen CreditAttribution: Zen commentedI agree it saves a query, but I don't like these on/off levels of compliance.. IMO Drupal should either be compliant or not. A black and white rule like this will ensure that all the queries are more portable, and ease some of the work for other maintainers - pgSQL etc.
Most contribs abuse this quite regularly. Also, this locale module patch is an example in core where the ID is retrieved with a second query. Such inconsistencies will stop with a hard-and-fast rule.
I would really just like to see some consistency :)
Karthik.
Comment #5
killes@www.drop.org CreditAttribution: killes@www.drop.org commentedI personally prefer pure, raw speed of page execution over anything else including consistency for consistency's sake. In any case the individual use cases of auto_increment need to be evaluated individually.
Comment #6
scroogie CreditAttribution: scroogie commentedThese issues could be handled per database if drupal had a sensible database abstraction. The mySQL abstraction could use auto_increment, and the pgSQL could use sequences, like in PEAR::DB, etc.
Comment #7
killes@www.drop.org CreditAttribution: killes@www.drop.org commentedscroogie: we do that since one or two years... even without "sensible database abstraction"
Is today global idiot day?
Comment #8
chx CreditAttribution: chx commentedKilles: apparently.
Comment #9
Zen CreditAttribution: Zen commentedI don't think it's a very good sign when senior developers on an open source project start calling contributors and/or people who voice slightly negative opinions.. "Idiots"..
-K
Comment #10
scroogie CreditAttribution: scroogie commentedI dont know whats wrong with the term "sensible database abstraction" (im german, maybe i hit the wrong words), anyways, it was not meant to be provocative.
Back to the topic, i dont think drupal does it as i mentioned. I thought Drupal used sequences in any case (at least thats what db_next_id() in database.mysql.inc seems to do). Whatever, please dont take this as an attack, because it was not meant to be one.
Comment #11
killes@www.drop.org CreditAttribution: killes@www.drop.org commented" if drupal had a sensible database abstraction" ~ "wenn Drupal eine vernünftige DB-Abstraktion hätte." Was wir momentan haben ist durchaus vernünftig weil klein und nicht übermäßig kompliziert.
Comment #12
Zen CreditAttribution: Zen commentedSince we are at loggerheads over the use of AUTO_INCREMENT, the attachment fixes all the UNSIGNED inconsistencies, while preserving the existing AUTO_INCREMENTs. I'll prepare a patch for updates.inc if needed.
And don't get me wrong, I have no problems with using AUTO_INCREMENTs. However, if it is fine to use them, then there is absolutely *no* apparent reason for using the sequences table. The whole point as far as I can tell of using the sequences table is to completely *avoid* the use of AUTO_INCREMENT. The current situation is neither here nor there.
If raw speed is the preference, then AUTO_INCREMENTs should be used everywhere. This saves you atleast three queries every time [The {sequences} LOCK, REPLACE and UNLOCK..].
This just doesn't make sense to me..
Regards,
Karthik.
Comment #13
Geary CreditAttribution: Geary commentedSurely nobody meant to say that, did they? Good, I didn't think so.
+1 on AUTO_INCREMENT (pun intended) if it would eliminate the LOCK TABLES. LOCK TABLES is a big hassle in many hosting situations. It's been argued that hosting companies should be more willing to allow it, but it remains a real nuisance.
Removing LOCK TABLES would greatly benefit Drupal's ease of installation. I have personally lost many hours to this one issue. Even if it were database specific it would be well worthwhile: the hosts where it's a problem are running MySQL.
Comment #14
killes@www.drop.org CreditAttribution: killes@www.drop.org commentedI think the unsigned part shoudl be made into a patch.
Comment #15
Cvbge CreditAttribution: Cvbge commented+1 on AUTO_INCREMENT (pun intended) if it would eliminate the LOCK TABLES. LOCK TABLES is a big hassle in many hosting situations. It's been argued that hosting companies should be more willing to allow it, but it remains a real nuisance.
LOCK TABLES was introduced to guard against a situation, where two concurrent connections try to insert the the same values. It has nothing to do with AUTO_INCREMENT.
Looking from PostgreSQL point of view I think it'd be possibile to use AUTO_INCREMENT/SERIAL column types (would have to investigate to be sure)
Comment #16
Zen CreditAttribution: Zen commentedI've added the UNSIGNED patch as a separate issue. Modified topic accordingly.
-K
Comment #17
knite CreditAttribution: knite commentedI would love to see a patch that changed everything to use AUTO_INCREMENT. Like Geary, I've lost countless hours in installation headaches related to the LOCK TABLES privilege problem. The bonus in improved performance sounds great.
Karthik, would it be quick for you to whip up a patch that replaces the LOCKs with AUTO_INCREMENT everywhere? I'd volunteer to help test it.
Comment #18
moshe weitzman CreditAttribution: moshe weitzman commentedrenaming acording to the current consensus and focus of this issue
Comment #19
kbahey CreditAttribution: kbahey commented+1 on using AUTO_INCREMENT.
This, combined with LAST_INSERT_ID(), eliminates the need for the locking we do today.
However, see this discussion in the development mailing list about db_next_id() and auto increments.
Seems safe to do unless someone uses MySQL pconnect.
Other databases has similar mechanism, so this does not preclude them. In Fact, the PostgreSQL layer, uses "SELECT nextval() ...".
Comment #20
chx CreditAttribution: chx commentedhttp://drupal.org/node/55516