Is it mgo itself that is changing the order internally? It should not do that.
On Wed, Jun 8, 2016 at 8:00 AM, roger peppe <[email protected]> wrote: > OK, I understand now, I think. > > The underlying problem is that subdocument searches in MongoDB > are order-sensitive. > > For example, I just tried this in a mongo shell: > > > db.foo.insert({_id: "one", x: {a: 1, b: 2}}) > > db.foo.find({x: {a: 1, b: 2}}) > { "_id" : "one", "x" : { "a" : 1, "b" : 2 } } > > db.foo.find({x: {b: 2, a: 1}}) > > > > The second find doesn't return anything even though it contains > the same fields with the same values as the first. > > Urk. I did not know about that. What a gotcha! > > So it *could* technically be OK if the fields in the struct (and > any bson.D) are lexically ordered to match the bson Marshaler, > but well worth avoiding. > > I think things would be considerably improved if mgo/bson preserved > order by default (by using bson.D) when unmarshaling. > Then at least you'd know that the assertion you specify > is exactly the one that gets executed. > > cheers, > rog. > > > > > On 8 June 2016 at 10:42, Menno Smits <[email protected]> wrote: > > > > > > On 8 June 2016 at 21:05, Tim Penhey <[email protected]> wrote: > >> > >> Hi folks, > >> > >> tl;dr: not use structs in transaction asserts > >> > >> ... > >> > >> The solution is to not use a field struct equality, even though it is > easy > >> to write, but to use the dotted field notation to check the embedded > field > >> values. > > > > > > > > To give a more concrete example, asserting on a embedded document field > like > > this is problematic: > > > > ops := []txn.Op{{ > > C: "collection", > > Id: ..., > > Assert: bson.D{{"some-field", Thing{A: "foo", B: 99}}}, > > Update: ... > > } > > > > Due to the way mgo works[1], the document the transaction operation is > > asserting against may have been written with A and B in reverse order, or > > the Thing struct in the Assert may have A and B swapped by the time it's > > used. Either way, the assertion will fail randomly. > > > > The correct approach is to express the assertion like this: > > > > ops := []txn.Op{{ > > C: "collection", > > Id: ..., > > Assert: bson.D{ > > {"some-field.A", "foo"}, > > {"some-field.B", 99}, > > }, > > Update: ... > > } > > > > or this: > > > > ops := []txn.Op{{ > > C: "collection", > > Id: ..., > > Assert: bson.M{ > > "some-field.A": "foo", > > "some-field.B": 99, > > }, > > Update: ... > > } > > > >> > >> Yet another thing to add to the list of things to check when doing > >> reviews. > > > > > > I think we can go a bit further and error on attempts to use structs for > > comparison in txn.Op asserts in Juju's txn layers in state. Just as we > > already do some munging and checking of database operations to ensure > > correct multi-model behaviour, we should be able to do this same for this > > issue and prevent it from happening again. > > > > - Menno > > > > [1] If transaction operations are loaded and used from the DB (more > likely > > under load when multiple runners are acting concurrently), the Insert, > > Update and Assert fields are loaded as bson.M (this is what the bson > > Unmarshaller does for interface{} typed fields). Once this happens field > > ordering is lost. > > > > > > > > > > -- > > Juju-dev mailing list > > [email protected] > > Modify settings or unsubscribe at: > > https://lists.ubuntu.com/mailman/listinfo/juju-dev > > > > -- > Juju-dev mailing list > [email protected] > Modify settings or unsubscribe at: > https://lists.ubuntu.com/mailman/listinfo/juju-dev > -- gustavo @ http://niemeyer.net
-- Juju-dev mailing list [email protected] Modify settings or unsubscribe at: https://lists.ubuntu.com/mailman/listinfo/juju-dev
