Extend the size of block.delta to 64

no hook_update in D8

For Views, we actually need 65 -- we use a 32 byte + a - + a 32 byte which can lead to a 65 character delta.

@dereine,
It's not no hook_update function. Just no hook_update_N during development. I can't point you to a relative issue but someone had told me about it when I created a patch with hook_update. I think the upgrade path will be add into system module at some point of time.

Hope someone could give us more info and double confirm about it.

Update hooks are most definently added during the development process. Sometime they move get dependencies added etc. At the moment there is no way to test the upgrade path though.

Patch updated to change the delta in block, block_rules, and block_language to 65 characters to accommodate the views block deltas.

I would suggest this gets committed, and update View's delta hashing to work with the 65 character delta in a followup issue.

Fixed patch and add 65 character length instead of 64 how described in comment #4

Needs review.

Is it correct that block_node_type's delta has a length of 64?

+function node_update_8011() {
+
+  db_change_field('block_node_type', 'delta', 'delta', array(
+    'type' => 'varchar',
+    'length' => 64,
+    'not null' => TRUE,
+    'description' => "The block's unique delta within module, from {block}.delta.",
+  ));
+}

Oops. Wrong character length in node "block_node_type" patch! Sorry.

Wrong file mode found .... ;(

The patch works and the database tables are correctly updated.

This will conflict with #1535868: Convert all blocks into plugins, and it also does not adjust the Views handling of 32 character block deltas.

W.r.t the views handling, it doesn't break it. I think updating views is something that can be done in a follow-up issue.

@tim.plunkett This issue seems to have stalled for over a year.

I want to make sure this doesn't get forgotten.

The patch here seems pretty reasonable. Should it be marked postponed based on #1535868: Convert all blocks into plugins?

Just ran into this problem on D7 & was hoping it would be something on it's way to be backported...

No, that issue was committed in Janurary 2013, this is still needs work (not postponed).

14: block-extend_the_size_of_block_delta_to_65-8.patch queued for re-testing.

This applies fine in git, but ya "Cannot redeclare block_update_8005()" at the very least it needs an update to see that the functions aren't duplicated.

@tim.plunkett did you have other concerns from #16?

Oh. None of those database tables exist anymore...
So either close it or backport it, I guess.

This is no longer relevant for D8, so safe to try to bring into D7.

Basically a re-roll.. I haven't tested it locally to see if it works yet.

25: block-extend_the_size_of_block_delta_to_65-24.patch queued for re-testing.

Ran into Cannot redeclare node_update_7014() had to auto increment.

I have encountered the same problem on a site that auto-generates some blocks and uses machine-names that can get up towards 64 chars.

I have updated the patch for D7 with the following:

• Removed block_language as it does not exist on D7
• Added a drop of the primary key for the two changes that affected fields that where a part of a primary key (the primary-key is recreated as a part of the change)
• Added the update to the relevant hook_schema implementations so that it also works on fresh installs

The patch applies cleanly on a 7.x and as far as I have testet that both the hook_schema and hook_install works.

The big question now is how big a overhead it will be to drop and recreate the indexes and whether it is a good idea to force this on every drupal site out there "just" to get the delta to 65 chars?

The decisions look good.. Thanks for pushing ahead on this patch.

Applied nicely here http://s4a5ed0421a6bbf8.s3.simplytest.me/

We could do it on new installs only...

Not sure that forcing all 1 million Drupal sites to re-index is worth it.

Interesting challenge though.

Been a while since @asaal touched this patch, so unasigning him.

I think we should just go with 64:

First off, it does seem that views is handling the limitation currently: http://cgit.drupalcode.org/views/tree/views.module#n663

  // block.module has a delta length limit of 32, but our deltas can
  // unfortunately be longer because view names can be 32 and display IDs
  // can also be 32. So for very long deltas, change to md5 hashes.
  $hashes = array();

  // get the keys because we're modifying the array and we don't want to
  // confuse PHP too much.
  $keys = array_keys($items);
  foreach ($keys as $delta) {
    if (strlen($delta) >= 32) {
      $hash = md5($delta);
      $hashes[$hash] = $delta;
      $items[$hash] = $items[$delta];
      unset($items[$delta]);
    }
  }

Secondly the actual value we would need in order to support non-hashed view deltas is 193:

The keys being hashed above are generated here http://cgit.drupalcode.org/views/tree/plugins/views_plugin_display_block... and as mentioned in #4 the code does indeed concatenate two strings with a "-" so the size we need is [length of view->name] + 1 + [length of display->id]

  /**
   * The default block handler doesn't support configurable items,
   * but extended block handlers might be able to do interesting
   * stuff with it.
   */
  function execute_hook_block_list($delta = 0, $edit = array()) {
    $delta = $this->view->name . '-' . $this->display->id;
    $desc = $this->get_option('block_description');
...

But looking into the schema-definition for the tables that are storing the view names and display id's they seems that their lengths are 128 and 64 chars respectively

Views display:
http://cgit.drupalcode.org/views/tree/views.install#n179

Views view (initial schema hook and subsequent update)
http://cgit.drupalcode.org/views/tree/views.install#n89
http://cgit.drupalcode.org/views/tree/views.install#n615

 $schema['views_display'] = array(
    'description' => 'Stores information about each display attached to a view.',
    'fields' => array(
      'vid' => array(
        'type' => 'int',
        'unsigned' => TRUE,
        'not null' => TRUE,
        'default' => 0,
        'description' => 'The view this display is attached to.',
        'no export' => TRUE,
      ),
      'id' => array(
        'type' => 'varchar',
        'length' => '64',
        'default' => '',
        'not null' => TRUE,
        'description' => 'An identifier for this display; usually generated from the display_plugin, so should be something like page or page_1 or block_2, etc.',
      ),
...
function views_schema_6000() {
  $schema['views_view'] = array(
    'description' => 'Stores the general data for a view.',
    'export' => array(
      'identifier' => 'view',
...
      'name' => array(
        'type' => 'varchar',
        'length' => '32',
        'default' => '',
        'not null' => TRUE,
        'description' => 'The unique name of the view. This is the primary field views are loaded from, and is used so that views may be internal and not necessarily in the database. May only be alphanumeric characters plus underscores.',
      ),
...
function views_schema_7301() {
  $schema = views_schema(__FUNCTION__);
  $schema['views_view']['fields']['name']['length'] = 128;
  return $schema;
}

(Name got bumped to 128 in https://www.drupal.org/node/1395430)

So, if we wanted to support non-md5 block names for views we would have to go to 128+64+1 = 193 char (or probably just all the way to 256 to avoid wird values).

Personally I thing we should just go with 64, updated patch attached for the sake of completeness.

Yeah, 65 is kind of random - 64 makes more sense.

Don't we also need to drop and recreate the unique key on the 'block' table (since it contains 'delta' too?).

And what about the 'block_node_type' table provided by the node module - don't we need to update that one too?

Adding a related issue which I closed as a duplicate of this one.