BeanieODM / beanie

Asynchronous Python ODM for MongoDB
http://beanie-odm.dev/
Apache License 2.0
2.06k stars 217 forks source link

[BUG] wrong condition convertion on comparing two fields #959

Open Karmenzind opened 4 months ago

Karmenzind commented 4 months ago

Describe the bug I need to compare two fields with model.field1 > model.field2 but Beanio generated the wrong filter.

My model is like:

class H(Document):
    last_sms_time: datetime
    next_sms_time: datetime

And H.find(H.next_sms_time < H.last_sms_time).get_filter_query() returned {'next_sms_time': {'$lt': 'last_sms_time'}} that compares next_sms_time with literal string 'last_sms_time'.

But the correct filter that will be recognized by Mongodb should be:

{"$expr": {"$lt": ["$next_sms_time", "$last_sms_time"]}}

or with $where:

{"$where": "this.next_sms_time < this.last_sms_time"}

To Reproduce Sorry I will complete the sample later.

Expected behavior See above.

Additional context Nothing.

github-actions[bot] commented 3 months ago

This issue is stale because it has been open 30 days with no activity.

github-actions[bot] commented 2 months ago

This issue was closed because it has been stalled for 14 days with no activity.

staticxterm commented 1 month ago

Hi @Karmenzind, why would you want to compare the two class fields? It is more likely that you wanted to compare a field to some value, right?

Doing H.find(H.next_sms_time < H.last_sms_time) will return the output that you posted since Beanie internally treats all class fields as string values. So H.next_sms_time for example is simply just the string 'next_sms_time'.

Now, if you do for example H.find(H.next_sms_time < datetime.now()).get_filter_query() you would get something like {'next_sms_time': {'$lt': datetime.datetime(2024, 10, 6, 22, 28, 56, 238695)}}. And, depending on the data you have, if you did a full fledged query like so:

results = await H.find(H.last_sms_time < datetime.now()).to_list()

You would get back the results (and proving that the generated query is correct):

(Pdb) results
[H(id=ObjectId('6702ecccaec3343a6109c3a8'), revision_id=None, last_sms_time=datetime.datetime(2024, 10, 6, 20, 2, 20, 617000), next_sms_time=datetime.datetime(2024, 10, 7, 20, 2, 20, 617000))]

So the Find interface is stable, it simply does not behave the way you expected it would.

Karmenzind commented 4 weeks ago

Thanks for your reply. @staticxterm

It is more likely that you wanted to compare a field to some value, right?

No that's not what I want. The requirement here is to compare two dynamically changing fields, other than a fixed value. Because each document has its own last_sms_time. I understand Find interface is stable. Maybe you can try another format or interface to implement {"$expr": {"$lt": ["$next_sms_time", "$last_sms_time"]}}, which is natively supported by MongoDB itself.

staticxterm commented 4 weeks ago

Okay, thank you for the reply. I now understand your use case a bit better. I guess we could make this 'overload' of sorts for the Find query to use the already present Expr and do some magic if both sides of some operator are the model field names. Still not sure if this is a good idea due to the current behind the scenes logic. Pull requests are welcome! Moving from bug to feature request label. Thank you for the report!