Fix Schema Division Handling #955
Conversation
This is useful if you are recreating the same client without having deleted the previous schema.
Dropping privileges when using a single database user across all tenants will attempt to drop all items in the main database and other client schemas as well.
mikebronner
changed the title
| @@ -54,6 +54,6 @@ public function updated(Updated $event, array $config, Connection $connection): | |||
|
|
|||
| protected function dropDatabase(IlluminateConnection $connection, array $config) | |||
| { | |||
| return $connection->statement("DROP SCHEMA IF EXISTS \"{$config['schema']}\""); | |||
| return $connection->statement("DROP SCHEMA IF EXISTS \"{$config['schema']}\" CASCADE"); | |||
There was a problem hiding this comment.
Using CASCADE is quite a big danger it, as far as my PSQL knowledge goes, this would delete everything in the schema. I think it would be better to use RESTRICT if we should change this at all. I think RESTRICT is even the default.
If we change this, we could risk people ending up deleting entire schema's while that's something that they would not want.
There was a problem hiding this comment.
My thoughts on this method is that we shouldn't be executing this code if our intent is not to drop the schema. And if our intent is to do so, why wouldn't we follow through with it? The problem I ran into when not adding the CASCADE is that the schema would never be dropped automatically, because the tables it contained were never dropped, as I disabled blanked removal of privileges (see change above), which is very risky.
| if (config("tenancy.db.tenant-division-mode") === Connection::DIVISION_MODE_SEPARATE_SCHEMA) { | ||
| return true; | ||
| } |
There was a problem hiding this comment.
Can you give a little insight into why we should not do this?
There was a problem hiding this comment.
In my use-case I have only one database user, that is the main database user used on the System connection as well as all other Tenant connections. Removing privileges for a user would remove all the items they owned, regardless of schema they were in. That is dangerous, and in my case broke functionality.
This PR aims to fix some issues with with schema division: