[CSHARP-439] Query.In with BsonArray created via IEnumerable<BsonValue> constructor generated wrong Created: 12/Apr/12  Updated: 20/Mar/14  Resolved: 13/Apr/12

Status: Closed
Project: C# Driver
Component/s: None
Affects Version/s: 1.3.1
Fix Version/s: None

Type: Bug Priority: Major - P3
Reporter: Brian Assignee: Craig Wilson
Resolution: Done Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified


 Description   

1) create 2 bson values:
BsonValue convertedValue1 = BsonString.Create("str1");
BsonValue convertedValue2 = BsonString.Create("str2");

2) Create BsonArray: var array = new BsonArray(new[]

{convertedValue1, convertedValue2}

);
3) Create query: Query.ElemMatch("Properties", Query.And(Query.EQ(propertyIdFieldName,"some Id"), Query.In("someProperty", array )));

Generated result:
{ "Properties" : { "$elemMatch" : { "PropertyDefinitionId" : "some Id", "someProperty" :

{ "$in" : [["str1", "str1"]] }

} } }
Expected result:
{ "Properties" : { "$elemMatch" : { "PropertyDefinitionId" : "some Id", "someProperty" :

{ "$in" : ["str1", "str1"] }

} } }



 Comments   
Comment by Craig Wilson [ 13/Apr/12 ]

Brian,
Thanks for persisting with this issue. Your feedback is important and I love that you care so much. Robert asked me to look over this as a second set of eyes.

The real crux of this issue lies around intent. Do you intend a query like this

{ $in: [1,2] }

or like this

{ $in: [[1,2]]}

. Both of these are legal and both of them are used. Because of this, we cannot simply infer your intent and instead must rely on you to be specific. In the csharp api as it is, that specificity comes by passing in an array or a single value. Even more interesting is that this is actually the csharp compiler making this decision on your behalf. Let me illustrate.

The three methods that exist for Query.In are defined as such:

1) public static QueryConditionList In(string name, params BsonValue[] values)

2) public static QueryConditionList In(string name, IEnumerable<BsonValue> values)

3) public static QueryConditionList In(string name, BsonArray values)

Given these, let's see which overload the compiler will choose:

1)
var value = BsonValue.Create(1);
Query.In(value); //this will choose method 1

2)
var value = BsonValue.Create(1);
var value2 = BsonValue.Create(2);
Query.In(value, value2); //this will choose method 1

3)
var value = BsonValue.Create(1);
var value2 = BsonValue.Create(2);
Query.In(new []

{ value, value2 }); //this will choose method 1

4)
var value = BsonValue.Create(1);
var value2 = BsonValue.Create(2);
var values = new [] { value, value 2}.ToList();
Query.In(values); //this will choose method 2

5)
var value = BsonValue.Create(1);
var value2 = BsonValue.Create(2);
var array = new BsonArray(new [] { value, value2 }

);
Query.In(array); //this will choose method 3

6)
var value = BsonValue.Create(1);
var value2 = BsonValue.Create(2);
BsonValue array = new BsonArray(new []

{ value, value2 }

);
Query.In(array); //this will choose method 1

Note that 5 and 6 are different solely in the type of the "array" variable. This isn't the driver making a decision on what to call, it is the compiler choosing a different overload. We need both these to support both the queries

{ $in: [1,2] }

and

{ $in: [[1,2]]}

. Simply because overload 1 is called with a single BsonArray value doesn't mean that you intended to call overload 3. We can't infer that intent and therefore must rely on you to provide us that information.

I'm sorry this is causing some frustration and hope that this helps clear up the reasons why we are reluctant to make a change.

Craig

Comment by Brian [ 13/Apr/12 ]

Environment: Windows 7, MongoDB 2.0.3
Object:

    public abstract class EntityObject
    {
        public string Id { get; set; }
        public Property[] Properties { get; set; }
    }
	
    public abstract class Property
    {
        public string PropertyDefinitionId { get; set; }
    }
 
    public abstract class Property<TValue> : Property
    {
        public List<TValue> Value { get; set; }
    }

Generated query with BsonArray as BsonValue:

{Properties:{$elemMatch:{"PropertyDefinitionId":"test name", "Value":{"$in":[["test name"]]}}}}

Expected query with non empty result set:

{Properties:{$elemMatch:{"PropertyDefinitionId":"test name", "Value":{"$in":["test name"]}}}}

MongoVue 1.5.0.2:
Generated query - empty result set
Expected query - not empty result set

Mongo shell:
Generated query - empty result set
Expected query - not empty result set

Comment by Robert Stam [ 13/Apr/12 ]

I don't think anything is broken. Both queries are valid, they are just different queries. Here's an example in the mongo shell showing the difference between the two. Note that the second query has double brackets after $in.

> db.test.find()
{ "_id" : ObjectId("4f8861e4f014d20bea9cbb72"), "a" : [ 1 ] }
{ "_id" : ObjectId("4f8861ecf014d20bea9cbb73"), "a" : [ 1, 2 ] }
{ "_id" : ObjectId("4f8861eff014d20bea9cbb74"), "a" : [ 1, 2, 3 ] }
{ "_id" : ObjectId("4f8861f5f014d20bea9cbb75"), "a" : [ [ 1, 2 ], 3 ] }
>
> db.test.find({a:{$in:[1,2]}})
{ "_id" : ObjectId("4f8861e4f014d20bea9cbb72"), "a" : [ 1 ] }
{ "_id" : ObjectId("4f8861ecf014d20bea9cbb73"), "a" : [ 1, 2 ] }
{ "_id" : ObjectId("4f8861eff014d20bea9cbb74"), "a" : [ 1, 2, 3 ] }
>
> db.test.find({a:{$in:[[1,2]]}})
{ "_id" : ObjectId("4f8861ecf014d20bea9cbb73"), "a" : [ 1, 2 ] }
{ "_id" : ObjectId("4f8861f5f014d20bea9cbb75"), "a" : [ [ 1, 2 ], 3 ] }
>

So if you want to produce the first query you declare your variable as type BsonArray. If you want to produce the second query you declare your variable as type BsonValue.

Comment by Brian [ 13/Apr/12 ]

The goal of my comment was to show use-case. Of cource I can do all this stuff to handle that situation. But I wanted to show the forest behind the trees: That there is a simple way to broke logic. I easily could create a hard detecting bug assuming that it's a legal operation - there is no exceptions, no invalid cast warning. There is no documentation that says that "When you see input parameter of BsonValue type you can give it everething but if you want to give BsonArray then cast it to BsonArray explicetly and only then give it or you'll got the wrong data because of wrong query generation"

I can handle with part when "AsXXX" operations throws exception instead of null return as all in .net does, but I thing that query generation part must be strict as much as it can be because it's a driver core

Comment by Robert Stam [ 13/Apr/12 ]

You can use a BsonArray with a single value. You don't have to treat the single value case differently. The following is just fine:

var array = new BsonArray { 1 };
var query = Query.In("x", array);

and results in the query:

{ x : { $in : [1] } }

But you may want to handle the single case specially anyway, because for just one value the following is probably faster:

var query = Query.EQ("x", 1)

which results in the query:

{ x : 1 }

If you declare your variable as BsonValue you are saying that you want it to be treated as a single value, not as an array. If you want it treated as an array, declare it as an array.

Comment by Brian [ 13/Apr/12 ]

Use-case: I create query builder from XML and I have a single BsonValue variable to store values for Query.In() and sometimes condition could be single-valued and sometimes there could be few values so I store them as BsonArray and it's comfortable to me - everything is in the one variable. I don't need to think about "If (InputValues.count>1) then

{bsonArray value query logic}

else

{simple BsonValue query logic}

"

So why when query is generated driver doesn't look on the variable real type? I think that if I CAN insert BsonArray as BsonVariable into query builder then driver SHOULD handle this situation properly.

Comment by Robert Stam [ 13/Apr/12 ]

When you declare your array variable as a BsonValue you are explicitly saying that you want to treat your two element array as a single BsonValue.

If you want your array variable to be treated as a BsonArray then you should declare it as a BsonArray.

Comment by Brian [ 13/Apr/12 ]

In sample replace this:
var array = new BsonArray(new[]

{ convertedValue1, convertedValue2 });
on this:
BsonValue array = new BsonArray(new[] { convertedValue1, convertedValue2 }

);

Then this method would be called:
public QueryConditionList In(params BsonValue[] values)
{
this.conditions.Add("$in", new BsonArray(values));
return this;
}
We got new BsonArray with BsonArray => double brackets

Comment by Robert Stam [ 12/Apr/12 ]

I am unable to reproduce this. Here's the code I used:

var propertyIdFieldName = "some Id";
BsonValue convertedValue1 = BsonString.Create("str1");
BsonValue convertedValue2 = BsonString.Create("str2");
var array = new BsonArray(new[] { convertedValue1, convertedValue2 });
var query = Query.ElemMatch("Properties", Query.And(
    Query.EQ(propertyIdFieldName, "some Id"),
    Query.In("someProperty", array)));
Console.WriteLine(query.ToJson());

And this is the query generated:

{ "Properties" : { "$elemMatch" : { "some Id" : "some Id", "someProperty" : { "$in" : ["str1", "str2"] } } } }

Comment by Brian [ 12/Apr/12 ]
{"$in":["str1", "str2"]}

of cource. Sorry for my ctrl+c, ctrl+v

Generated at Wed Feb 07 21:36:50 UTC 2024 using Jira 9.7.1#970001-sha1:2222b88b221c4928ef0de3161136cc90c8356a66.