On restore, provide option to drop all tables

I do a DROP plus CREATE DATABASE prior to restoring, which seems to work out exactly the same as deleting each table (and view, and everything of course :) separately in MySQL (namely, database permissions remain as before). Also works in PostgreSQL if I recall correctly.

It would be great if B&M had an option to do this (or something equivalent) automatically on the advanced restore options, perhaps even on by default. I'm sure many people don't realize they are accumulating orphan tables every time they play with poorly coded modules or create test tables of their own, and then revert.

Hmmm, I'm pretty sure MySQL doesn't automatically removes permissions, by design:

12.1.21. DROP DATABASE Syntax

When a database is dropped, user privileges on the database are not automatically dropped.

...

12.4.1.3. GRANT Syntax

In standard SQL, when you drop a table, all privileges for the table are revoked. In standard SQL, when you revoke a privilege, all privileges that were granted based on that privilege are also revoked. In MySQL, privileges can be dropped only with explicit DROP USER or REVOKE statements or by manipulating the MySQL grant tables directly.

This matches my own experience. What privileges did you had to recreate?

As for identifying orphan tables, you could simply use the Data module (Site Building > Data tables > Adopt tables).

The following patch creates a whitelist of all tables available during backup. On restore all other tables are dropped.
If the general solution is accepted, an option in the interface should be added...

Thanks, @frenkx, that seems like a great approach—and it works for me!

By the way, patches should be created from the module directory rather than the site document root (e.g. from sites/all/modules/backup_migrate/). Because I keep my contributed modules in sites/all/modules/contrib/, your patch didn't apply cleanly for me. Also, if you use the method in the Advanced patch contributor guide, your patch will contain your d.o username to ensure proper attribution to you.

So (at least) these things remain to complete this issue:

1. We should get a few more eyes on the patch to validate the approach. (I'm no MySQL guru.)
2. We need to do something for non-MySQL databases
3. We need to add an option to the interface to turn this feature on or off. We might also consider a Drush switch.

Thanks for getting this off the ground, @frenkx!

Subscribing - I am using 7.x-2.2, and I created a new field, before deciding to restore my database through Backup and Migrate. I tried creating the field again, but it said the table already existed (even though the field wasn't showing in the content type's field list). I couldn't figure out why the field's table was still there after I did the restore, until I realized it wasn't being deleted by Backup and Migrate.

susbscribe - This would be really useful

Subscribing - much needed option during the dev phase or when playing with modules with no uninstall hook.

There are lots of times when restoring a DB can get you into trouble if tables are sitting in there that were created later. For instance naively written update hooks will fail when reenabling modules. I think this is a big issue, that I am dealing with again today in a project. Please make backup and migrate behave like a full restore!

This is a bug, as it can cause all sorts of problems with installing modules after restoring a backup.

Eg:

- take a backup
- enable module foo, which adds its tables
- mess up the site in some way, so restore from backup
- try to enable module foo again.

On D6, an existing table would only cause a minor complaint, on D7 it's a big error that seems to have lasting effects.

This causes real problems when developing modules and testing their installation procedures.

Restoring will delete some or all of your data and cannot be undone. Always test your backups on a non-production server!

The obvious expectation is that one can restore without extraneous data. I tried searching the B&M the interface, expecting to find the option not to retain tables. Is an option even required for this?

The example @joachim gave is a good one. Would be good to see this issue dealt with! Thanks!

I tested the patch from #4, and while this is a great start it does have some potential issues to be aware of.

1. My database is case-sensitive. Therefore when I ran the backup and checked the file, the drop command didn't fill in the table names properly.

SELECT @SQL:=CONCAT('DROP TABLE IF EXISTS ', group_concat(TABLE_NAME), '; ') FROM information_schema.tables WHERE table_schema = DATABASE() AND table_name NOT IN ('', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '', '');

To resolve this, I updated the patch with the following change:
- create_function('$a', 'return "\'".$a["Name"]."\'";'),
+ create_function('$a', 'return "\'".$a["name"]."\'";'),

2. The SQL statements as written does not drop all tables if there are lots of extra tables. It does drop some, but not all. Repeating the import drops a few more, and if you repeat the restore enough times, sure enough all of the extra tables will drop. Researching the group_concat command, I found there appears to be a limit of 1024 chars by default. See http://dev.mysql.com/doc/refman/5.0/en/group-by-functions.html#function_.... How long this needs to be will depend on the number of tables and how long the table names are.

Possible resolution: During a restore, aren't all tables effectively deleted and recreated? If so, does the script need to indicate which ones not to drop? I may be missing something, but couldn't the script just drop all tables in the database, then the existing script would recreate the ones needed?

hmm, OK here a short summary:
* problem is that sql restore only inserts tables but does NOT drop pre-existing tables (which causes major WTF when eg creating fields that conflict with such leftover tables)
* patch #4 inserts some very smart code into the sql dump that drops all tables that wont get imported (but leaves the others, to be dropped immediately before import). said patch has some issues.

in #12 it is proposed to drop all tables before importing.

here is a patch onto current dev which does this and worksforme.
note that the dropping is done on restore (contrary to #4) which has implications:
* dropping old files does also work with legacy sql dumps
* just piping an export into mysql does not drop other tables
in the end i think dropping prior tables is the job of the importer (not of the dumped sql) so this is the right approach. ymmv.

this raises 2 questiond imho:

there seems to be no wrapping of the import in a transaction so if something fails on the way you are doomed.
but this seems tobe an issue with or without this patch. (or does someone know better??)

as for making this an option:
i propose:
* not make this an option as it is expected behavior to have no "leftovers"
* for those who have some non-drupal-tables that must be saved on a restore (some people wanted this in a similar drush issue) we might add a hook or option "tables protected from restore" or so - but thats outside the scope of this issue.

OOPSALA - wrong patch, ignore the last one and use this.

Attached patch for 6.x-2.5. (Wrong patch see below).

Patch for 6.x-2.5.

• uses db_query
• $a["Name"] instead of $a["name"] in table defs

Thanks, @axel.rutz. The patch in #14 works for me. I still think this needs to be optional behavior, though. If somebody's running a multisite installation or is using a CRM alongside Drupal or sharing the database in any number of other fairly common scenarios this could bite them in a big way. I think this needs to be opt-in behavior with a clear warning message. That probably means a web interface item and a Drush switch.

I think this needs to be opt-in behavior with a clear warning message.

@TravisCarden - I think this needs to be opt-out behaviour, i.e. enabled, set to destroy all tables by default. And in terms of a clear warning message, I quoted the existing one from the B&M module which has been in there for ages, but perhaps just needed this patch to follow through with its implications. Isn't this enough?

Restoring will delete some or all of your data and cannot be undone. Always test your backups on a non-production server!

It's a bit vague the purpose of retaining tables during a restore, opposed to leaving the functionality simple and the responsibility to the user. If more complex scenarios could be described, that might contribute to the pursuit of this as configuration, particularly as an option in the menu more importantly before it becomes a Drush switch. Anyone able to elaborate on these situations?

Did you read my comment in #17 and find it unclear, @Rory? What I'm trying to point out is that Drupal is designed to be able to share a database with other applications—even other Drupal sites. This patch would cause BaM to unquestioningly obliterate any such applications when restoring a backup. If it is the obvious expectation that restoring a backup won't leave behind unused tables, is it not a much more obvious expectation that it will not erase your CRM, multisite Drupal installation, third-party forum, or other completely unrelated application? This is a massively destructive operation with significant implications.

No, I don't think the existing error message is enough, because it's not adequately clear that the "some or all of your data" is not just your Drupal site but anything that shares the database with it. That merits a special callout. (If there's a checkbox to drop unused tables, that would be the perfect place for such a callout.)

We can still consider other approaches, too. For example, we could prompt the user on restore if there are tables in the database that are not present in the backup and and let them select which ones to delete. Or we could let the user select tables ahead of time to preserve, just like they already select tables to include in backups. I just think we need to think this through and be careful. We're talking about a contingency that could not only completely ruin someone's impression of Drupal but cause them tremendous data and financial loss and even lawsuits under certain circumstances.

I totally agree this behavior should not be enabled by default. When picking between two outcomes, each of which may surprise people, pick the one that is the least destructive. I also like your suggestion to somehow recognize which tables were left after the restore and provide a quick way to drop them (perhaps excluding common ones like cache_*, sessions and watchdog).

@TravisCarden - thanks for elaborating on the problem and considerations to take in, in addressing it. You've done this very well. If these ideas you've offered could be tackled, that would be so much better than what I was suggesting (which is to make the module more rudimentary so its scope be understood). Hopefully some time could be put into this as this kind of support would be very promising for B&M and Drupal in general.

It is generally assumed that BAM does a full backup and a full restore, without leaving artifacts, so I think this issue is high priority. I just got stuck trying to debug a Features deployment due to fields that already existed (they were left by BAM).

I understand the issue about permissions. Why not iterate through all tables in the DB and drop all? Something like: http://dba.stackexchange.com/questions/4984/clear-all-tables-with-one-de...

@TravisCarden: Just curious, does BAM do a backup of ALL tables, even those that aren't created by Drupal? If so, then I assume BAM would restore all of these tables as well?

BTW: For anyone who doesn't know basic SQL, for mySQL you can just add:

drop schema [yourdatabasename];
create schema [yourdatabasename];
use [yourdatabasename];

To the beginning of the .mysql file generated by BAM and then run the script manually and it will entirely remove all tables first and then recreate the tables using the BAM generated query. Be careful, this will drop (entirely) whatever db name you enter, so get it right!

Re @RogerRogers #21: BAM's default behavior is to back up all tables in the database. (You can exclude them arbitrarily.)

patch from #14 committed to the dev ?

#14 worked for me.Thanks.

#14 works good, but I also think that this must be an optional functionality, disabled by default.
A check box saying "Drop all tables before import" that can be tickable at the time of the import will be a good, for example.

But I don't think this is a bug.

Can I suggest changing the title to On restore, provide option to drop all tables.

Also see #386320: Option to drop database and then restore...

Ok, let's try it.

Attached patch is an update of #14, making the option optional (and off by default).
Also added in the description that this is a MySQL-only option. And minor cosmetics.

Patch should apply cleanly to 7.x-2.x

I agree that it should be OPT-in and that patch looks fine.

However, here's how I would like this to work:

Rather than drop all tables before the restore, I would like the module to keep track of which tables are in the import and then drop the tables which where not restored after the restore is done. That way if something happens during the restore you won't be left with an empty database with no way to recover. This will be a bit more code, but I think the risk of dropping the active db during a bootstrap seems to great to me.

Why the whole import process is not inside a transaction?

Just asking, I don't know how this was built

I agree with #30 from Ronan, "drop tables which where not restored after the restore is done."

I find the need to use the Schema module together with Backup and Migrate (BaM) for restores.

Immediately AFTER a BaM Restore.

1) check Schema > Compare > EXTRA (#) >>
2) from phpMyAdmin select the database >>
3) select extra tables (left column check boxes) >>
4) use "With Selected" drop down to choose "Drop Tables", then GO.

A phpMyAdmin method but should help those not adept with module patches, SSH commands mysql dumps, BASH scripts or GIT.

RE #30

If the main issue is that you do not want to drop all the tables and be left with an unusable drupal install then we could drop the tables that are not specifically registered in schema. That will leave us with a half broken site but should leave the admin section in-tacked.

In my experience this causes problems with field tables more than anything. Maybe we should remove all the field tables if we are attempting to be non-destructively destructive.

I really don't think it matters much if we do it before or after, a botched migration is never going to leave you at 100%.

@maynardsmith No need to comment in order to subscribe any longer. Click the "Follow" button at the top of the page instead, in order to prevent e-mailing everyone following the thread.

Patch #29 works fine with 7.x-2.8 in a multi site environment! Thank you Pere Orga.
Changed this issue to 'needs review' (is that permitted...?) and priority 'normal', with the question to update this to 7.x-3. (and 8).

Just ran into this issue today while developing features on a local dev environment. New tables added by the feature were not dropped when I restored from my pre-feature backup, requiring manual intervention in the database to continue development. Would love to see this in an upcoming release.

Patch #29 doesn't apply to 7.x-3.1.

We will want it to apply to 7.x-3.x not to the 3.1 tag.

Currently we are waiting for review of this to get into 2.x but since that version isn't supported anymore I think we should just move strait to 3.x

It doesn't seem this can directly be used with Drush, or am I wrong?

drush bam-restore db manual [...].msql.gz --utils_drop_all_tables

This may need to be rewritten based on the newest 7.x-3.2 release.

As per comment #39. This needs to be rerolled in the 7.x-3.x branch.

#41:
> This may need to be rewritten based on the newest 7.x-3.2 release.

Please set to needs-work if you *checked* this does not apply anymore.
Otherwise you just force other people unnecessary work which is not a nice thing.

Also "3.2" spreads confusion as rerolls always go onto the HEAD (3.x).

That said: Rock on!

@axel.rutz Thanks for the help. I processed hundreds of old issues in the 7.x branch dating back as much as 10 years at the request of the new maintainer and did the best I could given my inability to code. I have no way to check these patches myself, but we have had other people pitching in and validating after upgrading to 7.x-3.2. Only a few issues had old patches, so I am not going to go back and edit what I have written on those few especially since I was told to set all 7.x patches to "needs work" by @DamienMcKenna here. If you have any desire to assist with this module which is in great need of volunteers, please check out the latest list of priority tasks. Thanks for your input! :) Things should go back to normal in the queue once the backlog is caught up.

So suggestion: A maintainer might want to enable module testing (even if no tests are there) to automate the check if a patch still applies!

I've rerolled the patch to the latest snapshot of 7.x-3.x.
It applies cleanly on my site.

We really need this. Features is one of the projects that produces code that does not uninstall cleanly. When I debug a features project, I need to manually clean up the database every time I do an uninstall/reinstall cycle.

Backup and migrate provides the perfect workaround – except that it does not drop all tables, so the gunk left behind by the features export remains.

With this patch, you can just tick a box to clean out the gunk before restoring.

Please review.

Rerolled the patch in #51 to incorporate some automatic suggestions from codesniffer.

@gisle Thanks for all your help! The maintainers on this module have an extremely limited schedule for devoting to upgrades, but you'll probably see a flurry of activity and hopefully this patch reviewed and committed shortly before a new version is released. It could be months yet, but in the meantime hopefully people who need this feature will see the patch.

Removed the unwanted changes.

Minor code tidyup.

There are some smart quotes in the first comment in the patch, it looks like. They should be ASCII. I'm hoping to test this this week or next.

Fixed the comment.

#61 is working for me.

Actually, create_function() is deprecated as of PHP 7.2.0. To avoid using it I created a callback.

You might want to use a anonymous function for better readability. (But i do not want to bikeshed about this...)

Nah, I'm good with the new function, anonymous functions make code harder to follow.

@jacob.embree: Thanks for the review and improved patch.

Committed.

As someone who has stopped using this for the time being, could someone fill me in on what the current patches do, briefly?

The patch provides an option to drop a table before it's imported.

Will this get ported to D8 in this issue or should this be opened in a new issue.

Please open a new issue and list this one as a related issue. Thanks.

How bullet-proof is this?

niallmurphy-ie wrote:

How bullet-proof is this?

I do not understand this question.

I've used the patch for some time, and - as pointed in #51 - it provides a very useful feature when debugging projects that does not provide a clean uninstall on their own.

It does what Damien says it does in #68.
However, I don't understand how such a feature can be discussed in terms of being "bullet-proof".

I've failed to restore a database with this module at least three times, all because of new tables.

My ass in on the line when it comes to this stuff. So I'm hesitant of a patch.

The feature provided by the patch is "off" by default, and will not affect your site unless you expand "Advanced settings" and enable it before restoring.

I am using this module with this patch for manual and sceheduled backup on a number of production sites, without issues.

However, before relying on anything for backup on a production site, make sure that you test your backups (by restoring to an empty non-producation site) to make sure you've set things up properly and understand how the module works.

I would say, based on the comments on this issue, that at least jacob.embree and DamienMcKenna expect this commit to be stable, but until a stable release of 7.x-3.x comes out with this commit included it is still considered in development. This issue has #2942331: Plan for Backup and Migrate 7.x-3.6 as a parent which indicates that it is likely to be considered stable enough for a stable release, but you may want to wait for 7.x-3.6 to be sure. Please note, however, that even in a stable release, the following is included in the usage license:

This program is distributed in the hope that it will be useful,
but WITHOUT ANY WARRANTY; without even the implied warranty of
MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the
GNU General Public License for more details.

It's also worth remembering that this feature only affects the site when restoring a database.

@niallmurphy-ie maybe you should test it on a non-production environment to make sure it works in your use-case.

Hi, thanks for your replies. And sorry if I sounded unthankful or anything. As of today, backups work fine on my dev environment.

But I have decided against this module as a backup method for a variety of reasons. I prefer it to every other method, but the penalty for any fault is too large with my current exposure.

Thank you for the hard work. I still use this module and do appreciate it.

@niallmurphy-ie, my philosophy with backups is that if you only have two copies you're not backed up. Meaning for a robust DR solution, backup-migrate shouldn't be your only backup --and neither should any single backup solution.

In other words, design your backup plan as though your backup solution will fail.

@frob I love that philosophy. I've heard that "if you don't have 3 copies of data, it doesn't exist."

@frob and the others: Did anyone create the separate issue for the Drupal 8 version yet? (#69)

This is a very handy feature, especially for developers, but I couldn't find it in the Drupal 8 version yet?

EDIT: As I couldn't find one, I created it: #3246003: [WIP] Add "Drop Table" option for restore (like in 7.x)