Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fixed: cache_entity_* tables must be dropped before running update.php when upgrading from Drupal 7. #5340

Closed
klonos opened this issue Nov 2, 2021 · 15 comments · Fixed by backdrop/backdrop#3846

Comments

@klonos
Copy link
Member

klonos commented Nov 2, 2021

See: https://www.youtube.com/watch?v=U0RED0cZZwo&t=756s

The person running the update said that they have in the past run the update, and they have been getting errors if the following tables exist in the database:

  • cache_entity_comment
  • cache_entity_file
  • cache_entity_node
  • cache_entity_taxonomy_term
  • cache_entity_user

Their workaround has been to manually drop these tables. Can we do anything about that?


Steps to reproduce

  1. install vanilla D7 + Entity Cache module
  2. export the D7 database
  3. install a vanilla Backdrop instance
  4. import the db from step 2 above into the Backdrop site
  5. open your settings.php file, and change $settings['update_free_access'] = FALSE; to TRUE
  6. run /core/update.php

Actual result

screencapture-orkiejrb-lndo-site-core-update-php-2021-12-12-14_05_47

After the updates have finished, the following error is shown:

Warning: Use of undefined constant VIEWS_STORAGE_DEFAULT - assumed 'VIEWS_STORAGE_DEFAULT'
(this will throw an Error in a future version of PHP)
in filter_update_1004() (line 491 of /core/modules/filter/filter.install).

In the db updates report section, the following update failures are shown:

user module

Update #1016
Failed: DatabaseSchemaObjectExistsException: Table cache_entity_user already exists.
in DatabaseSchema->createTable() (line 661 of /core/includes/database/schema.inc).

taxonomy module

Update #1008
Failed: DatabaseSchemaObjectExistsException: Table cache_entity_taxonomy_term already exists.
in DatabaseSchema->createTable() (line 661 of /core/includes/database/schema.inc).

comment module

Update #1006
Failed: DatabaseSchemaObjectExistsException: Table cache_entity_comment already exists.
in DatabaseSchema->createTable() (line 661 of /core/includes/database/schema.inc).

node module

Update #1021
Failed: DatabaseSchemaObjectExistsException: Table cache_entity_node already exists.
in DatabaseSchema->createTable() (line 661 of /core/includes/database/schema.inc).

file module

Update #1008
Failed: DatabaseSchemaObjectExistsException: Table cache_entity_file already exists.
in DatabaseSchema->createTable() (line 661 of /core/includes/database/schema.inc).

Expected result

No errors.

@keiserjb
Copy link

keiserjb commented Nov 2, 2021

Yes, my experience has been that if the site is already installed Backdrop creates these database tables. My D7 database did not have these tables. update.php apparently tries to create these tables but can't when they already exist.

@laryn
Copy link
Contributor

laryn commented Nov 10, 2021

When #74 went in, the update hooks include lines like this:

db_create_table('cache_entity_comment', $table);

But should probably have checked first to see if the table exists already (in the case of upgrades that already had the table):

if (db_table_exists('cache_entity_comment')) {
  db_create_table('cache_entity_comment', $table);
}

Or:

  • dropped the table if it existed already and recreated fresh (to ensure schema integrity)
  • add a safety into db_create_table so it doesn't try to create a table that already exists?

@klonos
Copy link
Member Author

klonos commented Nov 11, 2021

Since these are cache tables, I think that it'd be safer to drop them and re-create them (the data they hold is by nature disposable & recreate-able anyway). Doing so would ensure that the tables are using the latest schema.

@indigoxela
Copy link
Member

indigoxela commented Nov 11, 2021

One thing maybe worth to consider: actually, if doing it that way, there might exist additional db tables, that cause trouble - other than the cache tables. Although it's not recommended to install/enable modules in B prior to upgrading.

Personally, I wouldn't recommend to just overwrite the database with the dump's contents, which means that pre-existing tables that are not overridden by the dump, are kept as-is. I'd suggest to actually either drop the B database and then import the D7 database (cleanly), or switch to a different one via the db credentials in settings.php.

There still might be problems with config - for instance if the D7 site did not use the comments module, but B enables the module for default installs. So the comment config is around, but there are no db tables for it in the D7 dump. But that's out of scope of this issue.

@laryn
Copy link
Contributor

laryn commented Nov 11, 2021

Yes, actually that makes sense that it may be because I ran the import multiple times and reloaded (but didn't empty first) the 'backdrop-ready' database each time. I was assuming it was because Entity Cache was active on the D7 site -- but I guess I don't know that I even verified that that was true in this case (update: I just checked and in this case Entity Cache was not active on the D7 site, but I think that is technically possible that EC would have created those tables in D7).

@indigoxela
Copy link
Member

I just checked and in this case Entity Cache was not active on the D7 site, but I think that is technically possible that EC would have created those tables in D7

@laryn you're, of course, right. Entity cache could be installed in D7, so in this case it's correct to drop and recreate the tables, to make sure, it's Backdrop's schema.

@herbdool
Copy link

herbdool commented Dec 2, 2021

Seemed like an easy one so created a PR backdrop/backdrop#3846

@laryn
Copy link
Contributor

laryn commented Dec 2, 2021

Code looks good 👍

@quicksketch quicksketch modified the milestones: 1.20.3, 1.20.4 Dec 3, 2021
@klonos
Copy link
Member Author

klonos commented Dec 10, 2021

Yup, the code change looks straight-forward to me too 👍🏼

@keiserjb do you have capacity/motivation to give this a go? ...if I get a chance, I'll re-watch the video and try to come up with minimal steps to reproduce this.

@keiserjb
Copy link

Works for me without needing to drop the tables.

@stpaultim
Copy link
Member

Is that enough testing to warrant a "Works for Me"?

@klonos
Copy link
Member Author

klonos commented Dec 10, 2021

I plan to also take a shot at it tomorrow:

  1. install vanilla D7 + Entity Cache module
  2. export db
  3. install vanilla Backdrop
  4. import the db from step 2 above
  5. run updates

I'll then try the above steps, but with a vanilla Backdrop + the changes from this PR.

@keiserjb
Copy link

The site I did this with did not use the Entity Cache module.

Under normal circumstances I would not try to upgrade a database after installing Backdrop. When making the video I wanted to try this in my cloud dev environment. It spins up the site as a vanilla install. To do everything in the cloud I needed to overwrite the database. I also did this upgrade a number of times by dropping all the tables in the database inside phpmyadmin and then importing anew. That took longer though.

My normal workflow would be to do the upgrade locally with Lando and not install vanilla Backdrop first. Just import the database that I wanted to upgrade. That process would drop the database before importing and therefore not require manually dropping tables.

@klonos
Copy link
Member Author

klonos commented Dec 12, 2021

I have just tested this on a vanilla Backdrop installation vs one patched with the changes in the PR. The db update errors are gone 👍🏼

(separate follow-up for the VIEWS_STORAGE_DEFAULT error: #5390)

image

I've updated the issue summary with steps to reproduce while at it.

@laryn
Copy link
Contributor

laryn commented Dec 16, 2021

Thanks @klonos for reporting and testing, and @herbdool for the fix. Thanks to @keiserjb and @indigoxela for input as well. I've merged backdrop/backdrop#3846 into 1.x and 1.20.x.

@jenlampton jenlampton changed the title D7 upgrades: cache_entity_* tables causing issues unless dropped before running update.php Fixed: cache_entity_* tables must be dropped before running update.php when upgrading from Drupal 7. Jan 16, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

8 participants