Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

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

Open
Karmenzind opened this issue Jul 2, 2024 · 5 comments
Open

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

Karmenzind opened this issue Jul 2, 2024 · 5 comments

Comments

@Karmenzind
Copy link

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.

Copy link
Contributor

github-actions bot commented Aug 2, 2024

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

@github-actions github-actions bot added the Stale label Aug 2, 2024
Copy link
Contributor

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

@adeelsohailahmed adeelsohailahmed added bug Something isn't working and removed Stale labels Aug 27, 2024
@staticxterm
Copy link

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
Copy link
Author

Karmenzind commented Oct 7, 2024

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
Copy link

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!

@staticxterm staticxterm added feature-request and removed bug Something isn't working labels Oct 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

3 participants