ObjectId comparison about mingo HOT 6 CLOSED

Thanks for report @stalniy

Unfortunately, mingo only tests for identity when custom objects are supplied. It has no knowledge of the native MongoDB ObjectId type.

That said, it makes sense to run custom equality checks for values of the same type on a best effort basis such as: comparing their toJSON() or toString() methods if present. I will consider this in the next version.

You can temporarily check against the string value of ObjectId as a workaround.

On a side note, I wrote Mingo first to target the client environment. Depending on the collection size, it will be more performant using the MongoDB query library directly if you are on the server.

from mingo.

@kofrasa ObjectID comparison was not completely fixed. There are several cases which doesn't work:

// oid - ObjectId constructor, id - instance of ObjectId

> new Query({ id }).test({ id: id })
true
> new Query({ id }).test({ id: oid(id.toString()) })
true

// the next cases fails
> new Query({ id }).test({ id: id.toString() })
false
> new Query({ id }).test({ id: id.toString() })
false
> new Query({ id: id.toString() }).test({ id })

from mingo.

Hi @stalniy

User-defined types can only be compared on a best effort basis with the requirement that operands are of the same type (see my earlier comment), so this is expected behaviour.

That means, comparing ObjectId against String will not work. However, two identical ObjectId values will compare correctly (as your second tests show). Previously, the only check for identity was using the === operator but the current release has improved on it by using hashes.

The library is not aware of custom types including ones defined by MongoDB. It is trivial for users to pass an identical type, or use a primitive javascript type for the operands. I think the behaviour is sound as is.

I am open to discuss if you have a different viewpoint.

Thanks

from mingo.

Probably the best option is to allow to specify custom comparator for client types. For ObjectId it may look like:

function compareObjectId(idA, idB) {
  const [valueA, valueB] = [String(idA), String(idB)]

  if (valueA === valueB) {
    return 0
  }

  return valueA > valueB ? 1 : -1
}

Then

mingo.setup({
   comparators: {
      ObjectId: compareObjectId
   }
})

what do you think?

from mingo.

This pattern can be confusing. I presume the use will be such: For unequal types, check whether the type name of any of the operands has a comparator. Use that to compare the types.

The problem lies in checking either operand (what order?). If the operands aren't restricted to be of the same type, then one can also mistakenly register different types which compare against each other circularly.

Example:

comparators: {
  A: aCompareB,
  B: bCompareC,
  C: cCompareA
}

Obviously this is not sound, but mingo cannot tell. I did consider adding a comparator for the purposes of sorting custom types but opted out for simplicity. Even so, the restriction that both operands are of the same type would still hold to retain the principle of least surprise.

So even with comparators subject to the restrictions mentioned, your examples will still not pass.
It does seem to me that it is still simpler to use JS native types to avoid surprising semantics.

from mingo.

I have same problem. I need a sort on ObjectId objects. Why not use valueOf function? Or convert object to string? Or allow specify to library a object to primitive function converter for compare.

from mingo.