Opened 6 years ago

Last modified 4 years ago

#29170 new Bug

Unable to add triggers in migrations on Oracle.

Reported by: Danny Willems Owned by: nobody
Component: Database layer (models, ORM) Version: 3.0
Severity: Normal Keywords: oracle trigger database
Cc: Mariusz Felisiak Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

We have several database triggers we need to insert into our Oracle database using a custom migration. The migration runs without raising an error and the triggers are created in the database, however, when they are invoked as a result of various operations in the Django admin we see an error that says that the trigger cannot be compiled. After investigation we realised that a required semicolon was being removed from the SQL defined in the migration. Ordinarily this is removed from standard SQL statements such as SELECT, INSERT etc but in the case of triggers it is required as a way to delimit multiple BEGIN...END statements.

After debugging the issue we found the cause in this line of code:

https://github.com/django/django/blob/master/django/db/backends/oracle/base.py#L481

It appears that the blanket assumption that cx_Oracle does not require semicolons does not hold for triggers.

Here is a simplified migration that shows the issue:

from django.db import migrations

class Migration(migrations.Migration):
    dependencies = []

    create_trigger_insert_entry = """
    CREATE OR REPLACE TRIGGER UPDATE_BALANCE
    AFTER DELETE OR INSERT OR UPDATE OF AMOUNT ON ENTRY
    BEGIN
      UPDATE ACCOUNT_BALANCE B
      SET (B.BALANCE, B.ACCOUNT) = (SELECT SUM(AMOUNT) sum_amount, account
                              FROM ENTRY e WHERE e.ACCOUNT = B.ACCOUNT GROUP BY ACCOUNT)
      WHERE EXISTS (SELECT 1 FROM ENTRY e WHERE e.ACCOUNT = B.ACCOUNT);
    END;
    """

    delete_trigger_insert_entry = "DROP TRIGGER UPDATE_BALANCE"

    operations = [
        migrations.CreateModel(
            name='Account',
            fields=[
                ('name', models.CharField(max_length=32, primary_key=True, serialize=False)),
            ],
            options={
                'db_table': 'account',
                'managed': True,
            },
        ),

        migrations.CreateModel(
            name='Entry',
            fields=[
                ('value_date', models.DateTimeField()),
                ('amount', models.DecimalField(decimal_places=8, max_digits=23)),
            ],
            options={
                'db_table': 'entry',
                'managed': True,
            },
        ),
        migrations.CreateModel(
            name='AccountBalance',
            fields=[
                ('account', models.OneToOneField(db_column='account', on_delete=django.db.models.deletion.DO_NOTHING, primary_key=True, serialize=False, to='app.Account')),
                ('balance', models.DecimalField(decimal_places=14, max_digits=38)),
            ],
            options={
                'db_table': 'account_balance',
                'managed': True,
            },
        ),
        migrations.AddField(
            model_name='entry',
            name='account',
            field=models.ForeignKey(db_column='account', on_delete=django.db.models.deletion.CASCADE, to='app.Account'),
        ),
        migrations.RunSQL(sql=create_trigger_insert_entry, reverse_sql=delete_trigger_insert_entry),
    ]

As a workaround, we « fixed » this issue by overriding the method _fix_for_params with the following code:

def _fix_for_params(self, query, params, unify_by_values=False):
    # cx_Oracle wants no trailing ';' for SQL statements.  For PL/SQL, it
    # it does want a trailing ';' but not a trailing '/'.  However, these
    # characters must be included in the original query in case the query
    # is being passed to SQL*Plus.
    # ---> Fix this issue 
    if query.endswith(" END;"):
        pass
    elif query.endswith(';') or query.endswith('/'):
        query = query[:-1]
    if params is None:
        params = []
        query = query
    [...]

and we used

django.db.backends.oracle.base.FormatStylePlaceholderCursor._fix_for_params = _fix_for_params

in the migration file as it doesn't impact all the Django project. We would be happy to raise a pull request to get this fixed and obviously if anyone has a better way of doing this, we'd gladly oblige.

Change History (6)

comment:1 by Mariusz Felisiak, 6 years ago

Cc: Mariusz Felisiak added

IMO you missed trailing "/" in the trigger statement:

create_trigger_insert_entry = """
    CREATE OR REPLACE TRIGGER UPDATE_BALANCE
    AFTER DELETE OR INSERT OR UPDATE OF AMOUNT ON ENTRY
    BEGIN
      UPDATE ACCOUNT_BALANCE B
      SET (B.BALANCE, B.ACCOUNT) = (SELECT SUM(AMOUNT) sum_amount, account
                              FROM ENTRY e WHERE e.ACCOUNT = B.ACCOUNT GROUP BY ACCOUNT)
      WHERE EXISTS (SELECT 1 FROM ENTRY e WHERE e.ACCOUNT = B.ACCOUNT);
    END;
/"""

Can you confirm that it doesn't work with trailing slash?

in reply to:  1 comment:2 by Danny Willems, 6 years ago

Replying to felixxm:

IMO you missed trailing "/" in the trigger statement:

create_trigger_insert_entry = """
    CREATE OR REPLACE TRIGGER UPDATE_BALANCE
    AFTER DELETE OR INSERT OR UPDATE OF AMOUNT ON ENTRY
    BEGIN
      UPDATE ACCOUNT_BALANCE B
      SET (B.BALANCE, B.ACCOUNT) = (SELECT SUM(AMOUNT) sum_amount, account
                              FROM ENTRY e WHERE e.ACCOUNT = B.ACCOUNT GROUP BY ACCOUNT)
      WHERE EXISTS (SELECT 1 FROM ENTRY e WHERE e.ACCOUNT = B.ACCOUNT);
    END;
/"""

Can you confirm that it doesn't work with trailing slash?

We confirm it doesn't work. We have this exception:

django.db.utils.DatabaseError: ORA-24373: invalid length specified for statement
Last edited 6 years ago by Danny Willems (previous) (diff)

comment:3 by Mariusz Felisiak, 6 years ago

🤔 ORA-24373 cannot be related with the reported issue. If you pass a trigger statement with trailing / , than _fix_for_params returns the same statement as in the ticket description

>>> from django.db import connection
>>> from django.db.backends.oracle.base import FormatStylePlaceholderCursor
>>> create_trigger_insert_entry = """
    CREATE OR REPLACE TRIGGER UPDATE_BALANCE
    AFTER DELETE OR INSERT OR UPDATE OF AMOUNT ON ENTRY
    BEGIN
      UPDATE ACCOUNT_BALANCE B
      SET (B.BALANCE, B.ACCOUNT) = (SELECT SUM(AMOUNT) sum_amount, account
                              FROM ENTRY e WHERE e.ACCOUNT = B.ACCOUNT GROUP BY ACCOUNT)
      WHERE EXISTS (SELECT 1 FROM ENTRY e WHERE e.ACCOUNT = B.ACCOUNT);
    END;
/"""
>>> query, _ =  FormatStylePlaceholderCursor(connection)._fix_for_params(create_trigger_insert_entry, [])
>>> print(query)

    CREATE OR REPLACE TRIGGER UPDATE_BALANCE
    AFTER DELETE OR INSERT OR UPDATE OF AMOUNT ON ENTRY
    BEGIN
      UPDATE ACCOUNT_BALANCE B
      SET (B.BALANCE, B.ACCOUNT) = (SELECT SUM(AMOUNT) sum_amount, account
                              FROM ENTRY e WHERE e.ACCOUNT = B.ACCOUNT GROUP BY ACCOUNT)
      WHERE EXISTS (SELECT 1 FROM ENTRY e WHERE e.ACCOUNT = B.ACCOUNT);
    END;

comment:4 by Mariusz Felisiak, 6 years ago

Resolution: worksforme
Status: newclosed

I cannot reproduce this issue. Please reopen this ticket if you can provide a falling test.

comment:5 by Václav Řehák, 4 years ago

Resolution: worksforme
Status: closednew
Version: 1.113.0

Hi,

I encountered exactly the same issue. To reproduce see migration 0002_add_trigger in https://github.com/washeck/django-tutorial

manage.py migrate fails with

  Applying polls.0002_add_trigger...Traceback (most recent call last):
  File "/home/vaclav/.local/share/virtualenvs/django-tutorial-eSoqXmQ3/lib/python3.6/site-packages/django/db/backends/utils.py", line 84, in _execute
    return self.cursor.execute(sql)
  File "/home/vaclav/.local/share/virtualenvs/django-tutorial-eSoqXmQ3/lib/python3.6/site-packages/django/db/backends/oracle/base.py", line 517, in execute
    return self.cursor.execute(query, self._param_generator(params))
cx_Oracle.DatabaseError: ORA-24373: invalid length specified for statement

When I remove the slash, the migration applies but the trigger is incorrect and

select * from user_errors where type = 'TRIGGER' and name = 'TRIG_PREVENT_QUESTION_DELETE';

shows

PLS-00103: Encountered the symbol "end-of-file" when expecting one of the following:

   ; <an identifier> <a double-quoted delimited-identifier>
The symbol ";" was substituted for "end-of-file" to continue.

comment:6 by Mariusz Felisiak, 4 years ago

Summary: Oracle - Unable to add triggers in migrations, semicolon removed.Unable to add triggers in migrations on Oracle.
Triage Stage: UnreviewedAccepted

Thanks Václav. I was able to reproduce this issue, it's related with splitting SQL into multiple statements with sqlparse.split(sql) (see prepare_sql_script()). It splits CREATE TRIGGER statement on END; into:

  • CREATE TRIGGER ... END; and
  • / (which is an empty string after fix_for_params().
Note: See TracTickets for help on using tickets.
Back to Top