When doing an sql-sync from A to B,
if db B has tables that are not present in db A,
those tables are still there at the end of the sync

So you don't end with A == B.

Kudos for drush + site aliases + sync tools. Awesome power.

Support from Acquia helps fund testing for Drupal Acquia logo

Comments

moshe weitzman’s picture

Status: Active » Closed (works as designed)

IMO, this is by design. We can't be deleting random tables from a DB. If you want this, delete all tables first or drop and recreate the db. sql-sync can do the create-db part (and perhaps should also do the DROP)? if you agree, reopen this with new title.

yched’s picture

Title: sql-sync leaves extraneaous tables in the destination db » add a --delete-all-tables-fisrt option for sql-sync ?
Status: Closed (works as designed) » Active

Well, the issue is that in some setups, the SQL user in settings.php doesn't have the create db grants.
I ended up doing a

TABLES=$($MYSQL -e 'SHOW TABLES' | awk '{ print $1}' | grep -v '^Tables' )
for t in $TABLES
do
 $MYSQL -e "DROP TABLE $t"
done

in a wrapper script around db-sync.

Would be kinda cool if db-sync command had an option to force clean db ?

yched’s picture

Category: bug » feature
moshe weitzman’s picture

there is a db-su option which will be used for create-db operation but it is not fully implemented in mysql. i think thats what you need. see #683844: db-su has no password passing feature?. not sure if create-db does a DROP first. It probably should (with confirmation?)

greg.1.anderson’s picture

Yes, --create-db will drop first.

moshe weitzman’s picture

Status: Active » Closed (duplicate)

ok, this is a dupe of the db-su issue then

yched’s picture

The issue with drop / recreate db is that you lose user grants that were set on the db.
That's why I went with the "drop all tables" code in #2 in my custom wrapper script.

moshe weitzman’s picture

Component: Code » PM (dl, en, up ...)
Assigned: Unassigned » moshe weitzman
Status: Closed (duplicate) » Active

I'm working on this.

moshe weitzman’s picture

Committed a sql-drop command. Still needs integration into sql-sync. I will do that soon. If anyone has an opinion on how/where to do that in the flow, let me know.

greg.1.anderson’s picture

Here are some thoughts. This problem is not as easy as it might first appear due to the fact that the target db might be remote. In that instance, to get the list of table names you would need to run a remote drush command to get the sql needed to drop the tables, because the tables in the target might be different than the tables in the source. The other problem you might have is that the list of table names could get really long, maybe (if the db had other stuff in it), and you only get 1000 chars or so on the command line before you overflow bash's line buffer. If that isn't an issue, the remote function could just be a 'drush sqlq "show tables;"', which is easy enough to run. You could then only use --drop remotely if there was an instance of drush on the remote machine, though.

Perhaps a cleaner solution execution-wise, but dirtier code-readability-wise would be to use mysql and postgres functions to iterate over the tables and drop them. Then you could send the sql function to the remote machine. I could write that fn in postgres, but don't know if it's possible in mysql. If this works, then it has another advantage of not requiring a remote drush.

Apparently, in Postgres you can do this:

DROP SCHEMA public CASCADE;

However, you need higher privs (similar to create db) to do that. If you have permissions, though, the cascade of the drop of the public schema will take all of your tables along with it. There's no shortcut like that in Mysql, at least not that I know of.

Someone used this technique to get the list of table names in mysql:

mysqldump -u[USERNAME] -p[PASSWORD] --add-drop-table --no-data [DATABASE] | grep ^DROP | mysql -u[USERNAME] -p[PASSWORD] [DATABASE]

Something similar could be done with postgres, or maybe use the drop schema public for postgres and the above trick for mysql? The advantage of this technique is that you can use it to get a sql script that drops all of your tables, and you could insert that code above into the bash script that you send over to the remote machine via ssh. So, you won't run out of command line chars, and you don't need a remote drush for it to work. This is perhaps the best approach; there may be other options as well.

greg.1.anderson’s picture

Update on the above: I think that using pg_dump --clean --schema-only | grep ^DROP | psql is better than droping the schema, since it requires fewer permissions.

Edit: Fixed small typo.
Edit again: Said --data-only when I meant --schema-only; fixed that too.

moshe weitzman’s picture

I think grep would be a new requirement for us. It a reasonable requirement, but each one is a burden for Windows folks I think. Lets do that in php instead.

greg.1.anderson’s picture

sql-sync does its remote operations via ssh + bash, so that drush does not need to be installed on the remote machine. I agree that filtering with php is probably best if the destination database is local, though.

greg.1.anderson’s picture

If you're not already working on a different solution, I'll put together a patch that uses php when the destination is local, and inserts code into the ssh / bash script when the destination is remote.

moshe weitzman’s picture

I'm getting a little uncomfortable with commands doing operations differently depending on how local vs. remote. We just did that with sql-sync sanitizing. My tentative vote is to just use grep everywhere.

moshe weitzman’s picture

But yeah, would be great if Greg took this on.

greg.1.anderson’s picture

Status: Active » Needs review
FileSize
15.82 KB

Here is a patch. I tried about four things on postgres, and settled on my second favorite solution, on account of the fact that my favored solution used 'sed'.

moshe weitzman’s picture

Version: All-versions-3.0-alpha1 »
Assigned: moshe weitzman » greg.1.anderson

Not sure this was supposed to be commented out. search for : // mysqldump

We should keep sql-drop as a separate command. Therefore, we should invoke that command from within sql-sync, IMO. Maybe in a COMMAND_PRE hook.

moshe weitzman’s picture

The approach looks good to me.

greg.1.anderson’s picture

Per #16, I used the same approach for local and remote target sites. If you prefer, I could invoke sql-drop for the local case (but probably not in the pre hook; that comes before the confirmation...)

moshe weitzman’s picture

Title: add a --delete-all-tables-fisrt option for sql-sync ? » add a --delete-all-tables-first option for sql-sync ?

I'm interested in enhancing sql-drop so that it does not require drush on the remote side. Doing so we would require us to refactor the dump+grep solution you have here.

greg.1.anderson’s picture

Hmmm. I had a similar idea to make an sql-import command that handled the remote half of sql-sync, and also moving the remote handling of the dump in sql-sync to the sql-dump file command. So we would then have sql-dump @a, sql-drop @b and sql-import @b, where @a and @b may be either local or remote, and sql-sync just dispatches to those three functions as needed.

What do you think of that? Should we add sql-create to round out the set?

moshe weitzman’s picture

That sounds really good to me.

sql-create makes sense as well. site-upgrade passes along --createdb so that might need a touch of refactoring afterwards. i can take care of that.

greg.1.anderson’s picture

Status: Needs review » Needs work

Sounds good.

greg.1.anderson’s picture

sql-sync is too tangled; I don't have time to factor out sql-dump and sql-import right now. This is also something better accomplished after unit tests, which means maybe post-drush-4 stable release (depending on how d7 stable is going...).

I will tackle refactoring dump and create, though.

dude4linux’s picture

+1 for making --drop-all-tables an option for sql-sync. I just got bit while transferring a site to my test server which had previously been used for other testing. I had just assumed that sql-sync would drop all tables and start with a clean database. Later I happened to notice that I had 110 tables instead of the desired 94. Fortunately it was discovered soon enough that I didn't waste a lot of effort.

After giving it some thought, I realised that in some cases the current behaviour is preferred. At some point, after upgrading my site, I'm going to want to move the content to a TurnKey Linux Drupal appliance. It has a pre-installed Drupal setup so I think I only want to overwrite tables that exist in the source while keeping those extra tables that belong to other modules in the destination.

Thanks for all the work you guys have been doing to enhance drush.

greg.1.anderson’s picture

@dude4linux: I would recommend caution in your plan to merge together two Drupal sites. It is very important that your code and database match up exactly. If you have a working Drupal site, it would be safer to blow away all of the pre-installed stuff, files and db both, and replace it with your site. Back up the pre-installed stuff first in case you want it back. If the pre-installed stuff includes different modules than you have on your side, then use drush to dl and enable them. If you do go ahead and merge two sites together, then do the merge in a dev site, test it, and then sync it back to live after it works.

dude4linux’s picture

@greg.1.anderson: Don't worry, I'm being very careful. Have backed up my live site to a development server. After I've completed upgrading from 5.23 to 6.19 and have tested the new site, I plan to move it to a TurnKey Linux Drupal appliance. I will make sure that all the same modules/versions are installed on the appliance so all I need to do is move the database, /files, settings.php, etc. The appliance has additional third party modules installed but not enabled. I don't see any reason why I would need to remove the modules and then replace them with the exact same files. When I'm finished, I'll just redirect traffic to the new host and shutdown the current site.

colan’s picture

Subscribing.

greg.1.anderson’s picture

Status: Needs work » Needs review

I'm not going to have time to do the refactor for drush-4. Should I submit an update of just #18 + #19 to include in this release, or shall we punt?

I would kind of like to have the option to drop in drush-4, even if the code will need a little refactoring in the future. The basic implementation in #18 is sound.

moshe weitzman’s picture

I think we should punt. I don't feel strongly about it, but thats my intuition.

greg.1.anderson’s picture

One reason for including it is --structure-tables is destructive without --drop. You can work around that limitation with cc all if the tables in your --structure-tables list are all caches that are cleared by cc. That is probably the case most of the time.

Is there a slick way to figure out what tables cc is going to clear (w/out running it and testing the dump)? Something like drush cc all -s (except that doesn't work, because the cc code is in Drupal).

Anyway, I'll presume we're going to punt unless the above changes your opinion in some way. I don't have a strong opinion about it.

moshe weitzman’s picture

there is no way to get that table list unless you do some real gymnastics like change the DB connection to a no op driver that logs statements. D7 only.

moshe weitzman’s picture

Priority: Normal » Major
Status: Needs review » Needs work

Lets get this into drush5. Upping priority.

Senpai’s picture

A --drop-all-tables flag could be *very* useful for importing a database using a single command such as:
drush sqlc < db_backup.sql
because often, new or changed tables that aren't in the backup, i.e. 'broken' dev work that needs to be destroyed and reverted, get left behind during a restore such as this.

Will this proposed --drop-all-tables flag also "just work" in the above situation, or do I need to open a separate issue for this concept?

kenorb’s picture

http://build2be.com/content/dumping-all-tables-drush

echo "\
SELECT concat('drop table ', table_name, ';') \
  FROM information_schema.tables \
  WHERE table_schema=schema()" \
| `drush sql-connect` \
| grep "^drop table " \
| `drush sql-connect`
greg.1.anderson’s picture

Yes. Drush also has sql-drop now, which works well; it would not be too hard to call it with a --drop flag in sql-sync. Note that if you have database create permissions, you can use --create-db to clear out your database contents.

moshe weitzman’s picture

FYI, there is also drush_sql_empty_db($target_alias) now.

cristian.stoica’s picture

sql-drop doesn't drop created VIEWS from the database.

greg.1.anderson’s picture

#1449056: sql-sync should (by default) delete target db suggests that --drop should be the default.

greg.1.anderson’s picture

In contrast, #1402854: site-install deletes too many tables desires that dropping all tables should not be the default for the site-install commands. Others might feel similarly about sql-sync if we made --drop the default here too. I tend to think that the less destructive option should be the default here.

geek-merlin’s picture

hmm, thought a bit about this and, really, seems there currently is no general way to know which tables are "ours".
we would need a "table of drupal owned tables" from core to tell.

Elijah Lynn’s picture

Another vote for:

drush sql-sync --drop @dev @loc

However, thanks for sql-drop, much appreciated. Makes me wonder how many people expect it to drop by default, I spent quite a bit of time thinking it did, like a year or so...

greg.1.anderson’s picture

Version: » 8.x-6.x-dev
Status: Needs work » Needs review
FileSize
2.31 KB

Here is a patch that implements --drop for sql-sync.

moshe weitzman’s picture

If we commit #45, we are making --drop dependant on having drush on the target side. Thats a new requirement, right? Ideally we would refactor sql-drop to not require drush. I guess the proposal is to get some new functionality and accept the new limitation?

greg.1.anderson’s picture

Yes, that is a new limitation, but it only applies if you are using --drop. If you are using sql-sync to pull a remote live or stage site to a local dev (typical), then Drush will be available de-facto.

It would be easier to do this in a Drush-independent way if we used techniques from #990812: Add a "permissions" subcommand to fix/set all file permissions in sql-sync, sql-drop, sql-create, etc., to more easily manage chains of commands to execute on a local or remote machine. I'd recommend committing this and doing a Drush-independent version later, but it would also be reasonable to just wait; it's not too hard to run sql-drop before sql-sync. The main thing that this patch adds is the ability to set 'drop' to true in a command-specific record in drushrc.php, for folks who want Drush to always drop before sync.

moshe weitzman’s picture

Status: Needs review » Needs work

When folks ask "does sql-sync require drush on the remote side", I want the answer to be an un unqualified "yes". So, deferring this until we can improve sql-drop.

I hope that any upcoming sql-drop is less voluminous than the code in #990812: Add a "permissions" subcommand to fix/set all file permissions.

greg.1.anderson’s picture

_drush_sql_drop is already a full page of php code that does different things based on the schema to first find the number of tables, and then run an sql drop on each table. My impression is that it would be quite a bit longer if we changed it to emit the bash commands required to drop all tables for any database schema.

moshe weitzman’s picture

FYI, some of this code was cleaned up when we introduced wildcard support for structure-tables and friends.

greg.1.anderson’s picture

Status: Needs work » Closed (won't fix)
Issue tags: +Needs migration

This issue was marked closed (won't fix) because Drush has moved to Github.

If this feature is still desired, you may copy it to our Github project. For best results, create a Pull Request that has been updated for the master branch. Post a link here to the PR, and please also change the status of this issue to closed (duplicate).

Please ask support questions on Drupal Answers.

R.Hendel’s picture

I created a new issue "sql-sync (and other operations) won't work after sql-drop" at @ github for this.
There is another issue @ github "sql-connect does not work if used after sql-drop" which is very similar and which is caused in my opionion by same reason.