Cockroach: CDB 1.1, Drop Database with Restrict by default is breaking migration tools

[tlvenn]

With CDB 1.1, the DROP DATABASE which was using CASCADE by default, now use RESTRICT so you must use DROP DATABASE xxx CASCADE to effectively drop a database that is not empty.

I understand the intention behind this change but at the same time, this breaks pretty much all the migration tools out there...

With Elixir / Ecto, I have no easy way to customize the statement that is being used to drop the database.

I suspect the same will happen with any other migration tools and I cant event submit a patch to Ecto so that we use cascade explicitly because it's not supported by Postgresql itself. The only option would be to fork the Postgresql adaptor and maintain it :(

I remember I think that @bdarnell voiced to have the same behaviour both in the CLI and with a connection directly but I wonder if it would not be better to only use RESTRICT by default in the CLI as to avoid any manual mistake but keep the CASCADE default otherwise. That might be a middle ground that is providing additional value and safety and at the same time, does not break migration tools.

[bdarnell]

Hmm. There's another wrinkle here, which is that CockroachDB has somewhat muddled terminology when it comes to databases and schemas. Our DROP DATABASE is equivalent to PostgreSQL's DROP SCHEMA, which does have the RESTRICT/CASCADE modifier (and defaults to RESTRICT).

For some reason I thought that postgres didn't have a DROP DATABASE command at all and this functionality was performed through the external dropdb command, so we'd only be changing a command that already deviated from postgres. Since postgres does in fact have this command, it seems like it's probably best to make it cascade-by-default. However, since it's almost time to release 1.1.0, this will probably have to wait until 1.1.1.

I still generally don't like having different behavior between the CLI and other interfaces for things like this. For delete/update without a where clause, truncated queries are a plausible risk that are worth this discrepancy, but I think it's unlikely that someone would type DROP DATABASE and not mean to drop the database (whether it's empty or not). The best way to prevent accidental drops is by making them reversible (move to trash) or restricting permissions, not adding a "yes I mean it" keyword to the command.

[knz]

I think we can compromise and have:

1) DROP DATABASE be CASCADE by default
2) if neither CASCADE nor RESTRICT is specified, then error out if sql_safe_updates is also set on the session, which the interactive shell sets by default.

What do you think?

[jordanlewis]

💯 on this - we absolutely must restore compatibility here and I think @knz's suggestion is good.

[knz]

on it

[knz]

@jseldess could you please add a note in the docs for DROP DATABASE that we're reverting the default back to where it was, and that there's a known limitation in 1.1 about this which will be fixed in 1.1.1?

[jseldess]

Opened a docs PR: https://github.com/cockroachdb/docs/pull/1996