Opened 4 years ago

Closed 4 years ago

Last modified 3 years ago

#15682 closed Bug (duplicate)

Postgresql last_insert_id() failing when using custom schemas

Reported by: akaariai Owned by: nobody
Component: Database layer (models, ORM) Version: 1.3
Severity: Normal Keywords: regression
Cc: inactivist@… Triage Stage: Unreviewed
Has patch: no Needs documentation: no
Needs tests: no Patch needs improvement: no
Easy pickings: no UI/UX: no

Description

Define a table like this:

create sequence foobar;
CREATE TABLE test (
    id integer not null primary key default nextval('foobar')
);

And a model like this:

class TestM(models.Model):
    pass
    class Meta:
         db_table = 'test'

In the shell, try this:

>>> t = TestM()
>>> t.save()
>>> t.id is None
True

The error here is that postgresql's last_insert_id is defined like this:

def last_insert_id(self, cursor, table_name, pk_name):
        # Use pg_get_serial_sequence to get the underlying sequence name
        # from the table name and column name (available since PostgreSQL 8)
        cursor.execute("SELECT CURRVAL(pg_get_serial_sequence('%s','%s'))" % (
            self.quote_name(table_name), pk_name))
        return cursor.fetchone()[0]

Now, what happens here is that pg_get_serial_sequence('test', 'id') will return NULL, and so will currval(NULL).

This is backwards incompatible as the same code would have worked in 1.2. Though I don't think it is documented anywhere that the above should work.

One possible fix is to throw an error if None is returned by cursor.fetchone()[0] above and also document in the release notes that if you have a database defined like this, you should issue a query:

ALTER SEQUENCE foobar OWNED BY test, id;

This will unfortunately not work if the sequence is used by multiple tables, as a sequence can only be owned by one column at a time.

The best fix would be to allow setting the sequence name as an Meta option in the table (as in #13295), but it is way too late for 1.3.

Related open tickets are at least #13179, #1946, #13295

Change History (7)

comment:1 Changed 4 years ago by akaariai

  • Needs documentation unset
  • Needs tests unset
  • Patch needs improvement unset

A correction:

ALTER SEQUENCE foobar OWNED BY test, id;

Should be:

ALTER SEQUENCE foobar OWNED BY test.id;

comment:2 Changed 4 years ago by anonymous

  • Keywords regression added; sequence postgresql removed

comment:3 Changed 4 years ago by lukeplant

  • Type set to Bug

comment:4 Changed 4 years ago by lukeplant

  • Severity set to Normal

comment:5 Changed 4 years ago by jacob

  • Resolution set to duplicate
  • Status changed from new to closed

Yeah, this is a duplicate of ##13295 - the fix is to allow a custom sequence declaration.

comment:6 Changed 3 years ago by jacob

  • milestone 1.3 deleted

Milestone 1.3 deleted

comment:7 Changed 3 years ago by inactivist

  • Cc inactivist@… added
  • Easy pickings unset
  • UI/UX unset
Note: See TracTickets for help on using tickets.
Back to Top