[SERVER-31773] BSONObj comparison Created: 31/Oct/17 Updated: 03/Nov/17 Resolved: 03/Nov/17 |
|
| Status: | Closed |
| Project: | Core Server |
| Component/s: | Internal Code, Querying |
| Affects Version/s: | None |
| Fix Version/s: | None |
| Type: | Improvement | Priority: | Minor - P4 |
| Reporter: | Simon | Assignee: | David Storch |
| Resolution: | Won't Fix | Votes: | 0 |
| Labels: | None | ||
| Remaining Estimate: | Not Specified | ||
| Time Spent: | Not Specified | ||
| Original Estimate: | Not Specified | ||
| Sprint: | Query 2017-11-13 |
| Participants: |
| Description |
|
The DeferredComparison for BSONObj uses BSONObj::woCompare This misses the opportunity to use the DeferredComparison of BSONElement for overloaded comparison operators. Although this is a strong constraint in the current implementation, this does not allow to specify a different BSONElement comparison implementaton using the DeferredComparison mechanism that could be used instead. Potential usage: implicit type conversion |
| Comments |
| Comment by David Storch [ 03/Nov/17 ] |
|
I see, thanks for the additional detail. Yes, we could change the various implementations of BSONObj::ComparatorInterface::compare() to use BSONElement::ComparatorInterface, though you're right that we would have to do something to avoid code duplication with BSONObj::woCompare(). This is not something we're planning to pursue in the near future, however. Best, |
| Comment by Simon [ 03/Nov/17 ] |
|
Hello David The fact that woCompare() exists shows that there are several comparison modes possible between objects (whether BSONObj, BSONElement or else). For a consistence point of view, in my opinion, it would make sense that
The fact that the current implementation all use woCompare() under the hood should not imply that shortcuts are taken and that BSONObj operator< use directly BSONElement::woCompare() for instance (that is not currently the case, this is for exemple only). Although this means probably duplicating some comparison code at this time (because they all currently use woCompare()), this would allow for a more modular and fine-tuned comparison logic. If woCompare() is always the desired comparison mode, why not always call it explicitely ? Meanwhile, I understand that this ticket is not about a bug and that it does not bring direct new functionalities considering the amount of rework it may imply and intensive regression tests so it is Ok with me as "Won't Fix" even though I believe it could bring a valuable improvement to the consistency and reliability of the internal code. |
| Comment by David Storch [ 03/Nov/17 ] |
|
Hi Uyttendaele, If I understand correctly, you are suggesting a pure refactor in which we change BSONObj::woCompare() to be implemented in terms of the BSONElement::ComparatorInterface. While this is certainly an interesting idea, the MongoDB query engineering team does not see the value in pursuing this refactor right now. It is certainly the case that we could implement many interesting BSONObj or BSONElement comparators, such as those that can compare with implicit type conversion. However, this work seems separate from the pure refactor you suggest. I am closing this ticket as Won't Fix. Please let me know if we misunderstood something, or if you have a more compelling reason for why our engineering team should consider this work. And thanks for your interest in MongoDB---several of the SERVER tickets you filed recently contain insightful observations! Best, |
| Comment by Mark Agarunov [ 31/Oct/17 ] |
|
Hello Uyttendaele, Thank you for the detailed description of this behavior. I've set the fixVersion to "Needs Triage" for this new feature to be scheduled against our currently planned work. Updates will be posted on this ticket as they are available. Thanks, |