search

cakephp / phinx

PHP Database Migrations for Everyone
https://phinx.org
MIT License
4.47k stars 891 forks source link

Allow us to change `protected string $schema` in SqlServerAdapter #2304

Open MichaelDesignamite opened 2 months ago

MichaelDesignamite commented 2 months ago

Currently the SqlServerAdapter has this property that cannot be changed:

protected string $schema = 'dbo'

Please allow us to change this.

MasterOdin commented 2 months ago

Postgres has the following to override using the public schema:

https://github.com/cakephp/phinx/blob/a093a9635ca8882ae3edc4b8d5da4d4fa750481f/src/Phinx/Db/Adapter/PostgresAdapter.php#L1548-L1553

Can add a similar function to SqlServerAdapter and anywhere it uses $this->schema, then use $this->getGlobalSchemaName() instead. Though, I think really better to still have the $schema class property and instead override it as part of setOptions, vs having this function that's called every time.

MasterOdin commented 2 months ago

Hm, actually, won't be quite as simple, as I don't think sqlserver has the same concept as postgres in terms of being able to set a search_path that affects all schema-less references following it. We'll need to instead make sure that we're always quantifying any table name references with the schema, which is definitely a going to be a painful process to implement.

I suppose we could minimally allow changing the $schema variable and it'd affect the 2 places it's referenced (in dealing with column comments), and then otherwise just advise end users to create a new user and set the default schema on the user. There's probably potential for things still breaking, but :shrug:

I think there's also potential bugs where if you have the DB objects with the same name in multiple schemas, then phinx may return data in unexpected ways from some functions (e.g. getColumns).