FIX: Schema flushing and model schemas#497
Conversation
|
@microsoft-github-policy-service agree |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run |
|
Commenter does not have sufficient privileges for PR 497 in repo microsoft/mssql-django |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
There was a problem hiding this comment.
Pull request overview
This PR extends the mssql-django backend’s support for non-default SQL Server schemas, aiming to ensure models using schema-qualified db_table names behave correctly in migrations, flushing, and introspection.
Changes:
- Add schema-aware FK constraint disabling/enabling during
flush. - Add schema-qualified table handling in introspection (table listing and per-table queries).
- Create missing schemas automatically during
CREATE TABLEmigrations, plus add test coverage/models/migrations for non-dbo schemas.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 5 comments.
Show a summary per file
| File | Description |
|---|---|
mssql/schema.py |
Adds auto-creation of schemas during create_model() and introduces sql_create_schema. |
mssql/operations.py |
Updates sql_flush() to include TABLE_SCHEMA so constraints are altered with schema-qualified table names. |
mssql/introspection.py |
Updates introspection to return schema-qualified table names and to handle schema-qualified inputs for several introspection methods. |
testapp/models.py |
Adds test models with db_table set to [events].[...] and [dbo].[...]. |
testapp/migrations/0026_parentschema_childschema.py |
Creates ParentSchema/ChildSchema tables in the [events] schema. |
testapp/migrations/0027_create_dboschema.py |
Creates DboSchema table in the [dbo] schema. |
testapp/tests/test_schema.py |
Adds tests intended to validate flushing and introspection with non-default schemas. |
| # map pyodbc's cursor.columns to db-api cursor description | ||
| _schema_name, table_name = get_table_name_with_schema(table_name=table_name) | ||
| columns = [[c[3], c[4], c[6], c[6], c[6], c[8], c[10], c[12]] for c in cursor.columns(table=table_name)] | ||
|
|
| TABLE_TYPE, | ||
| CAST(ep.value AS VARCHAR) AS COMMENT | ||
| FROM INFORMATION_SCHEMA.TABLES i | ||
| LEFT JOIN sys.tables t ON t.name = i.TABLE_NAME |
| sql_create_schema = ( | ||
| "IF NOT EXISTS (SELECT 1 FROM sys.schemas WHERE name = '%(schema)s') BEGIN " | ||
| "EXEC('CREATE SCHEMA [%(schema)s]') " | ||
| "END" | ||
| ) |
|
|
||
| sql = self.sql_create_schema % { | ||
| 'schema': schema_name.strip('[').strip(']') | ||
| } | ||
|
|
||
| self.execute(sql, None) | ||
|
|
| def test_inspectdb(self): | ||
| connection = connections['default'] | ||
|
|
||
| descs = connection.introspection.get_table_description(cursor=connection.cursor(), table_name='[events].[ParentSchema]') |
| constraints.insert(0, composite_pk_sql) | ||
|
|
||
| # Make the table schema | ||
| if '.' in model._meta.db_table: |
There was a problem hiding this comment.
. detection can be very error prone and confusing. There are a lot of edge cases that need to be considered. Table names can have . in them.
Our drivers for mssql server have a multi part identifier parsing. Once instance is https://github.com/microsoft/mssql-rs/blob/e374c4a743af9a45dfee29762afaeddaee96408b/mssql-tds/src/sql_identifier.rs#L46
We need to use a statemachine based parser instead of . detection to get this right.
| schema_name, _ = model._meta.db_table.split('.', 1) | ||
|
|
||
| sql = self.sql_create_schema % { | ||
| 'schema': schema_name.strip('[').strip(']') |
There was a problem hiding this comment.
Sql schema names can have ] in them too.
CREATE SCHEMA [odd]]schema]This code can incorrectly strip out wrong information
| # is supported | ||
|
|
||
|
|
||
| if '[' in table_name and ']' in table_name and '.' in table_name: |
There was a problem hiding this comment.
This requires a multi part parser. [ ] and . are valid characters of tablename
| types = {'BASE TABLE': 't', 'VIEW': 'v'} | ||
| if VERSION >= (4, 2) and self.connection.features.supports_comments: | ||
| return [TableInfo(row[0], types.get(row[1]), row[2]) | ||
| return [TableInfo(f'[{row[0]}].[{row[1]}]' if row[0] != 'dbo' else row[1], types.get(row[2]), row[3]) |
There was a problem hiding this comment.
Why this condition?
if row[0] != 'dbo'
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
|
@benclark158 - thanks for your contribution! |
Changes
Meta.db_table = [schema].[table_name]flushcommand to work as expected for models with non default schemasI have added tests for tables with different defined migrations as well as flushing
Fixes: #496