adamcharnock / django-hordak

Double entry accounting in Django
http://django-hordak.readthedocs.io
MIT License
238 stars 55 forks source link

Applying hordak.0035_account_currencies_json... raises psycopg2.errors #111

Closed paeduardo closed 4 months ago

paeduardo commented 8 months ago

Hi, when trying to apply hordak.0035_account_currencies_json (from django-hordak==1.13.0 to django-hordak==1.14.0) it appears that it cannot properly apply migration of account currencies field to JSON Anyone else also gets this error?


Django 4.2.9 python 3.12.1 postgres:12-bookworm django-tenants==3.6.1

Below log:

[1/1 (100%) standard:padom] === Starting migration
[1/1 (100%) standard:padom] Operations to perform:
[1/1 (100%) standard:padom]   Apply all migrations: admin, auth, contenttypes, core, hordak, invoicing, otp_static, otp_totp, otp_twilio, sequences, sessions, tax, tenants
[1/1 (100%) standard:padom] Running migrations:
[1/1 (100%) standard:padom]   Applying hordak.0035_account_currencies_json...
Traceback (most recent call last):
  File "/usr/local/lib/python3.12/site-packages/django/db/backends/utils.py", line 87, in _execute
    return self.cursor.execute(sql)
           ^^^^^^^^^^^^^^^^^^^^^^^^
psycopg2.errors.ObjectInUse: cannot CREATE INDEX "hordak_account" because it has pending trigger events

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

Traceback (most recent call last):
  File "/usr/src/app/manage.py", line 15, in <module>
    execute_from_command_line(sys.argv)
  File "/usr/local/lib/python3.12/site-packages/django/core/management/__init__.py", line 442, in execute_from_command_line
    utility.execute()
  File "/usr/local/lib/python3.12/site-packages/django/core/management/__init__.py", line 436, in execute
    self.fetch_command(subcommand).run_from_argv(self.argv)
  File "/usr/local/lib/python3.12/site-packages/django/core/management/base.py", line 412, in run_from_argv
    self.execute(*args, **cmd_options)
  File "/usr/local/lib/python3.12/site-packages/django/core/management/base.py", line 458, in execute
    output = self.handle(*args, **options)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/site-packages/django_tenants/management/commands/migrate_schemas.py", line 77, in handle
    executor.run_migrations(tenants=tenants)
  File "/usr/local/lib/python3.12/site-packages/django_tenants/migration_executors/standard.py", line 14, in run_migrations
    run_migrations(self.args, self.options, self.codename, schema_name, idx=idx, count=len(tenants))
  File "/usr/local/lib/python3.12/site-packages/django_tenants/migration_executors/base.py", line 59, in run_migrations
    migrate_command_class(stdout=stdout, stderr=stderr).execute(*args, **options)
  File "/usr/local/lib/python3.12/site-packages/django/core/management/base.py", line 458, in execute
    output = self.handle(*args, **options)
             ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/site-packages/django/core/management/base.py", line 106, in wrapper
    res = handle_func(*args, **kwargs)
          ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/site-packages/django/core/management/commands/migrate.py", line 356, in handle
    post_migrate_state = executor.migrate(
                         ^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/site-packages/django/db/migrations/executor.py", line 135, in migrate
    state = self._migrate_all_forwards(
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/site-packages/django/db/migrations/executor.py", line 167, in _migrate_all_forwards
    state = self.apply_migration(
            ^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/site-packages/django/db/migrations/executor.py", line 249, in apply_migration
    with self.connection.schema_editor(
  File "/usr/local/lib/python3.12/site-packages/django/db/backends/base/schema.py", line 166, in __exit__
    self.execute(sql)
  File "/usr/local/lib/python3.12/site-packages/django/db/backends/postgresql/schema.py", line 48, in execute
    return super().execute(sql, None)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/site-packages/django/db/backends/base/schema.py", line 201, in execute
    cursor.execute(sql, params)
  File "/usr/local/lib/python3.12/site-packages/django/db/backends/utils.py", line 102, in execute
    return super().execute(sql, params)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/site-packages/django/db/backends/utils.py", line 67, in execute
    return self._execute_with_wrappers(
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/site-packages/django/db/backends/utils.py", line 80, in _execute_with_wrappers
    return executor(sql, params, many, context)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/usr/local/lib/python3.12/site-packages/django/db/backends/utils.py", line 84, in _execute
    with self.db.wrap_database_errors:
  File "/usr/local/lib/python3.12/site-packages/django/db/utils.py", line 91, in __exit__
    raise dj_exc_value.with_traceback(traceback) from exc_value
  File "/usr/local/lib/python3.12/site-packages/django/db/backends/utils.py", line 87, in _execute
    return self.cursor.execute(sql)
           ^^^^^^^^^^^^^^^^^^^^^^^^
django.db.utils.OperationalError: cannot CREATE INDEX "hordak_account" because it has pending trigger events
adamcharnock commented 4 months ago

Hi @paeduardo. I'm guessing this is because transaction 0035 tries to affect both the schema and data in a single transaction. You can probably work around this using something like:

# 1. Comment out `migrations.RunPython(...)` on line 62 of 0035_account_currencies_json.py

# 2. Run the migration
./manage.py migrate hordak 0035

# 3. Manually run the SQL:
UPDATE hordak_account SET currencies_json = array_to_json(currencies)::jsonb;

# 4. Finish the migrations
./manage.py migrate hordak

I'm not sure what the solution is here as these migrations are already out in the wild.

@PetrDlouhy @nitsujri – Let's just watch out for this in the future (I.e. best use separate migrations for schema and data changes)

I'm going to close this now in the hope that the above workaround is sufficient, but if anyone is suffering with this then please leave a comment.

PetrDlouhy commented 4 months ago

@adamcharnock Sorry for merging that.

Maybe we could make a migration branch. I would have to look into it, but maybe this could be solved with alternate migration history and merged migration, so that users who didn't update yet would have not face this problem.

Do you think that this could be added to the testing? E.g. adding some initial data and then migrating?

adamcharnock commented 4 months ago

No worries, these things happen!

And I was just thinking about this. Being able to test these migrations would be great, but I'm not aware of Django built-in way to control migrations during testing. We could probably do it with a custom script though (migrate to migration X, load some data, finish migrations, check it all looks ok). This would be especially useful for Hordak 2.0.

EDIT: django-test-migrations

paeduardo commented 3 months ago

Hi @adamcharnock ! Thanks for support! Based on your solution I have splitted 0035_account_currencies_json.py migration file into 2 parts and have run it sequentially, after that 0036.., 0037.... And it worked perfectly! Splitted 0035... migration file: part 1: a0035_account_currencies_json.py

from django.db import migrations, models
import hordak

class Migration(migrations.Migration):
    dependencies = [
        ("hordak", "0034_alter_account_currencies_alter_leg_amount_currency"),
    ]

    operations = [
        migrations.AddField(
            model_name="account",
            name="currencies_json",
            field=models.JSONField(
                db_index=True,
                default=hordak.defaults.project_currencies,
            ),
        ),    
    ]

part 2: 0035_account_currencies_json.py

from django.db import migrations, models

import hordak

def copy_currencies_data(apps, schema_editor):
    # MySQL won't have any old data, because support has only now been added
    if schema_editor.connection.vendor == "postgresql":
        MyModel = apps.get_model(
            "hordak", "Account"
        )  # replace 'myapp' and 'MyModel' with your actual app and model names
        table_name = MyModel._meta.db_table
        with schema_editor.connection.cursor() as cursor:
            # only run this if there is data in the table (in which case we're an ARRAY to migrate);
            # disregard if migrations are being run on a fresh database
            if MyModel.objects.count() > 0:
                cursor.execute(
                    f"""
                    UPDATE {table_name}
                    SET currencies_json = array_to_json(currencies)::jsonb;
                """
                )
            else:
                cursor.execute(
                    f"""
                    UPDATE {table_name}
                    SET currencies_json = currencies;
                """
                )

class Migration(migrations.Migration):
    dependencies = [
        ("hordak", "a0035_account_currencies_json"),
    ]

    operations = [
        migrations.RunPython(copy_currencies_data),
    ]