Opened 7 days ago

Last modified 3 days ago

#36189 assigned Bug

Oracle Backend with `"use_returning_into": False` Option Fails to Retrieve Last Inserted ID

Reported by: Yeongbae Jeon Owned by: Amaan-ali03
Component: Database layer (models, ORM) Version: 5.1
Severity: Normal Keywords:
Cc: Yeongbae Jeon, Amaan-ali03, Mariusz Felisiak, Simon Charette Triage Stage: Accepted
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

When running tests in django-docker-box with Oracle, setting the "use_returning_into": False option in settings.py prevents Django from retrieving the last inserted ID. The issue can be reproduced by modifying the database options as shown below:

if engine.endswith(".oracle"):
    entry |= {
        "NAME": name,
        "OPTIONS": {
            "use_returning_into": False,
        }
    }
    ...

Cause of the Issue
The issue originates from the last_insert_id() method in django/db/backends/oracle/operations.py. The SQL executed in this method does not properly handle the query format, leading to a failure in retrieving the sequence value.

Current Implementation (Buggy Code)

def last_insert_id(self, cursor, table_name, pk_name):
    sq_name = self._get_sequence_name(cursor, strip_quotes(table_name), pk_name)
    cursor.execute('"%s".currval' % sq_name)
    return cursor.fetchone()[0]

Proposed Fix
The issue can be resolved by modifying the query to include the correct SQL syntax:

def last_insert_id(self, cursor, table_name, pk_name):
    sq_name = self._get_sequence_name(cursor, strip_quotes(table_name), pk_name)
    template = 'SELECT "%s".currval' + self.connection.features.bare_select_suffix

    cursor.execute(template % sq_name)
    return cursor.fetchone()[0]

Change History (7)

comment:1 by Amaan-ali03, 7 days ago

Cc: Amaan-ali03 added
Easy pickings: unset
Owner: set to Amaan-ali03
Status: newassigned

"Hey Django Team,
I’d love to work on this issue! From what I understand, the problem comes from the last_insert_id() method in oracle/operations.py, where the query fails when "use_returning_into": False is set.

Plan to Fix:
Reproduce the issue in django-docker-box with Oracle.
Update the query to correctly fetch currval, as proposed.
Add tests to ensure the fix works with and without "use_returning_into".
Run the full Django test suite to confirm no regressions.
Could you assign this to me? Looking forward to contributing!

Thanks,
Amaan Ali

comment:2 by Simon Charette, 6 days ago

Cc: Mariusz Felisiak added
Triage Stage: UnreviewedAccepted

I can replicate the issue but we should agree on a solution before proceeding here.

This relates to #11706 (which introduced use_returning_into) and #27789 which changed the way last_insert_id works.

I tried applying the proposed patch and there are many tests that fail with it against (Oracle 23.5.0.24.07) so I wonder if there is something else at play here. Given use_returning_into has been untested since its introduction 15 years ago and now appears to be broken in non trivial ways I wonder if we should just simply remove support for it instead of trying to resolve this issue.

Any thoughts on the current situation Mariusz?

comment:3 by Simon Charette, 6 days ago

I dug a bit more in the reason behind why so many tests were failing even with the SQL adjusted to be valid on Oracle 23 by adding the SELECT prefix and the reason is quite clear; using SELECT sequence_name.currval doesn't account for explicitly specified primary key values which Django allows.

In other words, AutoField are defined using IDENTITY but not GENERATED ALWAYS which means that an explicit value can be specified and must be supported. When it's the case returning the current value of the associated sequence is simply wrong. I think this is a significant oversight from 1f68dc4ad4ddc67831c6aa047683a5b53fa33a37 and another argument to deprecate this option altogether.

comment:4 by Simon Charette, 5 days ago

Another reason why use_returning_into=False is broken is that it SELECT sequence_name.currval doesn't account for concurrent INSERT. From what I understand, just like in Postgres, Oracle sequences are not tied to transactions which means that nothing prevents a concurrent INSERT from being executed in a separate transaction and increasing the sequence before it can be SELECTed.

e.g.

  • session0: INSERT
  • session1: INSERT
  • session0: SELECT sequence_name.currval
  • session1: SELECT sequence_name.currval

Both session 0 and 1 assume that their last_insert_id is the same; session 0 is wrong.

in reply to:  4 comment:5 by Yeongbae Jeon, 4 days ago

Replying to Simon Charette:

Another reason why use_returning_into=False is broken is that it SELECT sequence_name.currval doesn't account for concurrent INSERT. From what I understand, just like in Postgres, Oracle sequences are not tied to transactions which means that nothing prevents a concurrent INSERT from being executed in a separate transaction and increasing the sequence before it can be SELECTed.

e.g.

  • session0: INSERT
  • session1: INSERT
  • session0: SELECT sequence_name.currval
  • session1: SELECT sequence_name.currval

Both session 0 and 1 assume that their last_insert_id is the same; session 0 is wrong.

I agree with you. Another case I can think of (although I haven't tested it yet, so please correct me if I'm wrong, as this is based on my limited experience) is when a user explicitly provides a value for the primary key while creating an object, instead of relying on the nextval from the sequence.

For example, if the currval of the sequence is 1, but the user manually assigns 10 as the primary key when creating the object, then use_returning_into: False will execute the last_insert_id function, which will return the incorrect value of 1.


(Edit 1, 02/19/2025)

I just tested creating a table, sequence, and trigger in SQL*Plus and then performed inserts in two separate sessions. It seems that the sequence is tied to each transaction. Below is the example I executed. and the oracle version is 23.5.0.24.07

Code to Run Before Insert

CREATE SEQUENCE my_sequence
START WITH 1
INCREMENT BY 1
NOCACHE
NOCYCLE;

CREATE TABLE my_table (
    id NUMBER PRIMARY KEY,
    name VARCHAR2(100)
);

CREATE OR REPLACE TRIGGER my_table_trigger
BEFORE INSERT ON my_table
FOR EACH ROW
WHEN (NEW.id IS NULL)
BEGIN
    :NEW.id := my_sequence.NEXTVAL;
END;
/

Test Steps

  1. Open the first terminal tab running SQL*Plus
       INSERT INTO my_table (name) VALUES ('Alice');
    
       1 row created.
    
  1. Switch to the second terminal tab running SQL*Plus
       INSERT INTO my_table (name) VALUES ('Bob');
    
       1 row created.
    
  1. Back in the first tab
       SQL> SELECT my_sequence.CURRVAL FROM DUAL;
    
          CURRVAL
       ----------
             1
    
  1. Then, in the second tab
       SQL> SELECT my_sequence.CURRVAL FROM DUAL;
    
          CURRVAL
       ----------
             2
    

Another case that shows sequence is tied to transactions:

Code to Run Before Insert

CREATE SEQUENCE my_sequence
START WITH 1
INCREMENT BY 1
NOCACHE
NOCYCLE;

CREATE TABLE my_table (
    id NUMBER PRIMARY KEY,
    name VARCHAR2(100)
);

CREATE OR REPLACE TRIGGER my_table_trigger
BEFORE INSERT ON my_table
FOR EACH ROW
WHEN (NEW.id IS NULL)
BEGIN
    :NEW.id := my_sequence.NEXTVAL;
END;
/

Test Steps

  1. Open the first terminal tab running SQL*Plus
       SQL> INSERT INTO my_table (name) VALUES ('Alice');
    
       1 row created.
    
       SQL> SELECT my_sequence.CURRVAL FROM DUAL;
    
          CURRVAL
       ----------
             1
    
  1. Then, in the second tab, but this time do not insert
      SQL> select my_sequence.currval;
      select my_sequence.currval
             *
      ERROR at line 1:
      ORA-08002: Sequence MY_SEQUENCE.CURRVAL is not yet defined in this session.
      Help: https://docs.oracle.com/error-help/db/ora-08002/
    
Last edited 3 days ago by Yeongbae Jeon (previous) (diff)

comment:6 by Simon Charette, 4 days ago

Cc: Simon Charette added

I agree with you. Another case I can think of (although I haven't tested it yet, so please correct me if I'm wrong, as this is based on my limited experience) is when a user explicitly provides a value for the primary key while creating an object, instead of relying on the nextval from the sequence.

Right this is what is causing many test failures as described in comment:3 and why the solution is not appropriate as don't use GENERATED ALWAYS which would disallow explicit assignment of primary key values.

Another case that shows sequence is tied to transactions:

Thanks so at least this feature is not completely broken. I had not tried it it out myself but I read mixed reports online depending on the isolation level used (e.g. READ COMMITTED vs REPEATABLE READ).

Version 0, edited 4 days ago by Simon Charette (next)

comment:7 by Mariusz Felisiak, 4 days ago

I have no issue with deprecating this feature.

Note: See TracTickets for help on using tickets.
Back to Top