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.The SQLite driver doesn't respond well to compound primary keys, if one of the columns is a serial. Example from a module:
'fields' => array(
'id' => array(
'type' => 'serial',
'not null' => TRUE,
'description' => 'Entity id',
),
'path' => array(
'type' => 'varchar',
'length' => 255,
'not null' => TRUE,
'description' => 'The path of the page to apply meta tags to',
),
'lang' => array(
'type' => 'varchar',
'length' => 8,
'not null' => TRUE,
'description' => 'Language code',
),
),
'primary key' => array('id', 'path', 'lang'),
While the usefulness of the above primary key is up for discussion, it is valid, and will work fine in MySOL. However, when SQLite generates the CREATE TABLE, it adds PRIMARY KEY to the field definition in createFieldSql, and removes it from the set. After that, if the primary key set is not empty, it adds a PRIMARY KEY definition to the CREATE TABLE, using the left over columns, causing SQLite to complain 'PDOException: SQLSTATE[HY000]: General error: 1 table "metatags_quick_path_based" has more than one primary key'.
While it's possible to work around, it does break the consistency of the Database API.
I discovered this problem with metatags_quick 2.4, but a quick search tells me that other modules have run into the same bug.
| Comment | File | Size | Author |
|---|---|---|---|
| #5 | 1571842_workaround.patch | 787 bytes | dpovshed |
Comments
Comment #1
dpovshed CreditAttribution: dpovshed commentedConfirmed - the problem exists.
The metatags_quick is a good example to reproduce and debug.
Comment #2
Damien Tournoud CreditAttribution: Damien Tournoud commentedThis table definition doesn't make any sense.
Compound primary keys containing a serial columns have no purpose, because the serial part is already unique. I suggest you fix the definition of the table.
Comment #3
Xen CreditAttribution: Xen commentedI know it doesn't make any sense, the issue is that its a non-sensiality that's not uncommon in contrib.
The problem is that it works fine on MySQL, which means that those not knowing better doesn't find out for a long time.
If we define it as non-sensial, and wont support it for SQLite, why do we implicitly support it for MySQL/Postgres? If it's a common error, shouldn't the abstraction layer catch this and throw an error regardless of the database driver? That's exactly the reason for the existence of the database API, to provide a uniform interface, regardless of differences and bugs in the underlying systems.
Comment #4
Damien Tournoud CreditAttribution: Damien Tournoud commentedWe don't do any error checking in the schema API right now. I'm not saying we shouldn't be, it is just the way it is right now. For Drupal 7, you would be better off opening issues in the affected modules to raise awareness among the module maintainers.
Let's move to Drupal 8 for discussion on how to improve the situation.
Comment #5
dpovshed CreditAttribution: dpovshed commented@Xen, in case you're (or someone else) not in the mood to patch all the modules with suspicious definitions attached is the workaround patch.
It fix definition on the fly and allows all of such modules to be installed correctly.
Comment #6
dpovshed CreditAttribution: dpovshed commentedSorry guys, restoring headers
Comment #19
smustgrave CreditAttribution: smustgrave at Mobomo commentedWonder if this is still needed for D10?