system update 10100 cache tables fail

Created on 19 September 2023, 9 months ago
Updated 13 December 2023, 7 months ago

Problem/Motivation

Error Output:
================
> [notice] Update started: system_update_10100
> [error] Cannot change the definition of field 'cache_graphql.definitions.expire': field doesn't exist.
> [error] Update failed: system_update_10100
[error] Update aborted by: system_update_10100
[error] Finished performing updates.

Steps to reproduce

  1. Install Drupal 9
  2. Create a database table with a name that begins with cache_. Do not have a column named expire. This table could be created in a custom module with hook_schema() or created with a SQL tool or whatever.
  3. Upgrade to Drupal 10
  4. drush updatedb
  5. system_update_10100 will crash with "field doesn't exist".

Proposed resolution

In systrem.install on line 1800 after the foreach statement check if the cache table exist.

πŸ› Bug report
Status

Fixed

Version

10.1 ✨

Component
Database updateΒ  β†’

Last updated 6 days ago

No maintainer
Created by

πŸ‡©πŸ‡ͺGermany DiDebru

Live updates comments and jobs are added and updated live.
Sign in to follow issues

Merge Requests

Comments & Activities

  • Issue created by @DiDebru
  • πŸ‡©πŸ‡ͺGermany DiDebru
  • Status changed to Postponed: needs info 9 months ago
  • πŸ‡ΊπŸ‡ΈUnited States cilefen

    How did this happen?

  • πŸ‡©πŸ‡ͺGermany DiDebru

    Good question but unfortunately I don't have an answer yet but with that patch it does not happen.

  • Status changed to Needs review 8 months ago
  • last update 8 months ago
    29,676 pass
  • πŸ‡ΊπŸ‡ΈUnited States tetranz

    I've updated this to check if the table has an expire column. I got this error on a site which has a custom table with a name starting with cache_ but it does not have an expire column. It is used as a cache which never expires.

    I left DiDebru's check there but I don't really see how the table could not exist. I think that would give a different error message.

  • last update 8 months ago
    30,438 pass
  • Status changed to Postponed: needs info 8 months ago
  • πŸ‡ΊπŸ‡ΈUnited States smustgrave

    We still would need steps that caused this to happen. I did a 9.5 to 10.1 update just yesterday funny enough but did not hit this error. I hit a separate issue and turned out one of the custom modules had a problem. So wonder if that's the case here too.

  • πŸ‡ΊπŸ‡ΈUnited States tetranz
  • Status changed to Needs review 8 months ago
  • πŸ‡ΊπŸ‡ΈUnited States tetranz
  • Status changed to Needs work 8 months ago
  • πŸ‡ΊπŸ‡ΈUnited States smustgrave

    If an issue caused by core and not a custom module will need a test case also.

  • Our team ran into this issue and it hung after using the above patch. After looking into the update hook a bit further we compared changeField to watchdog's use of changefield and noticed a difference.

    We changed

    $schema->changeField('sessions', 'timestamp', 'timestamp', $new);

    to

    $connection->schema()->changeField('sessions', 'timestamp', 'timestamp', $new);

    Attached is a patch for 10.1.6. I couldn't grab the branch that the issue was applied to, but at least our patch is here for others to bypass this problem.

  • I wanted to repost that our issue was more to do with the size of the tables. In our case the size of our queue tables were substantial, but after truncating the tables we were able to upgrade with no issues.

  • πŸ‡¬πŸ‡§United Kingdom catch

    I don't think this needs explicit test coverage - we would have to make an additional database dump, with a custom/bogus cache_something table, then update that dump in the update test, to show that we don't get an error. Since we already test the update without such a table, we have implicit test coverage of the if statements in the usual case anyway.

    However this could use the patch in #5 converted to an MR, and if possible I'd like to get this committed before 10.2.0's release next week.

  • πŸ‡¬πŸ‡§United Kingdom alexpott πŸ‡ͺπŸ‡ΊπŸŒ

    I think we could build the list of cache bins from the container - or maybe even get it from there. And then we wouldn't be potentially touch other tables.

  • πŸ‡¬πŸ‡§United Kingdom alexpott πŸ‡ͺπŸ‡ΊπŸŒ

    Turns out #13 was optimistic because cache.container is not tagged and therefore not returned by Cache::getBins() - I'm pretty sure this is on purpose... but considering this is possible going for something simliar to #5 seems good.

  • Merge request !5718Non cache tables starting with cache_ β†’ (Open) created by alexpott
  • πŸ‡¬πŸ‡§United Kingdom alexpott πŸ‡ͺπŸ‡ΊπŸŒ

    alexpott β†’ changed the visibility of the branch 3388170-system-update-10100 to hidden.

  • πŸ‡¬πŸ‡§United Kingdom alexpott πŸ‡ͺπŸ‡ΊπŸŒ

    alexpott β†’ changed the visibility of the branch 10.1.x to hidden.

  • Status changed to Needs review 7 months ago
  • πŸ‡¬πŸ‡§United Kingdom alexpott πŸ‡ͺπŸ‡ΊπŸŒ

    Posted an MR with a test.

  • Status changed to RTBC 7 months ago
  • πŸ‡ΊπŸ‡ΈUnited States smustgrave

    Just posting here from the MR that alexpott ran

    1) Drupal\Tests\system\Functional\Update\Y2038TimestampUpdateTest::testUpdate
    The update failed with the following message: "Failed: Drupal\Core\Database\SchemaObjectDoesNotExistException: Cannot change the definition of field 'cache_bogus.expire': field doesn't exist. in Drupal\mysql\Driver\Database\mysql\Schema->changeField() (line 615 of /builds/project/drupal/core/modules/mysql/src/Driver/Database/mysql/Schema.php)."
    /builds/project/drupal/core/tests/Drupal/Tests/UpdatePathTestTrait.php:59
    /builds/project/drupal/core/modules/system/tests/src/Functional/Update/Y2038TimestampUpdateTest.php:115
    /builds/project/drupal/vendor/phpunit/phpunit/src/Framework/TestResult.php:728
    FAILURES!
    Tests: 1, Assertions: 36, Failures: 1.
    

    So test coverage appears to be there, applied the MR locally and can run the update just fine. Hope it can make it to 10.2

    • catch β†’ committed 288c0586 on 10.2.x
      Issue #3388170 by DiDebru, tetranz, alexpott, smustgrave: system update...
    • catch β†’ committed e8778fb2 on 11.x
      Issue #3388170 by DiDebru, tetranz, alexpott, smustgrave: system update...
  • Status changed to Downport 7 months ago
  • πŸ‡¬πŸ‡§United Kingdom catch

    Committed/pushed to 11.x and 10..2.x, thanks!

    10.1.x has had its last security release, so we need to decide what to do there a bit.

  • πŸ‡¬πŸ‡§United Kingdom catch

    Cherry-picked to 10.1.x, we can figure out if/when to release later.

  • Status changed to Fixed 7 months ago
    • catch β†’ committed 4948333b on 10.1.x
      Issue #3388170 by DiDebru, tetranz, alexpott, smustgrave: system update...
  • Automatically closed - issue fixed for 2 weeks with no activity.

Production build 0.69.0 2024