Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Can't add a field to an existing model without allowing null values when specifing table schema #402

Open
jdshupe opened this issue Jun 24, 2024 · 3 comments

Comments

@jdshupe
Copy link

jdshupe commented Jun 24, 2024

Software versions

  • Django: 4.2.13
  • mssql-django: 1.5
  • python: 3.9
  • SQL Server: 16.0.1000.6
  • OS: Windows 10

Table schema and Model

class Testing(models.Model):
    id = models.AutoField(primary_key=True)
    name = models.CharField(max_length=50)
    age = models.IntegerField()
    testing = models.CharField(max_length=50)

    class Meta:
        managed=True
        db_table = "Bronze].[example"

Database Connection Settings

DATABASES = {
    'default': {
        'ENGINE': 'mssql',
        'NAME': 'Dev',
        'USER': '********',
        'PASSWORD': '*************',
        'HOST': '****************', 
        'OPTIONS': {
            'driver': 'ODBC Driver 17 for SQL Server',
            'options': '-c search_path=Bronze'
        },
    }
}

Problem description and steps to reproduce
When adding a new field to an existing model without specifying a default value or allowing blanks/nulls, the resulting migration errors.

To reproduce:

  1. Create a new django project with mssql
  2. Add a new app to the project and create a model with a field and specify the table using the "schema].[table" notation
  3. Make and apply migrations for the new app
  4. Add an additional field to the model without allowing null values
  5. Make migrations functions fine but then errors when actually applying them

Expected behavior and actual behavior
I would expect the dialog that requests the user to either set a default value or to exit and make the change manually to show and then to properly set any existing lines to have the provided default and then add the column.

Instead, the dialog works and creates the migration but instead of setting the default value and then adding the new column the function errors out saying it was unable to drop a constraint matching the column name because it wasn't found.

Error message/stack trace
executing manage.py migrate
Operations to perform:
Apply all migrations: Example, admin, auth, contenttypes, sessions
Running migrations:
Applying Example.0004_alter_testing_test1...Traceback (most recent call last):
File "\source\repos\Testing MSSQL Django Error\Testing MSSQL Django Error\env\lib\site-packages\django\db\backends\utils.py", line 89, in _execute
return self.cursor.execute(sql, params)
File "
\source\repos\Testing MSSQL Django Error\Testing MSSQL Django Error\env\lib\site-packages\mssql\base.py", line 677, in execute
return self.cursor.execute(sql, params)
pyodbc.ProgrammingError: ('42000', "[42000] [Microsoft][ODBC Driver 17 for SQL Server][SQL Server]'test1' is not a constraint. (3728) (SQLExecDirectW); [42000] [Microsoft][ODBC Driver 17 for SQL Server][SQL Server]Could not drop constraint. See previous errors. (3727)")

The above exception was the direct cause of the following exception:

Traceback (most recent call last):
File "\source\repos\Testing MSSQL Django Error\Testing MSSQL Django Error\manage.py", line 25, in
execute_from_command_line(sys.argv)
File "
\source\repos\Testing MSSQL Django Error\Testing MSSQL Django Error\env\lib\site-packages\django\core\management_init_.py", line 442, in execute_from_command_line
utility.execute()
File "\source\repos\Testing MSSQL Django Error\Testing MSSQL Django Error\env\lib\site-packages\django\core\management_init_.py", line 436, in execute
self.fetch_command(subcommand).run_from_argv(self.argv)
File "
\source\repos\Testing MSSQL Django Error\Testing MSSQL Django Error\env\lib\site-packages\django\core\management\base.py", line 412, in run_from_argv
self.execute(args, **cmd_options)
File "
\source\repos\Testing MSSQL Django Error\Testing MSSQL Django Error\env\lib\site-packages\django\core\management\base.py", line 458, in execute
output = self.handle(args, **options)
File "
\source\repos\Testing MSSQL Django Error\Testing MSSQL Django Error\env\lib\site-packages\django\core\management\base.py", line 106, in wrapper
res = handle_func(args, **kwargs)
File "
\source\repos\Testing MSSQL Django Error\Testing MSSQL Django Error\env\lib\site-packages\django\core\management\commands\migrate.py", line 356, in handle
post_migrate_state = executor.migrate(
File "\source\repos\Testing MSSQL Django Error\Testing MSSQL Django Error\env\lib\site-packages\django\db\migrations\executor.py", line 135, in migrate
state = self._migrate_all_forwards(
File "
\source\repos\Testing MSSQL Django Error\Testing MSSQL Django Error\env\lib\site-packages\django\db\migrations\executor.py", line 167, in _migrate_all_forwards
state = self.apply_migration(
File "\source\repos\Testing MSSQL Django Error\Testing MSSQL Django Error\env\lib\site-packages\django\db\migrations\executor.py", line 252, in apply_migration
state = migration.apply(state, schema_editor)
File "
\source\repos\Testing MSSQL Django Error\Testing MSSQL Django Error\env\lib\site-packages\django\db\migrations\migration.py", line 132, in apply
operation.database_forwards(
File "\source\repos\Testing MSSQL Django Error\Testing MSSQL Django Error\env\lib\site-packages\django\db\migrations\operations\fields.py", line 235, in database_forwards
schema_editor.alter_field(from_model, from_field, to_field)
File "
\source\repos\Testing MSSQL Django Error\Testing MSSQL Django Error\env\lib\site-packages\django\db\backends\base\schema.py", line 831, in alter_field
self._alter_field(
File "\source\repos\Testing MSSQL Django Error\Testing MSSQL Django Error\env\lib\site-packages\mssql\schema.py", line 907, in _alter_field
self.execute(sql, params)
File "
\source\repos\Testing MSSQL Django Error\Testing MSSQL Django Error\env\lib\site-packages\mssql\schema.py", line 1392, in execute
cursor.execute(sql, params)
File "\source\repos\Testing MSSQL Django Error\Testing MSSQL Django Error\env\lib\site-packages\django\db\backends\utils.py", line 102, in execute
return super().execute(sql, params)
File "
\source\repos\Testing MSSQL Django Error\Testing MSSQL Django Error\env\lib\site-packages\django\db\backends\utils.py", line 67, in execute
return self._execute_with_wrappers(
File "\source\repos\Testing MSSQL Django Error\Testing MSSQL Django Error\env\lib\site-packages\django\db\backends\utils.py", line 80, in _execute_with_wrappers
return executor(sql, params, many, context)
File "
\source\repos\Testing MSSQL Django Error\Testing MSSQL Django Error\env\lib\site-packages\django\db\backends\utils.py", line 89, in _execute
return self.cursor.execute(sql, params)
File "\source\repos\Testing MSSQL Django Error\Testing MSSQL Django Error\env\lib\site-packages\django\db\utils.py", line 91, in exit
raise dj_exc_value.with_traceback(traceback) from exc_value
File "
\source\repos\Testing MSSQL Django Error\Testing MSSQL Django Error\env\lib\site-packages\django\db\backends\utils.py", line 89, in _execute
return self.cursor.execute(sql, params)
File "*\source\repos\Testing MSSQL Django Error\Testing MSSQL Django Error\env\lib\site-packages\mssql\base.py", line 677, in execute
return self.cursor.execute(sql, params)
django.db.utils.ProgrammingError: ('42000', "[42000] [Microsoft][ODBC Driver 17 for SQL Server][SQL Server]'test1' is not a constraint. (3728) (SQLExecDirectW); [42000] [Microsoft][ODBC Driver 17 for SQL Server][SQL Server]Could not drop constraint. See previous errors. (3727)")

Any other details that can be helpful
I have not dove too deeply into the code that seems to be producing the error but from the sqlmigrate output it looks like there is an issue with the "schema].[table" naming in one of the statements.

ALTER TABLE [Bronze].[example2] ADD DEFAULT 'Test' FOR [test1];
UPDATE [Bronze].[example2] SET [test1] = 'Test' WHERE [test1] IS NULL;
ALTER TABLE [Bronze].[example2] ALTER COLUMN [test1] nvarchar(50) NOT NULL;
SELECT d.name FROM sys.default_constraints d INNER JOIN sys.tables t ON d.parent_object_id = t.object_id INNER JOIN sys.columns c ON d.parent_object_id = c.object_id AND d.parent_column_id = c.column_id INNER JOIN sys.schemas s ON t.schema_id = s.schema_id WHERE t.name = 'Bronze].[example2' AND c.name = 'test1';
ALTER TABLE [Bronze].[example2] DROP CONSTRAINT [test1];
COMMIT;

It looks like the select statement doesn't properly wrap the table name with "[]" and therefore can't find the constraint it is looking for.

@mShan0
Copy link
Contributor

mShan0 commented Jun 25, 2024

Hi @jdshupe, what was the specific field you added that led to this error? Also, was the table name in the example intentional? I.e. db_table = "Bronze].[example" instead of db_table = "[Bronze].[example]"

@jdshupe
Copy link
Author

jdshupe commented Jun 25, 2024

It did it with a CharField but I haven't tested any others yet.

The table name was intentional. It was my understanding that that is how schemas have to be handled as of now. I don't have a reference to where I got that from on hand.

@jdshupe
Copy link
Author

jdshupe commented Jun 26, 2024

I believe I have found a solution. I have not extensively tested it yet to make sure it doesn't mess up anything else though.

Updating the following query string to filter by schema

# schema.py lines 51-68
class DatabaseSchemaEditor(BaseDatabaseSchemaEditor):

    _sql_check_constraint = " CONSTRAINT %(name)s CHECK (%(check)s)"
    _sql_select_default_constraint_name = "SELECT" \
                                          " d.name " \
                                          "FROM sys.default_constraints d " \
                                          "INNER JOIN sys.tables t ON" \
                                          " d.parent_object_id = t.object_id " \
                                          "INNER JOIN sys.columns c ON" \
                                          " d.parent_object_id = c.object_id AND" \
                                          " d.parent_column_id = c.column_id " \
                                          "INNER JOIN sys.schemas s ON" \
                                          " t.schema_id = s.schema_id " \
                                          "WHERE" \
                                          " t.name = %(table)s AND" \
                                          " c.name = %(column)s AND" \
                                          " s.name = %(schema)s" # updated this to have a schema option
    sql_alter_column_default = "ADD DEFAULT %(default)s FOR %(column)s"
    sql_alter_column_no_default = "DROP CONSTRAINT %(column)s"
    sql_alter_column_not_null = "ALTER COLUMN %(column)s %(type)s NOT NULL"

and then updating the call to look for schema by splitting the db_table on "].["

# schema.py lines 149-156
            result = self.execute(
                self._sql_select_default_constraint_name % {
                    "table": self.quote_value(model._meta.db_table.split('].[')[1]),
                    "column": self.quote_value(new_field.column),
                    "schema": self.quote_value(model._meta.db_table.split('].[')[0]),
                },
                has_result=True
            )

I realize this would break functionality where the tables are not being called using a schema but this fixes my problem for now. I will look into fully implementing this without breaking other cases.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants