DB Silliness: blocks_roles should reference bid, not module/delta

there is no DB change now. But I agree this must be fixed in D7 hence the critical.

subscribe

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'));

Similarly, when we configure asingle block we should load/save referencing bid, not module+delta

@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.

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.

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)

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...

@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)

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.

@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.

@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