microsoft/mssql-django

MSSQL FileField max_length restrictions

jens-Github opened this issue · 6 comments

Dear mssql-django team,
I appreciate the hard work you have put into developing this package. I'm currently encountering an issue which I am unsure whether it has been listed under your documented limitations .

Short Summary:
Our development team is integrating the django-pyas2 package into our project and has encountered a database level error with SQL Server regarding the maximum allowed character length for a Django CharField (4000 characters in this case).

Table schema and Model
Here is the model causing the issue:

class Message(models.Model):
    # ... other fields and methods

    payload = models.FileField(
        upload_to=get_message_store, null=True, blank=True, max_length=4096
    )

    # ... other fields and methods

Error message/stack trace
In executing our Django application, a django.db.utils.ProgrammingError occurs::

venv\lib\site-packages\mssql\base.py", line 621, in execute return self.cursor.execute(sql, params) django.db.utils.ProgrammingError: ('42000', "[42000] [Microsoft][ODBC Driver 17 for SQL Server][SQL Server]The size (4096) given to the parameter 'payload' exceeds the maximum allowed (4000). (2717) (SQLExecDirectW)"

Any other details that can be helpful
This problem has been briefly discussed in this abhishek-ram/django-pyas2#107

If this is indeed a known limitation, could it perhaps be clarified more in the documentation? If it's unknown, do you need any further information to investigate further or recommend possible workarounds?
Thank you for your assistance!

Hi @jens-Github

CharField and FileField is configured as nvarchar(n) which can be from 1 - 4,000 bytes: See SQL Server restrictions

'FileField': 'nvarchar(%(max_length)s)',

We do this for several performance reasons. If however, you are adamant on specifying max_length=4096 instead of max_length=4000, a potential fix may be to override the field mapping to be FileField: nvarchar(max) or change both CharField and FileField to be varchar(n).

Dear @chadgates and @dauinsight,

Firstly, allow me to extend my sincere thanks for your previous insights. Your in-depth knowledge and perspective have helped greatly in our understanding of this issue.

Following the suggestions provided by @dauinsight from the mssql-django team, we have considered two potential solutions:

  1. Overriding the field mapping for FileField to change it to NVARCHAR(MAX), which could potentially allow the field to store a significantly larger payload.

  2. Changing both CharField and FileField to VARCHAR(n).

However, as these changes would directly affect the Message model from the django-pyas2 package, we realize that making these alterations on our end could lead to potentially unforeseen complications. As this is a third-party package, adjustments of this nature might best be performed on the package side to ensure proper functionality and compatibility.

Therefore, we would like to ask if it would be possible for you (@chadgates), as the maintainer of the django-pyas2 package, to consider implementing these adjustments on your side based on @dauinsight's suggestions. This would help ensure that the package continues to function as expected with the MSSQL backend, despite its character limit for VARCHAR and NVARCHAR fields.

I understand that this might involve relative complexities and I would greatly appreciate your consideration on this matter. As always, your support and assistance have been invaluable thus far.

Best Regards, Jens

@dauinsight : I would suggest that to accommodate a particular logic here, as logic from django model to database is within the scope of this library. I'd suggest that a change is made so that any length which is above 4000, then NVARCHAR(MAX) is applied instead of (max_length) value. I would make PR, but I am not familiar enough with your package, nor directly affected by the issue.

@jens-Github : changing a migration file retroactively will create unforeseen issues with existing installs, even to data loss. Even if we did change the field length to 4000, your installation will fail as the already existing migration file would apply first and that would fail. And by the way, I am not maintainer of django-pyas2... just contributor.

Appreciate the suggestions @chadgates @jens-Github. I believe that adding the condition to the data_types dictionary is a viable solution. Before I make the change, I'd like to ask about how overriding the value in your settings could result in complications? Thanks!

Dear @chadgates and @dauinsight,

Thank you for your continued support and insightful recommendations.

@chadgates, my apologies for the mix-up. I appreciate your contributions to this conversation despite your role as a contributor and not a maintainer to django-pyas2. I absolutely agree with your reservations about retroactively changing a migration file, and I understand the possible implications, including data loss.

@dauinsight, I certainly agree with the suggestions to add conditions in the mssql-django package, which automatically assign NVARCHAR(MAX) to fields with length exceeding 4000. In answer to your question, the anticipated complications could relate to the possible clashes with the existing data types and potential loss of data.

Should the changes be implemented on your side with mssql-django, I would be more than willing to test it out on our end and share the results.

Again, I am grateful for your input and time spent in finding a solution to this issue.

Best Regards, Jens

Hi @jens-Github,
Upon further review, we'd like to suggest a workaround to the issue:

Can you modify your settings.py like so:

from mssql.base import DatabaseWrapper

DatabaseWrapper.data_types['FileField'] = 'nvarchar(max)'

While not the cleanest solution, I don't believe there will be any implications.