UDFs are not dumped to the clone MySQL test db

[Thomas Ormezzano]

Stored procedures and functions are not dumped to the clone db.

When creating the test database with mysql, the mysqldump command is only dumping the schemas but not the stored procedures and functions, when that should probably be the default.

Adding the --routines option to the mysqldump command would fix it (see ​https://dev.mysql.com/doc/refman/8.0/en/mysqldump.html#option_mysqldump_routines)

[Dan Davis]

thomazzo,

Can you clarify your database OPTIONS settings?

Without any options to force clone/mirror, Django should not preserve the DDL and data in the database. In the normal mode, Django is not cloning the database using mysqldump - it is dropping and recreating the database, and then running migrations.

I've created a git repository that verifies this occurs, but it is not a patch, but an example.

The git repo is ​https://github.com/danizen/django-review-29882.

If you set it up with mysql 5.7 on ubuntu and create a "djangoreview" user and "djangoreview" and "test_djangoreview" database to which that user has all privileges, you should be able to run ./manage.py test and verify you get:

django.db.utils.OperationalError: (1305, 'FUNCTION test_djangoreview.HELLO does not exist')

----------------------------------------------------------------------
Ran 2 tests in 0.026s

FAILED (errors=1)
Destroying test database for alias 'default'...

If you want, you could try to get this test to work in isolation by writing a custom migration as follows:

./manage.py makemigrations app --empty  -n add_hello_function

Then, edit the migration file created, and use the RunSQL migration operation to run text from the file sql/ddl/hello.sql. This shows how you can use the django migrations system to manage custom defined SQL.

There are add-on packages that do similar things, but I want to emphasize that Django core contains the functionality to read SQL files.
If you are using Python 3, I recommend you pathlib to read a file to get the text to run.

[Dan Davis]

thomazzo,

I've reviewed the general and MySQL specific OPTIONS and settings for the test database, as listed:

• here - ​https://docs.djangoproject.com/en/2.1/ref/settings/#databases
• and Here - ​https://docs.djangoproject.com/en/2.1/ref/databases/#mysql-notes

There is no option I'm aware of that would clone the entire database - think of how much time that would take. Also, the whole point of Django's migration support is to allow a developer such as yourself to manage DDL statements needed to define tables, views, functions, stored procedures, and such.

Django is an opinionated framework, and it makes table definitions and relationships easy, but if you need custom SQL, it provides mechanisms for including that sort of DDL. So, I think this is not actually a bug. It would be nice if Django's RunSQL migration operation supported filenames, or there was some other migration operation that handled files. However, that is a different feature and someone may already have filed it.

[Adam Johnson]

Dan, I think you missed a step here... @thomazzo is talking about when Django clones test database for parallel runs, which is done at the start of test runs by setup_databases: ​https://github.com/django/django/blob/master/django/test/utils.py#L176 . This calls through to backend specific functions, which Django has for MySQL here: ​https://github.com/django/django/blob/9a88c6dd6aa84a1b35e585faa0058a0996cc7ed7/django/db/backends/mysql/creation.py#L31

I think the request is really around passing --routines to mysqldump in _clone_db. I think this is a good idea and probably just needs one line in that function.

Dan, you could make a test case from your example repo by using the CREATE FUNCTION statement in a RunSQL in a migration.

[Dan Davis]

@adamchainz, I can add a 1-liner there and endeavor to add tests. Is ​https://github.com/django/django/blob/55b0b766fbeb2f71e68331a2e14205702f681012/tests/backends/mysql/test_creation.py#L13 the right place to put that test, or it should it tested with a separate repository because Django's tests are unit tests?

I ask because django.test.TestCase is so good it makes most of my tests really Service Level tests or Database Integration Tests, not really unit tests. Django itself may have a different philosophy about what the tests should be and how fast they should run...

[Adam Johnson]

That looks the right place to add a test. Since these tests are true unit tests and not actually calling mysqldump, you only need test that the --routines argument is passed to a mock for subprocess.Popen.

But I'd even be tempted to not add a test, we often don't for features that heavily rely on other systems - instead you can confirm the functionality works for an example project like you made before. Since the change is so small and pretty non-offensive, that should be fine.

[Dan Davis]

I've enhanced ​https://github.com/danizen/django-review-29882 so that the tests run fine without a parallel run, but fail when run in parallel, reproducing the issue raised by the ticket.

The issue can be reproduced by running:

/manage.py test --parallel 2 -v 3

Fixing it is indeed a 1-liner, and I will submit a pull request shortly.

[Dan Davis]

Pull request exists as ​https://github.com/django/django/pull/10579

[Thomas Ormezzano]

Hi,

Sorry folks I was busy this weekend and couldn't reply. Yes Adam that's exactly what I meant.

I am not sure what the usual process is here but I had assigned this ticket to myself and set it to Has patch as I already had a patch for it and was waiting for this ticket to be accepted to submit the PR. Going to submit it now that it has been accepted and I don't think Dan's suggestion works.

[Thomas Ormezzano]

​PR

[Adam Johnson]

Thomas, next time you have a patch please submit a PR immediately. Even if it's wrong it can help the undesrtanding of the issue.

[Carlton Gibson]

Hi Thomas.

Yes, please do submit in future. I saw the issue, saw you had self-assigned and assumed the PR would land soonish: I was waiting for the PR to be able to assess the issue.

All good now. 🙂

Thanks for the input!

[Tim Graham <timograham@…>]

In 9625d13:

Fixed #29882 -- Added events and stored routines to MySQL's cloned test databases.