litestar-org / advanced-alchemy

A carefully crafted, thoroughly tested, optimized companion library for SQLAlchemy
http://docs.advanced-alchemy.litestar.dev/
MIT License
217 stars 26 forks source link

Enhancement: make `.get_one` respect None filters #187

Open funkindy opened 4 months ago

funkindy commented 4 months ago

Summary

I use soft delete for my models, and i try to get instance with repo like this:

obj = await repo.get_one(id='id', deleted_at=None)

because i dont want instance to be selected if it is deleted.

The problem is that with these kwargs i dont get any object at all, because the filter doesn't get converted to deleted_at is null for the db query but to deleted_at = 'None'

Is it as intended? For now the workaround to this is to pass custom statement to get_one, but its not very convenient. Thank you.

Basic Example

No response

Drawbacks and Impact

No response

Unresolved questions

No response

peterschutt commented 4 months ago

Does this work?

from sqlalchemy import null

obj = await repo.get_one(id='id', deleted_at=null())
cofin commented 4 months ago

You can also do something like this:

from sqlalchemy import null

obj = await repo.get_one(MyModel.deleted_at.is_(None), id='id')

Depending on the data type used, the item.field = None will automatically convert to the IS NULL/IS NOT NULL syntax. But, not all work like this (or at least that's what I am currently remembering)

funkindy commented 4 months ago
from sqlalchemy import null

No, under the hood FilterableRepository._filter_by_where so i even cant render the statement in the debugger:

sqlalchemy.exc.CompileError: Could not render literal value "<sqlalchemy.sql.elements.Null object at 0x1075f13a0>" with datatype DATETIME; see parent stack trace for more detail.

Looks like SA tries to convert this explicilty into DATETIME:

 WHERE user.id = $1::VARCHAR AND user.deleted_at = $2::TIMESTAMP WITH TIME ZONE]
funkindy commented 4 months ago

You can also do something like this:

from sqlalchemy import null

obj = await repo.get_one(MyModel.deleted_at.is_(None), id='id')

Depending on the data type used, the item.field = None will automatically convert to the IS NULL/IS NOT NULL syntax. But, not all work like this (or at least that's what I am currently remembering)

Yes, this is exactly my current workaround. But it would be nice to have it in kwargs either with None or null()