-
Notifications
You must be signed in to change notification settings - Fork 98
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
$eq operator is broken with certain string values #1000
Comments
Hehe, sorry, I'm responsible for this one! Without it all virtual columns using numbers were broken :) Any quickfixes you could think of? We could check if each character is in the 0-9 range? That would still have incorrect edge cases, such as the scientific notation here, but would be more in line with the expectations? |
@Helveg thanks for getting back to me. I opened a PR with what might be a way to address this. Let me know what you think. |
Hi guys! Note that prepended zeros are required too as field model in database contains text data type. |
We could fix this by treating strings with leading zeros as strings and not numbers. |
@Helveg Sounds like there could be more potential edge cases out there. I can't help but feel that perhaps this should be handled differently. Perhaps, type coercion / casting should be opt-in, so that it doesn't happen at all unless the user asks for it - that would avoid having to cater for edge cases in the nestjs-paginate code. |
Yea, there's similar problems with treating booleans and null (see a couple of other open issues). One approach is to go by TypeORM column type perhaps? I think the end goal user experience is that for 99.999% of cases you should be able to simply give your value, and that we figure it out. |
That does indeed sound ideal. I'll try and see if that's easy for me to implement in the open PR. |
@Helveg I had a quick look and couldn't see an easy/obvious way to implement this. Let me know if you have time to take a look, if not then I will invest more time in digging. |
You can propagate the You can find columns/relations quite easily, but I'm not sure how easy it is to deal with nested relationships and columns. I'm sure that somewhere else in the codebase we deal with this already. |
Steps to reproduce
Perform a filter query with an
$eq
filter with a string that contains "what JS believes to be a number, but that is actually a string"™For example,
filter.referenceId
=$eq:651581942751358e5
Expected result
Elements with the given filter are found.
Actual result
No elements are found
Additional notes
This regression was introduced in #952. It happens because the above string passes the
Number.isNumber
check, even though the value is not meant to be a number.In the example above:
The
Number()
conversion is not problematic with values that are actually numbers - but unfortunately JS has a loose interpretation of which strings actually qualify as numbers.The text was updated successfully, but these errors were encountered: