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

Bugfix/Add support for SQL Server database #255

Merged
merged 3 commits into from Jul 22, 2019

Conversation

@c-w
Copy link
Member

c-w commented Jun 24, 2019

When running doccano with a SQL Server backend, applying the migrations fails with the following error:

Screenshot showing migrate error with SQL Server

This is because the Seq2seqAnnotation model has a TextField that participates in the unique_together constraint. However, in SQL Server, the maximum index field size is 900 bytes so the potential unbounded length text field fails validation.

The solution for this is to change the text field to a char field with a generous maximum length. Note that the Django migrations apparatus doesn't seem to support editing the unique together constraint so a manual pre-processing step is required before running the new migrations (namely to drop the unique together constraint so that the migration may add it back).


When running doccano's tests with a SQL Server backend, we get the following error:

Violation of UNIQUE KEY constraint 'api_label_project_id_prefix_key_suffix_key_1b3d8f77_uniq'.
Cannot insert duplicate key in object 'dbo.api_label'.
The duplicate key value is (32, <NULL>, <NULL>).

This is because we currently use the unique_together constraint to ensure that if a label has a shortcut key combination, no other label has the same combination. However, on SQL Server, the unique_together constraint also looks at null values which breaks the unique_together constraint since we now can no longer have two labels without shortcut keys.

Moving the validation logic from the database into Django fixes the issue and makes the constraint validation logic somewhat clearer.


Tested with Microsoft SQL Azure (RTM) - 12.0.2000.8 and mcr.microsoft.com/mssql/server:2017-latest.

You can run all the tests against SQL Server in Docker as follows:

# start a sql server instance

mssql_password='AT3st!Passw0rd'
mssql_db='db'

docker network create doccano

docker run --network doccano --rm -e ACCEPT_EULA=Y -e SA_PASSWORD="${mssql_password}" --name mssql -d mcr.microsoft.com/mssql/server:2017-latest

# create a test database

docker exec mssql /opt/mssql-tools/bin/sqlcmd -S localhost -U SA -P "${mssql_password}" -Q "CREATE DATABASE ${mssql_db};"

# run the unit tests against the test database

docker build --tag=doccano-test .

mssql_url="mssql://SA:${mssql_password}@mssql:1433/${mssql_db}"

docker run --network doccano -e DATABASE_URL="${mssql_url}" -it doccano-test sh -c '/doccano/app/manage.py migrate && /doccano/app/manage.py test api.tests server.tests'
@c-w c-w force-pushed the CatalystCode:bugfix/support-sql-server branch 3 times, most recently from 3d857bf to 95a32ca Jun 26, 2019
c-w added 3 commits Jun 24, 2019
Currently we use the unique_together constraint to ensure that if a label has a
shortcut key combination, no other label has the same combination.

However, on SQL Server, the unique_together constraint also looks at null
values which breaks the unique_together constraint since we now can no longer
have two labels without shortcut keys. The error message is as follows:

```
Violation of UNIQUE KEY constraint 'api_label_project_id_prefix_key_suffix_key_1b3d8f77_uniq'.
Cannot insert duplicate key in object 'dbo.api_label'.
The duplicate key value is (32, <NULL>, <NULL>).
```

Moving the validation logic from the database into Django fixes the issue and
makes the constraint clearer.
@c-w c-w force-pushed the CatalystCode:bugfix/support-sql-server branch from 55b686f to 6b0e32e Jul 16, 2019
@Hironsan Hironsan merged commit 1f316b6 into doccano:master Jul 22, 2019
2 checks passed
2 checks passed
Codacy/PR Quality Review Up to standards. A positive pull request.
Details
Travis CI - Pull Request Build Passed
Details
@c-w c-w deleted the CatalystCode:bugfix/support-sql-server branch Jul 22, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
2 participants
You can’t perform that action at this time.