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

Hmm..Can't see why not?
In the Drupal core though, only the core/modules/link/src/Plugin/Field/FieldType/LinkItem.php (uri column) has length 2048.

Some browsers have max url char limit though, but I believe 2048 is on safe side.

D.

I checked the code as well and the only other fields/columns using 255 I found besides ones in core/lib/Drupal/Core/Menu/MenuTreeStorage.php are:

• core/tests/Drupal/Tests/Core/Entity/Sql/SqlContentEntityStorageSchemaTest.php: name
• core/lib/Drupal/Core/Config/DatabaseStorage.php: description, name
• core/modules/aggregator/src/FeedStorageSchema.php: url

so should FeedStorageSchema be updated as well?

I also looked for fields/columns called url, uri, and link and noticed that uri in core/modules/file/src/FileStorageSchema.php has no size limit at all which seems odd:

   case 'uri':
     $this->addSharedTableFieldIndex($storage_definition, $schema, TRUE);

Should this be updated?

Checked the missing size argument in core/modules/file/src/FileStorageSchema.php, the same is for D 8.8.* core, and from the comments of the function addSharedTableFieldIndex:

* @param int $size
   *   (optional) The index size. Defaults to no limit.

About the core/modules/aggregator/src/FeedStorageSchema.php: Can be updated also (I would go with $size=NULL for no limit). I tested though with long URLs and it doesn't seem to have issues.

D.

This probably dates to MyISAM unicode field length days, where 333 was the most characters we could safely put in a machine name key. This may be less of an issue now that MyISAM is no longer the default, but we should still consider that case.

IMO 255 is too short for URL/link storage and, while unlimited might seem like a good option, I'd prefer to limit database sizes when there is a reasonable limit that can be determined. 2048 seems like a good length based on browser support and could be used for FeedStorageSchema as well. That doesn't address @xjm's comment above regarding MyISAM but not sure how to address that.

But, since FileStorageSchema has no limit, then I see three approaches:

1) Update the patch so MenuTreeStorage and FeedStorageSchema use 2048 and leave FileStorageSchema with no limit. Downside is these are not consistent.

2) Update the patch so MenuTreeStorage and FeedStorageSchema use unlimited and leave FileStorageSchema with no limit. Downside (IMO) is there is no limit when there is a reasonable limit to expect with URLs/links based on browser support.

3) Update the patch so MenuTreeStorage, FeedStorageSchema, and FileStorageSchema all use 2048. Downside FileStorageSchema limit would be less than before.

Nice analysis @Kristen!
I would with option #1 (although as you mention is inconsistent), because limiting FileStorageSchema to 2048 could cause some trouble (edge casesprobably, but as far as I know the PATH_MAX default on some Linux distros is 4096)

Agreed with @dspachos on Option 1.
Attaching the patch, kindly review the same.

@mayurjadhav

I believe the

$this->addSharedTableFieldIndex($storage_definition, $schema, TRUE, 255);

refers to the index size.

From the database schema, I see that there is already:

"url" VARCHAR(2048) COLLATE NOCASE_UTF8 NOT NULL,

which is also the case in `/core/modules/link/src/Plugin/Field/FieldType/LinkItem.php`

...
'uri' => [
          'description' => 'The URI of the link.',
          'type' => 'varchar',
          'length' => 2048,
        ],
..

so, not sure if we need to modify something here

Not including an interdiff as the patch is small and #10 had some unrelated changes as suggested by #12

Applied patch #18 on 9.2.x and it works .The length of link field is changed to 2048 in this patch. Adding screenshot below

After patch:

The update numbering should be updated to 9.2.x, although if we want to backport it to Drupal 8.9, we should add it with an 89xx update number.

See #3108658: Handling update path divergence between 11.x and 10.x for more discussion of the update numbering issue.

This should also have an upgrade path test I think.

edit: after thinking through this more again, we may need to schedule this patch for 9.3.x to avoid the various update conflict scenarious with the 8.9 branch.

In this case, isn't this update hook a no-op if the schema has already changed, in which case there is no problem in running twice? Assume the scenario is that it is backported to 8.9, run there, and then a site upgrades to 9.2 and runs it again?

@longwave there are more scenarios. If it's committed to 9.2.x and not backported, then a later patch we need to backport needs special handling.

If it's committed to 9.2.x and 8.9.x, but not 9.1.x and/or 9.0.x, then 8.9.x sites updating to 9.0.x or 9.1.x will have the update 'missing'. I've updated the issue summary at #3108658: Handling update path divergence between 11.x and 10.x with slightly more explanation of the various ways this goes wrong.

It's true that in this case the update is non-destructive and doesn't need to migrate data, so re-running would probably be OK, but unfortunately that's not the only problem we have.

But why can't we commit it as _920x in 9.2 and _890x in 8.9? In this case there seems no harm in having it run already in the case the 8.9 site then upgrades to 9.1, except that the schema is slightly out of sync (but as the field is longer than the schema specifies, this isn't really a problem), and that will be fixed when the site upgrades to 9.2 - the schema definition is back in sync and the update hook will harmlessly run again.

@longwave that doesn't cover the case where it's committed to 8.9.x, then the site updates to 9.1.x (not 9.2.x), and the update is neither in the code base nor recorded as removed (i.e. it just goes missing from the perspective of the site). We've never had a case where an update would bypass a previous stable branch before and may not even have test coverage for it.

We don't record which specific hook_update_Ns have run, we only keep a high water mark and guarantee that we can upgrade from a minimum previous version via hook_update_last_removed, so I don't think there is an issue here either, but I could be wrong.

Applied patch #10 in drupal-9.3.x-dev applied successfully
and looks good for me
Thanks for the patch
For ref sharing screenshot ....

I have an error if I add the facet page to the menu: Example tree_menu.route_param_key
f39=&f29=&f22=&f23=&f24=&f25=&f26=&f27=&f28=&f30=&f20=&f31=&f32=&f33=&f34=&f35=&f36=&f37=&f38=&f21=&f19=&f0=&f8=&f1=&f2=&f3=&f4=&f5=&f6=&f7=&f9=&f18=&f10=&f11=&f12=&f13=&f14=&f15=&f16=&f17=&facets_query=product_human_url

Not sure if this is correct, but it fixes the bug.

Thanks for the update.

+++ b/core/modules/system/system.install
@@ -1644,3 +1644,25 @@ function _system_check_array_table_prefixes() {
+ * Increase length of 'menu_tree.url' and 'menu_tree.route_param_key' field from 255 to 2048.

Nitpick: Wrap at 80 characters.

Reviewed and tested the patch from #26
The length has been increased

Updated the patch with the Nitpick: Wrap at 80 characters.

Confirming patch #28 works on 9.4.x:

* Before applying patch: route_param_key and url fields have maximum length of 255 chars

cli-drupal:/app$ drush sqlc
Welcome to the MariaDB monitor.  Commands end with ; or \g.
Your MariaDB connection id is 91
Server version: 10.4.19-MariaDB MariaDB Server

Copyright (c) 2000, 2018, Oracle, MariaDB Corporation Ab and others.

Type 'help;' or '\h' for help. Type '\c' to clear the current input statement.

MariaDB [drupal]> SELECT COLUMN_NAME, CHARACTER_MAXIMUM_LENGTH FROM information_schema.COLUMNS WHERE TABLE_SCHEMA = 'drupal' AND TABLE_NAME = 'menu_tree' AND COLUMN_NAME IN('route_param_key', 'url');
+-----------------+--------------------------+
| COLUMN_NAME     | CHARACTER_MAXIMUM_LENGTH |
+-----------------+--------------------------+
| route_param_key |                      255 |
| url             |                      255 |
+-----------------+--------------------------+
2 rows in set (0.001 sec)

MariaDB [drupal]> exit
Bye

* After applying patch: route_param_key and url fields have maximum length of 2048 chars

cli-drupal:/app$ drush updb
 -------- ----------- --------------- -----------------------------------------
  Module   Update ID   Type            Description
 -------- ----------- --------------- -----------------------------------------
  system   9101        hook_update_n   9101 - Update length of menu_tree
                                       fields url and route_param_key from 255
                                       to 2048.
 -------- ----------- --------------- -----------------------------------------


 Do you wish to run the specified pending updates? (yes/no) [yes]:
 > yes

>  [notice] Update started: system_update_9101
>  [notice] Update completed: system_update_9101
 [success] Finished performing updates.
cli-drupal:/app$ drush sqlc
Welcome to the MariaDB monitor.  Commands end with ; or \g.
Your MariaDB connection id is 98
Server version: 10.4.19-MariaDB MariaDB Server

Copyright (c) 2000, 2018, Oracle, MariaDB Corporation Ab and others.

Type 'help;' or '\h' for help. Type '\c' to clear the current input statement.

MariaDB [drupal]> SELECT COLUMN_NAME, CHARACTER_MAXIMUM_LENGTH FROM information_schema.COLUMNS WHERE TABLE_SCHEMA = 'drupal' AND TABLE_NAME = 'menu_tree' AND COLUMN_NAME IN('route_param_key', 'url');
+-----------------+--------------------------+
| COLUMN_NAME     | CHARACTER_MAXIMUM_LENGTH |
+-----------------+--------------------------+
| route_param_key |                     2048 |
| url             |                     2048 |
+-----------------+--------------------------+
2 rows in set (0.001 sec)

Link field of Add menu link form also has maxlength of 2048

The issue summary (and title) don't explain why we're also changing route_param_key here. It makes sense to do both in one update, but the issue summary should document why both need to be changed.

There's also no test coverage here for the update, but we don't have a schema introspection API so I think it's probably OK to rely on the existing test coverage that at least the update runs without errors, and manual testing has been done.

This will need to be committed to 9.5.x and 10.0.x with the same update number.

Thanks everyone! Marking RTBC based on:

1. Updated issue summary to reflect the new focus.

2. Comment #31 indicates no tests are needed.

3. Manual testing was successful.

4. Code is straightforward and only addresses the issue in the issue summary.

5. Tests pass.

+++ b/core/modules/system/system.install
@@ -1644,3 +1644,25 @@ function _system_check_array_table_prefixes() {
+function system_update_9101() {

This will need to be renumbered to something starting with 95xx now, as that's the lowest version it could be committed to.

However I'm not sure we'd consider this for 9.5 or 10.0 at this point, I'll check with a release manager, it may be this is 10.1 only at this point.

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.

Given this is a numbered update that's changing the schema of a table in the critical path, I think we should defer it to 10.1.x

It's also still not clear to me why route_param_key is being increased in length. External URLs can be an arbitrary length, but route params are generally fairly limited and controllable. If we're not sure, I'd open a new issue for that change and handle it separately.

Re: #34

It's also still not clear to me why route_param_key is being increased in length. External URLs can be an arbitrary length, but route params are generally fairly limited and controllable.

I got an error with route_param_key on an attempt to add the Search API menu link like this:

/search/category/900/translation_excluded/30/translation_excluded/836

I have 30+ facets defined, so it looks like that is making the route_param_key parameter very long.

So, it seems that route_param_key needs an update as well.

Here is my error in full:

The website encountered an unexpected error. Please try again later.

Drupal\Core\Entity\EntityStorageException: SQLSTATE[22001]: String data, right truncated: 1406 Data too long for column 'route_param_key' at row 1: UPDATE "menu_tree" SET "menu_name"=:db_update_placeholder_0, "url"=:db_update_placeholder_1, "route_name"=:db_update_placeholder_2, "route_parameters"=:db_update_placeholder_3, "options"=:db_update_placeholder_4, "title"=:db_update_placeholder_5, "description"=:db_update_placeholder_6, "weight"=:db_update_placeholder_7, "enabled"=:db_update_placeholder_8, "expanded"=:db_update_placeholder_9, "parent"=:db_update_placeholder_10, "id"=:db_update_placeholder_11, "class"=:db_update_placeholder_12, "provider"=:db_update_placeholder_13, "metadata"=:db_update_placeholder_14, "form_class"=:db_update_placeholder_15, "has_children"=:db_update_placeholder_16, "discovered"=:db_update_placeholder_17, "depth"=:db_update_placeholder_18, "p1"=:db_update_placeholder_19, "p2"=:db_update_placeholder_20, "p3"=:db_update_placeholder_21, "p4"=:db_update_placeholder_22, "p5"=:db_update_placeholder_23, "p6"=:db_update_placeholder_24, "p7"=:db_update_placeholder_25, "p8"=:db_update_placeholder_26, "p9"=:db_update_placeholder_27, "route_param_key"=:db_update_placeholder_28 WHERE "mlid" = :db_condition_placeholder_0; Array ( [:db_update_placeholder_0] => main [:db_update_placeholder_1] => [:db_update_placeholder_2] => view.source_search.page_1 [:db_update_placeholder_3] => a:41:{s:3:"f39";s:0:"";s:3:"f29";s:0:"";s:3:"f22";s:0:"";s:3:"f23";s:0:"";s:3:"f24";s:0:"";s:3:"f25";s:0:"";s:3:"f26";s:0:"";s:3:"f27";s:0:"";s:3:"f28";s:0:"";s:3:"f30";s:0:"";s:3:"f20";s:0:"";s:3:"f31";s:0:"";s:3:"f32";s:0:"";s:3:"f33";s:0:"";s:3:"f34";s:0:"";s:3:"f35";s:0:"";s:3:"f36";s:0:"";s:3:"f37";s:0:"";s:3:"f38";s:0:"";s:3:"f21";s:0:"";s:3:"f19";s:0:"";s:2:"f0";s:0:"";s:2:"f8";s:0:"";s:2:"f1";s:0:"";s:2:"f2";s:0:"";s:2:"f3";s:0:"";s:2:"f4";s:0:"";s:2:"f5";s:0:"";s:2:"f6";s:0:"";s:2:"f7";s:0:"";s:2:"f9";s:0:"";s:3:"f18";s:0:"";s:3:"f10";s:0:"";s:3:"f11";s:0:"";s:3:"f12";s:0:"";s:3:"f13";s:0:"";s:3:"f14";s:0:"";s:3:"f15";s:0:"";s:3:"f16";s:0:"";s:3:"f17";s:0:"";s:12:"facets_query";s:61:"category/900/translation_excluded/30/translation_excluded/836";} [:db_update_placeholder_4] => a:1:{s:5:"query";a:0:{}} [:db_update_placeholder_5] => s:26:"Field Notes (English only)"; [:db_update_placeholder_6] => [:db_update_placeholder_7] => -50 [:db_update_placeholder_8] => 1 [:db_update_placeholder_9] => 0 [:db_update_placeholder_10] => menu_link_content:90a4439c-d6aa-448b-8b50-411c8ec0b1ca [:db_update_placeholder_11] => menu_link_content:77b5d6ca-f6c6-4865-864e-960623bb3441 [:db_update_placeholder_12] => Drupal\menu_link_content\Plugin\Menu\MenuLinkContent [:db_update_placeholder_13] => menu_link_content [:db_update_placeholder_14] => a:1:{s:9:"entity_id";s:3:"102";} [:db_update_placeholder_15] => \Drupal\menu_link_content\Form\MenuLinkContentForm [:db_update_placeholder_16] => 0 [:db_update_placeholder_17] => 0 [:db_update_placeholder_18] => 3 [:db_update_placeholder_19] => 656 [:db_update_placeholder_20] => 657 [:db_update_placeholder_21] => 783 [:db_update_placeholder_22] => 0 [:db_update_placeholder_23] => 0 [:db_update_placeholder_24] => 0 [:db_update_placeholder_25] => 0 [:db_update_placeholder_26] => 0 [:db_update_placeholder_27] => 0 [:db_update_placeholder_28] => f39=&f29=&f22=&f23=&f24=&f25=&f26=&f27=&f28=&f30=&f20=&f31=&f32=&f33=&f34=&f35=&f36=&f37=&f38=&f21=&f19=&f0=&f8=&f1=&f2=&f3=&f4=&f5=&f6=&f7=&f9=&f18=&f10=&f11=&f12=&f13=&f14=&f15=&f16=&f17=&facets_query=category/900/translation_excluded/30/translation_excluded/836 [:db_condition_placeholder_0] => 783 ) in Drupal\Core\Entity\Sql\SqlContentEntityStorage->save() (line 811 of core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorage.php).

I agree with @devad. Here is another Search API example:

Drupal\Core\Entity\EntityStorageException: SQLSTATE[22001]: String data, right truncated: 1406 Data too long for column 'route_param_key' at row 1: UPDATE "menu_tree" SET "menu_name"=:db_update_placeholder_0, "route_name"=:db_update_placeholder_1, "route_parameters"=:db_update_placeholder_2, "url"=:db_update_placeholder_3, "title"=:db_update_placeholder_4, "description"=:db_update_placeholder_5, "parent"=:db_update_placeholder_6, "weight"=:db_update_placeholder_7, "options"=:db_update_placeholder_8, "expanded"=:db_update_placeholder_9, "enabled"=:db_update_placeholder_10, "provider"=:db_update_placeholder_11, "metadata"=:db_update_placeholder_12, "class"=:db_update_placeholder_13, "form_class"=:db_update_placeholder_14, "id"=:db_update_placeholder_15, "discovered"=:db_update_placeholder_16, "has_children"=:db_update_placeholder_17, "depth"=:db_update_placeholder_18, "p1"=:db_update_placeholder_19, "p2"=:db_update_placeholder_20, "p3"=:db_update_placeholder_21, "p4"=:db_update_placeholder_22, "p5"=:db_update_placeholder_23, "p6"=:db_update_placeholder_24, "p7"=:db_update_placeholder_25, "p8"=:db_update_placeholder_26, "p9"=:db_update_placeholder_27, "route_param_key"=:db_update_placeholder_28 WHERE "mlid" = :db_condition_placeholder_0; Array ( [:db_update_placeholder_0] => main [:db_update_placeholder_1] => view.search.page_1 [:db_update_placeholder_2] => a:41:{s:2:"f0";s:0:"";s:2:"f1";s:0:"";s:2:"f2";s:0:"";s:2:"f3";s:0:"";s:2:"f4";s:0:"";s:2:"f5";s:0:"";s:2:"f6";s:0:"";s:2:"f7";s:0:"";s:2:"f8";s:0:"";s:2:"f9";s:0:"";s:3:"f10";s:0:"";s:3:"f11";s:0:"";s:3:"f12";s:0:"";s:3:"f13";s:0:"";s:3:"f14";s:0:"";s:3:"f15";s:0:"";s:3:"f16";s:0:"";s:3:"f17";s:0:"";s:3:"f18";s:0:"";s:3:"f19";s:0:"";s:3:"f20";s:0:"";s:3:"f21";s:0:"";s:3:"f22";s:0:"";s:3:"f23";s:0:"";s:3:"f24";s:0:"";s:3:"f25";s:0:"";s:3:"f26";s:0:"";s:3:"f27";s:0:"";s:3:"f28";s:0:"";s:3:"f29";s:0:"";s:3:"f30";s:0:"";s:3:"f31";s:0:"";s:3:"f32";s:0:"";s:3:"f33";s:0:"";s:3:"f34";s:0:"";s:3:"f35";s:0:"";s:3:"f36";s:0:"";s:3:"f37";s:0:"";s:3:"f38";s:0:"";s:3:"f39";s:0:"";s:12:"facets_query";s:70:"type/participant/field_part_entite/atelier-de-bio-informatique-abi-167";} [:db_update_placeholder_3] => [:db_update_placeholder_4] => s:38:"Liste du personnel de l'équipe A.B.I."; [:db_update_placeholder_5] => [:db_update_placeholder_6] => menu_link_content:bad907c0-f360-41d6-baaf-27252dd75e20 [:db_update_placeholder_7] => 0 [:db_update_placeholder_8] => a:1:{s:5:"query";a:0:{}} [:db_update_placeholder_9] => 0 [:db_update_placeholder_10] => 1 [:db_update_placeholder_11] => menu_link_content [:db_update_placeholder_12] => a:1:{s:9:"entity_id";s:2:"48";} [:db_update_placeholder_13] => Drupal\menu_link_content\Plugin\Menu\MenuLinkContent [:db_update_placeholder_14] => \Drupal\menu_link_content\Form\MenuLinkContentForm [:db_update_placeholder_15] => menu_link_content:6aace182-2d00-404b-bce2-cff5e8467fea [:db_update_placeholder_16] => 0 [:db_update_placeholder_17] => 0 [:db_update_placeholder_18] => 3 [:db_update_placeholder_19] => 268 [:db_update_placeholder_20] => 279 [:db_update_placeholder_21] => 295 [:db_update_placeholder_22] => 0 [:db_update_placeholder_23] => 0 [:db_update_placeholder_24] => 0 [:db_update_placeholder_25] => 0 [:db_update_placeholder_26] => 0 [:db_update_placeholder_27] => 0 [: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=&facets_query=type/participant/field_part_entite/atelier-de-bio-informatique-abi-167 [:db_condition_placeholder_0] => 295 ) in Drupal\Core\Entity\Sql\SqlContentEntityStorage->save() (line 811 of core/lib/Drupal/Core/Entity/Sql/SqlContentEntityStorage.php).

Patch #36 works for Drupal 9.4.8.

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

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

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

Just uploading a 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 is 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.

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 ?

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 from the older patches. 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.

.

This is a re-roll for 10.4.x (update hook number needs to be updated).

I am still on Drupal 10.5.1 with Facets, and as others also get an "1406 Data too long for column 'route_param_key'" error from a string like this:

facets_query=&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=browse_motives_search_api&display_id=motives.

Is a Drupal 10 backport worth considering, or is the recommendation to upgrade to Drupal 11?

EDIT: It seems like I now get the above error, every time I rebuild caches ...

I think it would be really helpful, if someone who knew the details could share the steps to fix this in Drupal 10. Perhaps something along these lines, under a new "Workaround" header?

Workaround for Drupal 10

You can patch Drupal 10 with these steps:

1. Download the latest re-roll for Drupal 10.5 from comment #XYZ
2. Set the update hook number to (what?)

@ressa, here's what worked for me. There is no need to reset any hook update number, unless you had applied a previous patch or have any other patch adding the same hook number.

You can patch Drupal 10 with these steps:

Download the patch from #79
Run database update

Thanks @bohemier! Though, what if later on, an actual official Drupal core 10401 update is released, fixing something entirely different. Will it not then be blocked from getting executed, since that number has already been run?

Though I guess, if your Drupal version is for example 10.5.1, it should be safe to use 10401, since going forward, only updates in the range of 105xx and upwards will be run on a Drupal 10.5 installation? I guess I answered myself there:

• Drupal +10.5 can use 10401
  If on Drupal +10.5, all is well and you can use 10401.
• Drupal 10.4 should not use 10401
  Do not use 10401 if you are still on 10.4. Since Drupal 10.4 may still get updates ("Security release window for Drupal 11.1.x and 10.4.x"), using 10401 in a custom update hook might block an actual, later Drupal 10.4 update.

I have now added a new Setting number for custom update hook section, it would be great if someone could review it, and make sure it's not wrong? Thanks!

I have now created a patch with #79 and patched Drupal 10.5.1 with it. It went well, and I can create a Menu item for a Search API View with many Facets:

$ composer install
[...]
  - Applying patches for drupal/core
    assets/patches/3106205-length-menu-tree-too-short-10.4.x-79.patch
    (3106205: Length of menu_tree.url and menu_tree.route_param_key are too short (255 characters)
    https://www.drupal.org/project/drupal/issues/3106205)

$ drush updatedb
 -------- ----------- --------------- ---------------------------------------
  Module   Update ID   Type            Description                           
 -------- ----------- --------------- ---------------------------------------
  system   10401       hook_update_n   10401 - 
  Update length of menu_tree fields url and route_param_key from 255 to 2048.  
 -------- ----------- --------------- ---------------------------------------
 ┌ Do you wish to run the specified pending updates? ───────────┐
 │ Yes                                                          │
 └──────────────────────────────────────────────────────────────┘
>  [notice] Update started: system_update_10401
>  [notice] Update completed: system_update_10401
 [success] Finished performing updates.

I skimmed the interesting issue #3108658: Handling update path divergence between 11.x and 10.x (a long discussion about numbering upgrade hooks) and added it to the update hook doc page.

I also added link to the doc page in the Issue Summary.

Re-rolled for 10.6