Unexpected model create behaviour for non-integer primary keys

[ambrozic]

When primary key is i.e. String, model instance returned from model.object.create has primary key as sequential integer. Once instance is fetched again, primary key is as expected - type and value wise.
Postgres behaviour is similar. Returned instance primary key in this case is actually None. Same for UUID type. Once fetched again, everything is as expected.

Could be that problem originates from databases library or incorrect usage of it.

Two copy/paste tests.

import random

import databases
import pytest
import sqlalchemy

import orm
from tests.settings import DATABASE_URL
from tests.test_columns import async_adapter

database = databases.Database(DATABASE_URL, force_rollback=True)
metadata = sqlalchemy.MetaData()


def key():
    return "".join(random.choice("abcdefgh123456") for _ in range(8))


class Model(orm.Model):
    __tablename__ = "model"
    __metadata__ = metadata
    __database__ = database

    id = orm.String(primary_key=True, default=key, max_length=8)
    name = orm.String(max_length=32)


@pytest.fixture(autouse=True, scope="function")
def create_test_database():
    engine = sqlalchemy.create_engine(DATABASE_URL)
    metadata.create_all(engine)
    yield
    metadata.drop_all(engine)


@async_adapter
async def test_pk_1():
    model = await Model.objects.create(name="NAME")
    assert isinstance(model.id, str)


@async_adapter
async def test_pk_2():
    model = await Model.objects.create(name="NAME")
    assert await Model.objects.all() == [model]

[Dodger666]

Does somebody has an update on this? I have the same issue. using a str uuid as PK, but the id is always an integer before the new object is read from DB

[unittolabs]

seems it's asyncpg issue - MagicStack/asyncpg#526

[Nipsuli]

Noticed same issue, ended up writing own create in our BaseModel did basically same as is done in QuerySet.create but added the returning clause to the insert statement.

class BaseModel(orm.Model):
    __abstract__ = True

    @classmethod
    async def create(cls, **kwargs):
        fields = cls.fields
        required = [key for key, value in fields.items() if not value.has_default()]
        validator = typesystem.Object(
            properties=fields, required=required, additional_properties=False
        )
        kwargs = validator.validate(kwargs)

        pk_column = getattr(cls.__table__.c, cls.__pkname__)
        expr = cls.__table__.insert().returning(pk_column)
        expr = expr.values(**kwargs)

        instance = cls(kwargs)
        instance.pk = await cls.__database__.execute(expr)
        return instance

[tomchristie]

@Nipsuli That's really helpful thanks - we probably ought to update the create method along those lines.

[jmgurney]

I am able to reproduce this using sqlite.

$ pip list
Package           Version
----------------- -------
aiosqlite         0.16.0
databases         0.4.1
orm               0.1.5
pip               20.2.4
pydantic          1.7.2
setuptools        50.3.2
SQLAlchemy        1.3.20
typesystem        0.2.4
typing-extensions 3.7.4.3
wheel             0.35.1

The script:

import databases
from pydantic import BaseModel, Field
import orm
import sqlalchemy
import asyncio

database = databases.Database('sqlite:///test.sqlite')
metadata = sqlalchemy.MetaData()

class Foo(orm.Model):
	__tablename__ = 'footable'
	__database__ = database
	__metadata__ = metadata

	fielda = orm.Text(primary_key=True)
	fieldb = orm.Text(index=True)

engine = sqlalchemy.create_engine(str(database.url))
metadata.create_all(engine)

async def foo():
	a = await Foo.objects.create(fielda='dataa', fieldb='datab')
	print('fa:', repr(a.fielda))
	print('fb:', repr(a.fieldb))

asyncio.run(foo())

The results:

$ python test.py 
fa: 1
fb: 'datab'

As you can see, the returned results are an integer. If the data was fetched again, it'd be present.

The database is created correctly:

sqlite> .dump footable
PRAGMA foreign_keys=OFF;
BEGIN TRANSACTION;
CREATE TABLE footable (
	fielda TEXT NOT NULL, 
	fieldb TEXT NOT NULL, 
	PRIMARY KEY (fielda)
);
INSERT INTO footable VALUES('dataa','datab');
CREATE INDEX ix_footable_fieldb ON footable (fieldb);
COMMIT;

and as you can see above in the dump, the correct data is in the database, just the object was created incorrectly.

[jmgurney]

Just ran a test, and it goes wrong here:

instance.pk = await self.database.execute(expr)

a print of the instance before that line has the data being correct. An option is that if pk is present in kwargs, simply don't do the assignment. I don't know the layers well enough to say what is correct though. Also, not sure how/if this addresses the case where multiple fields are the primary key.

[aminalaee]

@tomchristie @Nipsuli
The proposed solution will work in PostgreSQL but not in MySQL and SQLite.
SQLite does not support RETURNING clauses until version 3.35.0 and that is not still supported on current python versions.

For anyone interested, the reason this is happening is that SQLite uses a virtual column ROWID, after an INSERT is done, the driver will call last_insert_rowid() which returns an Integer ROWID.
This can be solved by using the RETURNING clause but that's not available yet.

As far as I know MySQL doesn't support that either.

Any feedback is appreciated.