DB Silliness: blocks_roles should reference bid, not module/delta
webchick - August 9, 2007 - 04:43
| Project: | Drupal |
| Version: | 7.x-dev |
| Component: | database system |
| Category: | bug report |
| Priority: | critical |
| Assigned: | Unassigned |
| Status: | duplicate |
Jump to:
Description
We now have this nice primary key in the blocks table; block_roles should reference it, rather than the old crusty module/delta combo.
I filed under 6, as this seems like a clear over-sight, but in case it's too major to fix there, please re-file against 7.
#1
there is no DB change now. But I agree this must be fixed in D7 hence the critical.
#2
subscribe
#3
Updating the title. This typo confused the hell out of me when trying to grep for the mysteriously non-existant table.
For further information, here are the current references to {blocks_roles} in HEAD.
$ grep -R blocks_roles *
modules/block/block.admin.inc: $result = db_query("SELECT rid FROM {blocks_roles} WHERE module = '%s' AND delta = '%s'", $module, $delta);
modules/block/block.admin.inc: db_query("DELETE FROM {blocks_roles} WHERE module = '%s' AND delta = '%s'", $form_state['values']['module'], $form_state['values']['delta']);
modules/block/block.admin.inc: db_query("INSERT INTO {blocks_roles} (rid, module, delta) VALUES (%d, '%s', '%s')", $rid, $form_state['values']['module'], $form_state['values']['delta']);
modules/block/block.admin.inc: db_query("INSERT INTO {blocks_roles} (rid, module, delta) VALUES (%d, '%s', '%s')", $rid, $form_state['values']['module'], $delta);
modules/block/block.module: $result = db_query("SELECT DISTINCT b.* FROM {blocks} b LEFT JOIN {blocks_roles} r ON b.module = r.module AND b.delta = r.delta WHERE b.status = 1 AND b.custom != 0 AND (r.rid IN (". db_placeholders($rids) .") OR r.rid IS NULL) ORDER BY b.weight, b.module", $rids);
modules/block/block.module: $result = db_query(db_rewrite_sql("SELECT DISTINCT b.* FROM {blocks} b LEFT JOIN {blocks_roles} r ON b.module = r.module AND b.delta = r.delta WHERE b.theme = '%s' AND b.status = 1 AND (r.rid IN (". db_placeholders($rids) .") OR r.rid IS NULL) ORDER BY b.region, b.weight, b.module", 'b', 'bid'), array_merge(array($theme_key), $rids));
modules/block/block.install: $schema['blocks_roles'] = array(
modules/menu/menu.admin.inc: db_query("DELETE FROM {blocks_roles} WHERE module = 'menu' AND delta = '%s'", $menu['menu_name']);
modules/system/system.install: $ret[] = update_sql("UPDATE {blocks_roles} SET delta = '". $menu_name ."' WHERE module = 'menu' AND delta = '". $mid ."'");
modules/system/system.install: db_add_index($ret, 'blocks_roles', 'rid', array('rid'));
#4
Similarly, when we configure asingle block we should load/save referencing bid, not module+delta
#5
@webchick - are you sure? I think this proposed change is wrong. Note that the you can have multiple themes where a single block is active, but bid will only link to a block within a SINGLE theme.
#6
Upon further inspection, I think pwolanin is right, and I have no idea why bid is even there to begin with? Why do we need a unique id for a block/theme/region correlation?
#7
Every table needs a primary key; If for no other reason than various database management tools break if you don't. :-) However, that doesn't mean it has to be a surrogate key (bid). If module/delta/theme provides a suitably unique value, then the bid is extraneous.
#8
we added the new UNIQUE key here: http://cvs.drupal.org/viewvc.py/drupal/drupal/modules/block/block.instal...
I guess we could have removed bid then by making that the PRIMARY key? I don't remember whether that was on the table, or it just seemed an extra fight to remove bid too (given that I think that patch went in pretty late)
#9
My gut is telling me varchar keys are expensive. That might just be superstitious, but I'd like to see how it affects things.
the becoming tied to a theme thing is also interesting...
#10
@neclimdul - this is not a large table, so I don't think it will really be an issue - we already use varchar PRIMARY for a couple tables (e.g. menu_router)
#11
I thinks that something similar to the proposed here would be OK: http://drupal.org/node/257032#comment-843364
Getting rid of the block deltas and use instead unique blocks' ID for each block and for the primary key of the blocks table is worth considering. Each module should provide blocks with unique names, in accordance with Drupal namespace rules (i.e. MODULE_block_id). Then the relation bid, theme, region is stored in a separate blocks_regions table.
#12
@dropcube - no, I think there is no reason to have bid at all. If each block has a unique name, then you should use that as the primary key.
#13
@pwolanin: I agree, with bid I meant a block unique ID, which is in fact the block_name.
I am setting the status to duplicate to continue the discussion of the block system and block database schema refactoring at http://drupal.org/node/257032