[SERVER-3366] ! NumberInt(0) is broken Created: 05/Jul/11  Updated: 12/Jul/16  Resolved: 06/Jul/11

Status: Closed
Project: Core Server
Component/s: JavaScript
Affects Version/s: None
Fix Version/s: 1.9.1

Type: Bug Priority: Major - P3
Reporter: Eliot Horowitz (Inactive) Assignee: Antoine Girbal
Resolution: Done Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Operating System: ALL
Participants:

 Description   

see jstests/numberint.js



 Comments   
Comment by Antoine Girbal [ 06/Jul/11 ]

ok I fixed the behavior in SM, see SERVER-854

Comment by Antoine Girbal [ 06/Jul/11 ]

ok I fixed the behavior in SM, see SERVER-854

Comment by Eliot Horowitz (Inactive) [ 06/Jul/11 ]

Should do this before 1.9.1 is release as could break things in bad ways.

Comment by Eliot Horowitz (Inactive) [ 06/Jul/11 ]

What's really important is normal document road tripping - i.e. load modify save, and having an explicit way to create one.

So I think #2 is good.

Comment by Antoine Girbal [ 06/Jul/11 ]

the Integer is an internal C++ class.
when manipulating that value (say to/from json) it would not carry any information forcing it to be an integer.
So it would be equivalent to solution #2 above:

  • ask v8 if a number is an int with isInt32(), if so serialize it to integer in BSON.
  • can still have explicit NumberInt method but it would just return an Integer value. No way to change toString() etc
Comment by Eliot Horowitz (Inactive) [ 06/Jul/11 ]

In v8 it seems there is an Integer class?
Are you using that?
That should handle this correctly, no?

Comment by Antoine Girbal [ 06/Jul/11 ]

I see that you assigned it to 1.9.2.
Do you consider not releasing 1.9.1 until it's fixed?

possible solutions:
1) just revert back and not support explicit integer

2) make the use of integer implicit.
When converting to bson if value is integer, would be stored as integer.
this is easy with v8, but again with any implicit automatic conversion there could be side effects

3) use v8 and modify source to change behavior of "!" to call a method if an object.
For example it would return the value of toBoolean if exists.
downside is that we would be forced to use v8 and stuck with our own source.

Comment by Eliot Horowitz (Inactive) [ 06/Jul/11 ]

Yes - its currently breaking various unit tests, so certainly could break a lot of code in the wild with possibly disastrous consequences.

Comment by Antoine Girbal [ 06/Jul/11 ]

I doubt it.
There is nothing in js standard to allow overriding this behavior, the only standard ones are toString() and valueOf() (and then equals, compareTo).
The only workaround is to do a toNumber()
SECONDARY> n = NumberInt(0)
NumberInt(0)
SECONDARY> !n.toNumber()
true
SECONDARY> !n
false

what case do you think it may be an issue?
If someone uses an integer value as a flag and then tries to check it from JS?

Comment by Eliot Horowitz (Inactive) [ 06/Jul/11 ]

Is it fixable in v8?

Comment by Antoine Girbal [ 05/Jul/11 ]

not sure that this is fixable..
JS considers any non null object as true it converts it to boolean.
I have not seen any way to change that behavior (like overloading x.asBoolean() or Boolean)

Comment by auto [ 05/Jul/11 ]

Author:

{u'login': u'erh', u'name': u'Eliot Horowitz', u'email': u'eliot@10gen.com'}

Message: SERVER-3366 test
Branch: master
https://github.com/mongodb/mongo/commit/14688132ca56d89f2c6d45e6cac0caa6c5e91b94

Generated at Thu Feb 08 03:02:52 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.