encode / orm

An async ORM. 🗃
https://www.encode.io/orm
BSD 3-Clause "New" or "Revised" License
1.78k stars 98 forks source link

Primary key field not being added to any queries #143

Closed nexy7574 closed 2 years ago

nexy7574 commented 2 years ago

Checklist

Describe the bug

Despite the column id (the table's primary key) being included in queries explicitly via the name (id=), and via implicit (pk=), the same result is still yielded. I don't know what could be causing this as I've been using encode/orm since 0.2 and its worked fine, up until this one god forsaken project.

To reproduce

I'm honestly not too sure what's causing this so im going to provide my table, the queries I'm making, and some example data

# Define the table
import datetime
import orm
from databases import Database

registry = orm.ModelRegistry(Database("sqlite:///main.db"))

class User(orm.Model):
    tablename = "users"
    registry = registry
    fields = {
        "id": orm.BigInteger(primary_key=True),
        "access_token": orm.Text(default=None),
        "refresh_token": orm.Text(default=None),
        "expires": orm.DateTime(default=None),
        "session": orm.Text(default=None),
        "bio": orm.String(default="Default bio", min_length=2, max_length=4069),
    }

# elsewhere in an async function:
async def main():  # not production code, I'm running this in a fastapi server - but that's probably irrelivant 
    data = {
        "id": 421698654180012064,
        "access_token": "aoigniawghaoiwhgnagwuwgnuip",
        "refresh_token": "aiwngfuagnpauiwaiwbguiwgbuiawg",
        "expires": datetime.datetime.now() + datetime.timedelta(days=7),
        "session": "hello world"
    }
    await User.objects.create(**data)  # , pk=data["id"]) also raises the same error
    # I also tried with the below code, and it yielded a stranger error, but pretty much the same
    """
    await User.objects.update_or_create(id=data["id"], defaults=data)  # specifying pk still does nothing
    """

Expected behavior

An entry is correctly created

Actual behavior

Sometimes (unclear why) the id field autoincrements itself(???), or (more often than not) the code errors out with a not null constraint failure

Debugging material

Notable tracebacks:

Traceback from console ```python Authorized user: { 'id': '123456788989898989898', 'username': '...', 'avatar': 'redacted', 'discriminator': '0000', 'public_flags': 64, 'flags': 64, 'banner': None, 'banner_color': '#36393f', 'accent_color': 3553599, 'locale': 'en-GB', 'mfa_enabled': True, 'email': 'redacted@gmail.com', 'verified': True } Authorized user's auth data: {'access_token': 'awdadaawdawdawd', 'token_type': 'Bearer', 'expires_in': 604800, 'refresh_token': 'dawsdwadswasd', 'scope': 'identify email guilds'} INFO: 127.0.0.1:44550 - "GET /callbacks/authorise?code=redacted HTTP/1.1" 500 Internal Server Error ERROR: Exception in ASGI application Traceback (most recent call last): File "/home/nexus/PycharmProjects/Unnamed-bot-List/venv/lib/python3.9/site-packages/uvicorn/protocols/http/httptools_impl.py", line 376, in run_asgi result = await app(self.scope, self.receive, self.send) File "/home/nexus/PycharmProjects/Unnamed-bot-List/venv/lib/python3.9/site-packages/uvicorn/middleware/proxy_headers.py", line 75, in __call__ return await self.app(scope, receive, send) File "/home/nexus/PycharmProjects/Unnamed-bot-List/venv/lib/python3.9/site-packages/uvicorn/middleware/debug.py", line 96, in __call__ raise exc from None File "/home/nexus/PycharmProjects/Unnamed-bot-List/venv/lib/python3.9/site-packages/uvicorn/middleware/debug.py", line 93, in __call__ await self.app(scope, receive, inner_send) File "/home/nexus/PycharmProjects/Unnamed-bot-List/venv/lib/python3.9/site-packages/fastapi/applications.py", line 208, in __call__ await super().__call__(scope, receive, send) File "/home/nexus/PycharmProjects/Unnamed-bot-List/venv/lib/python3.9/site-packages/starlette/applications.py", line 112, in __call__ await self.middleware_stack(scope, receive, send) File "/home/nexus/PycharmProjects/Unnamed-bot-List/venv/lib/python3.9/site-packages/starlette/middleware/errors.py", line 181, in __call__ raise exc File "/home/nexus/PycharmProjects/Unnamed-bot-List/venv/lib/python3.9/site-packages/starlette/middleware/errors.py", line 159, in __call__ await self.app(scope, receive, _send) File "/home/nexus/PycharmProjects/Unnamed-bot-List/venv/lib/python3.9/site-packages/starlette/exceptions.py", line 82, in __call__ raise exc File "/home/nexus/PycharmProjects/Unnamed-bot-List/venv/lib/python3.9/site-packages/starlette/exceptions.py", line 71, in __call__ await self.app(scope, receive, sender) File "/home/nexus/PycharmProjects/Unnamed-bot-List/venv/lib/python3.9/site-packages/starlette/routing.py", line 656, in __call__ await route.handle(scope, receive, send) File "/home/nexus/PycharmProjects/Unnamed-bot-List/venv/lib/python3.9/site-packages/starlette/routing.py", line 259, in handle await self.app(scope, receive, send) File "/home/nexus/PycharmProjects/Unnamed-bot-List/venv/lib/python3.9/site-packages/starlette/routing.py", line 61, in app response = await func(request) File "/home/nexus/PycharmProjects/Unnamed-bot-List/venv/lib/python3.9/site-packages/fastapi/routing.py", line 226, in app raw_response = await run_endpoint_function( File "/home/nexus/PycharmProjects/Unnamed-bot-List/venv/lib/python3.9/site-packages/fastapi/routing.py", line 159, in run_endpoint_function return await dependant.call(**values) File "/home/nexus/PycharmProjects/Unnamed-bot-List/server/src/main.py", line 55, in authorise_user user_db = await User.objects.create( File "/home/nexus/PycharmProjects/Unnamed-bot-List/venv/lib/python3.9/site-packages/orm/models.py", line 412, in create instance.pk = await self.database.execute(expr) File "/home/nexus/PycharmProjects/Unnamed-bot-List/venv/lib/python3.9/site-packages/databases/core.py", line 169, in execute return await connection.execute(query, values) File "/home/nexus/PycharmProjects/Unnamed-bot-List/venv/lib/python3.9/site-packages/databases/core.py", line 295, in execute return await self._connection.execute(built_query) File "/home/nexus/PycharmProjects/Unnamed-bot-List/venv/lib/python3.9/site-packages/databases/backends/sqlite.py", line 128, in execute await cursor.execute(query_str, args) File "/home/nexus/PycharmProjects/Unnamed-bot-List/venv/lib/python3.9/site-packages/aiosqlite/cursor.py", line 37, in execute await self._execute(self._cursor.execute, sql, parameters) File "/home/nexus/PycharmProjects/Unnamed-bot-List/venv/lib/python3.9/site-packages/aiosqlite/cursor.py", line 31, in _execute return await self._conn._execute(fn, *args, **kwargs) File "/home/nexus/PycharmProjects/Unnamed-bot-List/venv/lib/python3.9/site-packages/aiosqlite/core.py", line 129, in _execute return await future File "/home/nexus/PycharmProjects/Unnamed-bot-List/venv/lib/python3.9/site-packages/aiosqlite/core.py", line 102, in run result = function() sqlite3.IntegrityError: NOT NULL constraint failed: users.id ```
From debug logs (via `logging` module): ``` DEBUG:databases:Query: INSERT INTO users (access_token, refresh_token, expires, session, bio) VALUES (?, ?, ?, ?, ?) Args: ('redacted (access token)', 'redacted (refresh token)', '2021-12-27 14:32:28.747869', 'session token', 'Default bio. Go to user settings -> bio to change.') DEBUG:aiosqlite:executing functools.partial() DEBUG:aiosqlite:operation functools.partial() completed DEBUG:aiosqlite:executing functools.partial(, 'INSERT INTO users (access_token, refresh_token, expires, session, bio) VALUES (?, ?, ?, ?, ?)', ['access token', 'refresh token', '2021-12-27 14:32:28.747869', 'session token', 'Default bio. Go to user settings -> bio to change.']) DEBUG:aiosqlite:returning exception NOT NULL constraint failed: users.id ```
Traceback from web debugger ``` 500 Server Error Traceback (most recent call last): File "/home/nexus/PycharmProjects/Unnamed-bot-List/venv/lib/python3.9/site-packages/starlette/middleware/errors.py", line 159, in __call__ await self.app(scope, receive, _send) File "/home/nexus/PycharmProjects/Unnamed-bot-List/venv/lib/python3.9/site-packages/starlette/exceptions.py", line 82, in __call__ raise exc File "/home/nexus/PycharmProjects/Unnamed-bot-List/venv/lib/python3.9/site-packages/starlette/exceptions.py", line 71, in __call__ await self.app(scope, receive, sender) File "/home/nexus/PycharmProjects/Unnamed-bot-List/venv/lib/python3.9/site-packages/starlette/routing.py", line 656, in __call__ await route.handle(scope, receive, send) File "/home/nexus/PycharmProjects/Unnamed-bot-List/venv/lib/python3.9/site-packages/starlette/routing.py", line 259, in handle await self.app(scope, receive, send) File "/home/nexus/PycharmProjects/Unnamed-bot-List/venv/lib/python3.9/site-packages/starlette/routing.py", line 61, in app response = await func(request) File "/home/nexus/PycharmProjects/Unnamed-bot-List/venv/lib/python3.9/site-packages/fastapi/routing.py", line 226, in app raw_response = await run_endpoint_function( File "/home/nexus/PycharmProjects/Unnamed-bot-List/venv/lib/python3.9/site-packages/fastapi/routing.py", line 159, in run_endpoint_function return await dependant.call(**values) File "/home/nexus/PycharmProjects/Unnamed-bot-List/server/src/main.py", line 55, in authorise_user user_db = await User.objects.create( File "/home/nexus/PycharmProjects/Unnamed-bot-List/venv/lib/python3.9/site-packages/orm/models.py", line 412, in create instance.pk = await self.database.execute(expr) File "/home/nexus/PycharmProjects/Unnamed-bot-List/venv/lib/python3.9/site-packages/databases/core.py", line 169, in execute return await connection.execute(query, values) File "/home/nexus/PycharmProjects/Unnamed-bot-List/venv/lib/python3.9/site-packages/databases/core.py", line 295, in execute return await self._connection.execute(built_query) File "/home/nexus/PycharmProjects/Unnamed-bot-List/venv/lib/python3.9/site-packages/databases/backends/sqlite.py", line 128, in execute await cursor.execute(query_str, args) File "/home/nexus/PycharmProjects/Unnamed-bot-List/venv/lib/python3.9/site-packages/aiosqlite/cursor.py", line 37, in execute await self._execute(self._cursor.execute, sql, parameters) File "/home/nexus/PycharmProjects/Unnamed-bot-List/venv/lib/python3.9/site-packages/aiosqlite/cursor.py", line 31, in _execute return await self._conn._execute(fn, *args, **kwargs) File "/home/nexus/PycharmProjects/Unnamed-bot-List/venv/lib/python3.9/site-packages/aiosqlite/core.py", line 129, in _execute return await future File "/home/nexus/PycharmProjects/Unnamed-bot-List/venv/lib/python3.9/site-packages/aiosqlite/core.py", line 102, in run result = function() sqlite3.IntegrityError: NOT NULL constraint failed: users.id During handling of the above exception, another exception occurred: Traceback (most recent call last): File "/home/nexus/PycharmProjects/Unnamed-bot-List/venv/lib/python3.9/site-packages/uvicorn/middleware/debug.py", line 93, in __call__ await self.app(scope, receive, inner_send) File "/home/nexus/PycharmProjects/Unnamed-bot-List/venv/lib/python3.9/site-packages/fastapi/applications.py", line 208, in __call__ await super().__call__(scope, receive, send) File "/home/nexus/PycharmProjects/Unnamed-bot-List/venv/lib/python3.9/site-packages/starlette/applications.py", line 112, in __call__ await self.middleware_stack(scope, receive, send) File "/home/nexus/PycharmProjects/Unnamed-bot-List/venv/lib/python3.9/site-packages/starlette/middleware/errors.py", line 165, in __call__ response = self.debug_response(request, exc) File "/home/nexus/PycharmProjects/Unnamed-bot-List/venv/lib/python3.9/site-packages/starlette/middleware/errors.py", line 245, in debug_response content = self.generate_html(exc) File "/home/nexus/PycharmProjects/Unnamed-bot-List/venv/lib/python3.9/site-packages/starlette/middleware/errors.py", line 217, in generate_html traceback_obj = traceback.TracebackException.from_exception( File "/home/nexus/.pyenv/versions/3.9.7/lib/python3.9/traceback.py", line 538, in from_exception return cls(type(exc), exc, exc.__traceback__, *args, **kwargs) File "/home/nexus/.pyenv/versions/3.9.7/lib/python3.9/traceback.py", line 517, in __init__ self.stack = StackSummary.extract( File "/home/nexus/.pyenv/versions/3.9.7/lib/python3.9/traceback.py", line 359, in extract result.append(FrameSummary( File "/home/nexus/.pyenv/versions/3.9.7/lib/python3.9/traceback.py", line 260, in __init__ self.locals = {k: repr(v) for k, v in locals.items()} if locals else None File "/home/nexus/.pyenv/versions/3.9.7/lib/python3.9/traceback.py", line 260, in self.locals = {k: repr(v) for k, v in locals.items()} if locals else None File "/home/nexus/PycharmProjects/Unnamed-bot-List/venv/lib/python3.9/site-packages/orm/models.py", line 493, in __repr__ return f"<{self.__class__.__name__}: {self}>" File "/home/nexus/PycharmProjects/Unnamed-bot-List/venv/lib/python3.9/site-packages/orm/models.py", line 496, in __str__ return f"{self.__class__.__name__}({self.pkname}={self.pk})" File "/home/nexus/PycharmProjects/Unnamed-bot-List/venv/lib/python3.9/site-packages/orm/models.py", line 486, in pk return getattr(self, self.pkname) AttributeError: 'User' object has no attribute 'id' ```

Environment

aminalaee commented 2 years ago

Just to be clear, what's the schema of the table? How do you create the table?

nexy7574 commented 2 years ago

Just to be clear, what's the schema of the table? How do you create the table?

await registry.create_all()

SQLite version 3.37.0 2021-11-27 14:13:22
Enter ".help" for usage hints.
sqlite> .d
PRAGMA foreign_keys=OFF;
BEGIN TRANSACTION;
CREATE TABLE users (
        id BIGINT NOT NULL, 
        access_token TEXT NOT NULL, 
        refresh_token TEXT NOT NULL, 
        expires DATETIME NOT NULL, 
        session TEXT NOT NULL, 
        bio VARCHAR(4069) NOT NULL, 
        PRIMARY KEY (id)
);
COMMIT;
sqlite> 
aminalaee commented 2 years ago

Thanks for the input.

Did you use BIGINT in previous versions too? I'm not sure if SQLite allows BIGINT as primary key.

nexy7574 commented 2 years ago

Yep. INT also causes the same issue. I also think I had the same issue when it was text, so I don't think the column type matters

nexy7574 commented 2 years ago

I just changed the id column to orm.Text(...), and now I get this warning in the console when trying to write a value to it:

/home/nexus/PycharmProjects/Unnamed-bot-List/venv/lib/python3.9/site-packages/databases/backends/sqlite.py:160: SAWarning: Column 'users.id' is marked as a member of the primary key for table 'users', but has no Python-side or server-side default generator indicated, nor does it indicate 'autoincrement=True' or 'nullable=True', and no explicit value is passed.  Primary key columns typically may not store NULL.
nexy7574 commented 2 years ago
sqlite> PRAGMA main.table_info("users");
0|id|TEXT|1||1
1|access_token|TEXT|1||0
2|refresh_token|TEXT|1||0
3|expires|DATETIME|1||0
4|session|TEXT|1||0
5|bio|VARCHAR(4069)|1||0
sqlite> INSERT INTO users (id, access_token, refresh_token, expires, session, bio) VALUES (421698654189912064, "foobar", "barfoo", "2021-12-20T17:01:56.066857", "hello world", "cool bio");
sqlite> SELECT * FROM users;
421698654189912064|foobar|barfoo|2021-12-20T17:01:56.066857|hello world|cool bio

Doesn't look like its an issue with the generated schema

with BIGINT as pk type (no effect) ``` sqlite> PRAGMA main.table_info("users"); 0|id|BIGINT|1||1 1|access_token|TEXT|1||0 2|refresh_token|TEXT|1||0 3|expires|DATETIME|1||0 4|session|TEXT|1||0 5|bio|VARCHAR(4069)|1||0 sqlite> INSERT INTO users (id, access_token, refresh_token, expires, session, bio) VALUES (421698654189912064, "foobar", "barfoo", "2021-12-20T17:01:56.066857", "hello world", "cool bio"); sqlite> SELECT * FROM users; 421698654189912064|foobar|barfoo|2021-12-20T17:01:56.066857|hello world|cool bio ```
aminalaee commented 2 years ago

A few points here:

When using the orm.Text you're getting a warning which is explaining the warning, because you should either set that to non nullable or specify a default. Maybe a method returning str(uuid.uuid4())?

And for the BIGINT or INTEGER from SQLite docs:

With one exception noted below, if a rowid table has a primary key that consists of a single column and the declared type of that column is "INTEGER" in any mixture of upper and lower case, then the column becomes an alias for the rowid. Such a column is usually referred to as an "integer primary key". A PRIMARY KEY column only becomes an integer primary key if the declared type name is exactly "INTEGER". Other integer type names like "INT" or "BIGINT" or "SHORT INTEGER" or "UNSIGNED INTEGER" causes the primary key column to behave as an ordinary table column with integer affinity and a unique index, not as an alias for the rowid.

I think the error you mentioned of unstable auto increment is explained here, INTEGER PRIMARY KEY will be a shortcut to rowid but BIGINT PRIMARY KEY won't be. And as far as I know SQLite doesn't have BIGINT type, so you'd rather use INT PRIMARY KEY.

Anyways please try again with the Integer and see if it solves your issue.

nexy7574 commented 2 years ago

Oh my bad, I thought sqlite made all primary keys non-nullalbe (coming from postgres which seemed to do the same thing)

Integer(primary_key=True, allow_null=False) fixed the issue. Thank you @aminalaee :1st_place_medal:

nexy7574 commented 2 years ago

I retract my statement. Instead of inserting the actual data, it just autoincremented the id column again.

(Just to clarify, the value I'm passing to id in objects.create isn't being passed to the query when it should be, I don't want a default.)

aminalaee commented 2 years ago

Yeah, I think that's done by decision.

You shouldn't pass the primary key to the query. You either set on on python-side by specifying a default like this:

https://github.com/encode/orm/blob/c6b9c0e7a2fee25509a75d1983f203a334376e95/tests/test_columns.py#L45-L53

Or by letting the database to set it:

https://github.com/encode/orm/blob/c6b9c0e7a2fee25509a75d1983f203a334376e95/tests/test_columns.py#L28-L42

nexy7574 commented 2 years ago

Wait.. so can I not specify my own primary key value? does it have to be decided by the db/default?

aminalaee commented 2 years ago

Yeah, it's removed here https://github.com/encode/orm/blob/master/orm/models.py#L402

I'm just saying this is expected, if you find maybe Django or SQLAlchemy have a different behaviour, we could change it.

nexy7574 commented 2 years ago

Ah I see, just a misunderstanding on my part then 😅

Thanks again