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

fix Bson.opEquals: order of object keys is not determined #2183

Merged
merged 1 commit into from Jul 15, 2018

Conversation

Projects
None yet
3 participants
@IgorStepanov
Copy link
Contributor

commented Jul 6, 2018

Subj. This PR is needed for druntime#2243
If it is possible, make a new tag if this will be approved, please.

@IgorStepanov

This comment has been minimized.

Copy link
Contributor Author

commented Jul 7, 2018

Looks like that fail unrelated with this PR.

@wilzbach

This comment has been minimized.

Copy link
Member

commented Jul 7, 2018

Yes, Vibe.d currently doesn't compile with 2.081. A new patch release is under its way.

@IgorStepanov

This comment has been minimized.

Copy link
Contributor Author

commented Jul 9, 2018

Ping

@wilzbach

This comment has been minimized.

Copy link
Member

commented Jul 11, 2018

Ping @s-ludwig - are you okay with this?
BTW doesn't suffer the Json implementation from the same flaw?

@wilzbach

This comment has been minimized.

Copy link
Member

commented Jul 11, 2018

(going to merge this if there are no objections within the next 72h)

@s-ludwig

This comment has been minimized.

Copy link
Member

commented Jul 11, 2018

I wonder what the correct approach here is, or whether there is one (the spec doesn't seem to be helpful). It could be argued that the order is part of the semantics. In fact, MongoDB appears to compare objects including their order.

@wilzbach

This comment has been minimized.

Copy link
Member

commented Jul 11, 2018

I wonder what the correct approach here is, or whether there is one (the spec doesn't seem to be helpful). It could be argued that the order is part of the semantics. I

The spec is pretty clear on that:

Implementation Defined: The built-in associative arrays do not preserve the order of the keys inserted into the array. In particular, in a foreach loop the order in which the elements are iterated is typically unspecified.

https://dlang.org/spec/hash-map.html

Of course, it might make sense for Bson to have a defined order, but you can't rely on built-in arrays for this.

@IgorStepanov

This comment has been minimized.

Copy link
Contributor Author

commented Jul 14, 2018

@s-ludwig @wilzbach ping.

@s-ludwig, If we want to create Bson implementation which preserve fields order, we should rewrite Bson and don`t use associative array anywhere. This PR fixes univocal trouble of current implementation.

@wilzbach wilzbach merged commit 94729e7 into vibe-d:master Jul 15, 2018

3 checks passed

codecov/patch 45.454% of diff hit (target 59.163%)
Details
continuous-integration/appveyor/pr AppVeyor build succeeded
Details
continuous-integration/travis-ci/pr The Travis CI build passed
Details
@wilzbach

This comment has been minimized.

Copy link
Member

commented Jul 15, 2018

As releasing a new version is a bit more difficult for Vibe.d, I set the version to be tested at the project tester to master: dlang/ci#241

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.