Length of menu_tree.url and menu_tree.route_param_key are too short (255 characters)

Patch #36 (for 9.5) was a clean re-roll of #28 (for 9.4) except that the hook update was renamed from system_update_9101() to system_update_9500(). There was also a new patch #36 for 10.1.x which has system_update_10101() but this no longer applies in 10.1.x. The current 10.1.x. system.install only has one hook update:

/**
 * Implements hook_update_last_removed().
 */
function system_update_last_removed() {
  return 8805;
}

/**
 * Update the stored schema data for entity identifier fields.
 */
function system_update_8901() 

So what should the new update be called in 10.1.x? Is system_update_10101() correct? I have converted patch 36 to a MR and left it at 10101 but I do not know if this is correct.

From @larowlan in #34

Also, I'm not sure its a bug, more-so a feature or task, and that influences my thinking about where we'd commit it.

I would say it is a bug, because generating content via the Devel module can fail with this error, see #3027826: String data, right truncated: 1406 Data too long for column 'menu_name' → and the follow-up on https://gitlab.com/drupalspoons/devel/-/issues/212

jonathan1055 → closed merge request !3437

Posting the static patch because using the MR doesn't allow pinning to a specific commit, so anyone can submit pretty much anything and inject it into our codebase IIRC. This also fixed our issue.

Thank you, was facing the same problem when linking to a facet page.
The patch in #46 fixes the issue on Drupal 10.1.6.

I am rerolling for 10.2.

As a bug will need test coverage.

Update hook will require a test as well.

May need submaintainer sign off.

Hello, I am experiencing the same issue as described in #47 (link to views with facets), however, the link is short: "products/school-type/2," while the that path has a long route_param_key:

[:db_update_placeholder_28] => f0=&f1=&f2=&f3=&f4=&f5=&f6=&f7=&f8=&f9=&f10=&f11=&f12=&f13=&f14=&f15=&f16=&f17=&f18=&f19=&f20=&f21=&f22=&f23=&f24=&f25=&f26=&f27=&f28=&f29=&f30=&f31=&f32=&f33=&f34=&f35=&f36=&f37=&f38=&f39=&view_id=product_catalog&display_id=product_catalog&facets_query=school-type/primary-2 [:db_condition_placeholder_0] => 500 ) in Drupal\Core\Menu\MenuTreeStorage->doSave() (line 303 of core/lib/Drupal/Core/Menu/MenuTreeStorage.php)

I have decided to use external link http://mywebsite.com/products/school-type/2 for my internal path to resolve that issue, however, this is not an ideal solution.

jrglasgow → changed the visibility of the branch 3106205-menutree-url-and-route-length to hidden.

I created a new merge request to 11.x and changed the system_update hook to 10202 so it ensured to run.

Just uploading an static patch from the MR, after upgrading the core to 10.2, the hook update conflicted with an older one. If anyone else is having the same issue, I also had to update the hook update number in order to ensure the hooks will be executed.

FYI: drush ev "\Drupal::keyValue('system.schema')->set('system', 10100)";

fwiw, we have a few dozen domains each with their own menu on our site, and we had to change route_param_key to a longblob - some values are over 10K.

Code updated against 11.x and upgrade path test coverage added.

Applied some nitpicky typehints and returns

Ran test-only feature, https://git.drupalcode.org/issue/drupal-3106205/-/jobs/2347336 which shows coverage

Applied locally and update hook ran fine.

Believe this is good.

Left comments in the MR. This will need a change record. And this one for a similar change, https://www.drupal.org/node/3227494 →

Re-roll for 10.3.

This is ready for another review again.

There are multiple branches open for this ticket. Could the ones not in use be hidden.

believe the test failure is random, but was tagged for a change record which still appears needed.

murilohp → changed the visibility of the branch 10.2.x to hidden.

murilohp → changed the visibility of the branch 10.1.x to hidden.

Just added a CR for this issue, and hide the unnecessary branches. Moving back to NR

Took a look at the 11.x MR. Looks like a simple and sensible fix, so moving back to RTBC.

Committed and pushed 35044e4519e to 11.x and 5b7419e8dfa to 11.1.x. Thanks!

FWIW we could backport this to 10.4.x / 10.5.x using https://www.drupal.org/node/3459876 →

Just checking, will this not be committed to 11.0.x ?

Automatically closed - issue fixed for 2 weeks with no activity.

Well, this issue is closed, but I would really like to get this working in 10.3.10 and I don't want to have any phantom core update hooks lying around. I considered creating a local patch for the schema definition and manually setting the length in the DB but that seems unwise.

So given the timing, I'll settle for a 10.4 release. I'm not really sure how to proceed with a fork in this scenario so I regenerated a patch using the API suggestion in #71. I think it will also require patching core 11.x (and 11.1.x?) with this:

$equivalent_update = \Drupal::service('update.update_hook_registry')->getEquivalentUpdate();
  if ($equivalent_update instanceof \Drupal\Core\Update\EquivalentUpdate) {
    return $equivalent_update->toSkipMessage();
  }

Is this patch helpful or do we need a core maintainer/git ninja to backport system.install?

For the quick sulution for exisitng website, can I just run ALTER TABLE menu_tree MODIFY route_param_key VARCHAR(1024);
Do you see any drawbacks in such a solution?

@adpo the reason that I'm not doing that is because it will cause a mismatch with the schema definition in /core/lib/Drupal/Core/Menu/MenuTreeStorage.php. That may lead to unexpected issues with Schema API that I'm not willing to risk.

quietone → closed merge request !6619