collerek / ormar

python async orm with fastapi in mind and pydantic validation
https://collerek.github.io/ormar/
MIT License
1.66k stars 87 forks source link

'str' object has no attribute 'toordinal' #541

Open sondrelg opened 2 years ago

sondrelg commented 2 years ago

Describe the bug Hi!

After the latest update (0.10.24), it looks like querying for dates, using strings, is no longer working.

Querying with the plain string value worked before (on 10.23).

Stack trace

../../.virtualenvs/project/lib/python3.10/site-packages/ormar/queryset/queryset.py:948: in get
    return await self.filter(*args, **kwargs).get()
../../.virtualenvs/project/lib/python3.10/site-packages/ormar/queryset/queryset.py:968: in get
    rows = await self.database.fetch_all(expr)
../../.virtualenvs/project/lib/python3.10/site-packages/databases/core.py:149: in fetch_all
    return await connection.fetch_all(query, values)
../../.virtualenvs/project/lib/python3.10/site-packages/databases/core.py:271: in fetch_all
    return await self._connection.fetch_all(built_query)
../../.virtualenvs/project/lib/python3.10/site-packages/databases/backends/postgres.py:174: in fetch_all
    rows = await self._connection.fetch(query_str, *args)
../../.virtualenvs/project/lib/python3.10/site-packages/asyncpg/connection.py:601: in fetch
    return await self._execute(
../../.virtualenvs/project/lib/python3.10/site-packages/asyncpg/connection.py:1639: in _execute
    result, _ = await self.__execute(
../../.virtualenvs/project/lib/python3.10/site-packages/asyncpg/connection.py:1664: in __execute
    return await self._do_execute(
../../.virtualenvs/project/lib/python3.10/site-packages/asyncpg/connection.py:1711: in _do_execute
    result = await executor(stmt, None)
asyncpg/protocol/protocol.pyx:183: in bind_execute
    ???
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ 

>   ???
E   asyncpg.exceptions.DataError: invalid input for query argument $2: '2022-01-20' ('str' object has no attribute 'toordinal')

Let me know if you want me to try and create a reproducible example. I thought I would open the issue first, in case you immediately knew what might have changed.

Thanks for maintaining the package! 👏

collerek commented 2 years ago

Hmm, what changed is that I no longer stringify the queries as that didn't work with complex types (i.e. postgres arays).

Now I pass sqlalchemy query and bindparams to databases and it seems that asyncpg does not accept string as dates without extra work. https://github.com/MagicStack/asyncpg/issues/692

If you provide a meaningful example I can try to work this out/around either here or in databases.

sondrelg commented 2 years ago

This replicates it for me 👍 Really only the model and query is enough

import asyncio
from datetime import datetime, date
from typing import Optional

import ormar
import sqlalchemy
from databases import Database
from app.core.config import settings
from app.main import startup, shutdown
from sqlalchemy import func

url = settings.DATABASE_URL.replace(scheme='postgresql')

if not settings.TESTING:
    database = Database(url=url)
else:
    # Roll back the test database to a clean state between each test case
    database = Database(url=url, force_rollback=True)

metadata = sqlalchemy.MetaData()

class BaseMeta:
    metadata = metadata
    database = database

class ModelBase(ormar.Model):
    id: int = ormar.Integer(autoincrement=True, primary_key=True)
    created_at: datetime = ormar.DateTime(timezone=True, server_default=func.now())
    updated_at: datetime = ormar.DateTime(timezone=True, server_default=func.now(), onupdate=func.now())

    class Meta:
        abstract = True

class SomeModel(ModelBase):
    due_date: Optional[date] = ormar.Date(nullable=True)

    class Meta(BaseMeta):
        tablename = 'somemodels'

async def main():
    await startup()
    await SomeModel.objects.get(due_date='2022-01-20')
    await shutdown()

if __name__ == '__main__':
    asyncio.run(main())
sondrelg commented 2 years ago

Looks like querying for an integer with a string is also no longer supported. This seems like a slight regression in DX 🙂

DataError("invalid input for query argument $1: '342254' ('str' object cannot be interpreted as an integer)")

Would it be possible to revert to the old behaviour? Would that break complex types again?

collerek commented 2 years ago

Yes, unfortunately it would break complex types, it was also using not really safe literal binds docs. Although I was re-parsing it to sqlalchemy text clause you couldn't inject SQL it was still against best practices, and was only working for simple types (didn't work for example with dates, I had to dump them to isoformat string, which in turn was breaking the timezones 😱 )

As a quick workaround can you try the other driver from databases? Maybe it's more permissive (aiopg instead of asyncpg)? Or maybe that needs to be fixed in databases itself, will check that.

sondrelg commented 2 years ago

It looks like aiopg handled the date casting like before. I'll use this for a bit and report back, but looks good 👍