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.

Sign up for GitHub

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

CSHARP-5356: Support OfType and is with scalar discriminators when using class mapped serializers. #1559

Closed
wants to merge 9 commits into from
Closed

CSHARP-5356: Support OfType and is with scalar discriminators when using class mapped serializers. #1559

Changes from 1 commit
Commits
Show all changes
9 commits
Select commit Hold shift + click to select a range
31273b8
CSHARP-5356: Support `OfType` and `is` with scalar discriminators whe…
rstam Nov 27, 2024
1128d64
CSHARP-5356: Code review changes.
rstam Nov 28, 2024
c048bb4
CSHARP-5356: Code review changes.
rstam Nov 29, 2024
0346c91
CSHARP-5356: Added support for OfType in aggregation expressions.
rstam Nov 29, 2024
0488d2f
CSHARP-5356: Added support for wrapped arrays as stage inputs to OfTy…
rstam Nov 29, 2024
fd329f2
CSHARP-5356: Fix failin tests that were discovered in Evergreen patch…
rstam Nov 29, 2024
00c3a11
CSHARP-5356: Minor changes.
rstam Nov 30, 2024
37d10fe
CSHARP-5356: Code review changes.
rstam Jan 27, 2025
f4033a4
CSHARP-5356: Add quotes around type names in exception messages.
rstam Jan 30, 2025
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Prev Previous commit
Next Next commit
CSHARP-5356: Code review changes.
  • Loading branch information
@rstam
rstam committed Nov 28, 2024

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.
GPG key ID: 4AEE18F83AFDEB23
Expired
Verified
Learn about vigilant mode
commit 1128d649e20fad02daacb2f5fb222d35fc36302f
1 change: 0 additions & 1 deletion src/MongoDB.Bson/Serialization/BsonClassMapScalarDiscriminatorConvention.cs
View file
Original file line number Diff line number Diff line change
@@ -139,7 +139,6 @@ private BsonValue ReadDiscriminator(IBsonReader bsonReader)
var context = BsonDeserializationContext.CreateRoot(bsonReader);
return BsonValueSerializer.Instance.Deserialize(context);
}
bsonReader.ReturnToBookmark(bookmark);
}
finally
{
360 changes: 360 additions & 0 deletions tests/MongoDB.Driver.Tests/Linq/Linq3Implementation/Jira/CSharp5356Tests.cs
View file
Original file line number Diff line number Diff line change
@@ -112,6 +112,366 @@ public void OfType_Snake_should_work()
results.Select(x => x.Id).Should().Equal(3);
}

[Fact]
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Added a whole bunch of new tests, some of which required implementing support for missing features.

public void Where_is_Animal_should_work()
{
var collection = GetCollection();

var queryable = collection.AsQueryable()
.Where(x => x is Animal);

var stages = Translate(collection, queryable);
AssertStages(stages, "{ $match : { } }");
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tried optimizing this stage away but that broke a lot of other tests.

Might be worth discussing and if we want to do it create a new ticket just for that.


var results = queryable.ToList();
results.Select(x => x.Id).Should().Equal(1, 2, 3);
}

[Fact]
public void Where_is_Mammal_should_work()
{
var collection = GetCollection();

var queryable = collection.AsQueryable()
.Where(x => x is Mammal);

var stages = Translate(collection, queryable);
AssertStages(stages, "{ $match : { _t : { $in : ['Cat', 'Dog', 'Mammal'] } } }");

var results = queryable.ToList();
results.Select(x => x.Id).Should().Equal(1, 2);
}

[Fact]
public void Where_is_Cat_should_work()
{
var collection = GetCollection();

var queryable = collection.AsQueryable()
.Where(x => x is Cat);

var stages = Translate(collection, queryable);
AssertStages(stages, "{ $match : { _t : 'Cat' } }");

var results = queryable.ToList();
results.Select(x => x.Id).Should().Equal(1);
}

[Fact]
public void Where_is_Reptile_should_work()
{
var collection = GetCollection();

var queryable = collection.AsQueryable()
.Where(x => x is Reptile);

var stages = Translate(collection, queryable);
AssertStages(stages, "{ $match : { _t : { $in : ['Reptile', 'Snake'] } } }");

var results = queryable.ToList();
results.Select(x => x.Id).Should().Equal(3);
}

[Fact]
public void Where_is_Snake_should_work()
{
var collection = GetCollection();

var queryable = collection.AsQueryable()
.Where(x => x is Snake);

var stages = Translate(collection, queryable);
AssertStages(stages, "{ $match : { _t : 'Snake' } }");

var results = queryable.ToList();
results.Select(x => x.Id).Should().Equal(3);
}

[Fact]
public void Where_not_is_Animal_should_work()
{
var collection = GetCollection();

var queryable = collection.AsQueryable()
.Where(x => !(x is Animal));

var stages = Translate(collection, queryable);
AssertStages(stages, "{ $match : { _id : { $type : -1 } } }");

var results = queryable.ToList();
results.Select(x => x.Id).Should().Equal();
}

[Fact]
public void Where_not_is_Mammal_should_work()
{
var collection = GetCollection();

var queryable = collection.AsQueryable()
.Where(x => !(x is Mammal));

var stages = Translate(collection, queryable);
AssertStages(stages, "{ $match : { _t : { $nin : ['Cat', 'Dog', 'Mammal'] } } }");

var results = queryable.ToList();
results.Select(x => x.Id).Should().Equal(3);
}

[Fact]
public void Where_not_is_Cat_should_work()
{
var collection = GetCollection();

var queryable = collection.AsQueryable()
.Where(x => !(x is Cat));

var stages = Translate(collection, queryable);
AssertStages(stages, "{ $match : { _t : { $ne : 'Cat' } } }");

var results = queryable.ToList();
results.Select(x => x.Id).Should().Equal(2, 3);
}

[Fact]
public void Where_not_is_Reptile_should_work()
{
var collection = GetCollection();

var queryable = collection.AsQueryable()
.Where(x => !(x is Reptile));

var stages = Translate(collection, queryable);
AssertStages(stages, "{ $match : { _t : { $nin : ['Reptile', 'Snake'] } } }");

var results = queryable.ToList();
results.Select(x => x.Id).Should().Equal(1, 2);
}

[Fact]
public void Where_not_is_Snake_should_work()
{
var collection = GetCollection();

var queryable = collection.AsQueryable()
.Where(x => !(x is Snake));

var stages = Translate(collection, queryable);
AssertStages(stages, "{ $match : { _t : { $ne : 'Snake' } } }");

var results = queryable.ToList();
results.Select(x => x.Id).Should().Equal(1, 2);
}

[Fact]
public void Array_Where_is_Animal_should_work()
{
var collection = GetCollection();

var queryable = collection.AsQueryable()
.GroupBy(x => 1)
.Select(x => new { A = x.ToArray() })
.Select(x => new { A = x.A.Where(x => x is Animal).ToArray() });

var stages = Translate(collection, queryable);
AssertStages(
stages,
"{ $group : { _id : 1, _elements : { $push : '$$ROOT' } } }",
"{ $project : { A : '$_elements', _id : 0 } }",
"{ $project : { A : { $filter : { input : '$A', as : 'x', cond : true } }, _id : 0 } }");
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Potential MQL optimization here since cond : true.

Is it worth it?


var result = queryable.Single();
result.A.Select(x => x.Id).Should().Equal(1, 2, 3);
}

[Fact]
public void Array_Where_is_Mammal_should_work()
{
var collection = GetCollection();

var queryable = collection.AsQueryable()
.GroupBy(x => 1)
.Select(x => new { A = x.ToArray() })
.Select(x => new { A = x.A.Where(x => x is Mammal).ToArray() });

var stages = Translate(collection, queryable);
AssertStages(
stages,
"{ $group : { _id : 1, _elements : { $push : '$$ROOT' } } }",
"{ $project : { A : '$_elements', _id : 0 } }",
"{ $project : { A : { $filter : { input : '$A', as : 'x', cond : { $in : ['$$x._t', ['Cat', 'Dog', 'Mammal']] } } }, _id : 0 } }");

var result = queryable.Single();
result.A.Select(x => x.Id).Should().Equal(1, 2);
}

[Fact]
public void Array_Where_is_Cat_should_work()
{
var collection = GetCollection();

var queryable = collection.AsQueryable()
.GroupBy(x => 1)
.Select(x => new { A = x.ToArray() })
.Select(x => new { A = x.A.Where(x => x is Cat).ToArray() });

var stages = Translate(collection, queryable);
AssertStages(
stages,
"{ $group : { _id : 1, _elements : { $push : '$$ROOT' } } }",
"{ $project : { A : '$_elements', _id : 0 } }",
"{ $project : { A : { $filter : { input : '$A', as : 'x', cond : { $eq : ['$$x._t', 'Cat'] } } }, _id : 0 } }");

var result = queryable.Single();
result.A.Select(x => x.Id).Should().Equal(1);
}

[Fact]
public void Array_Where_is_Reptile_should_work()
{
var collection = GetCollection();

var queryable = collection.AsQueryable()
.GroupBy(x => 1)
.Select(x => new { A = x.ToArray() })
.Select(x => new { A = x.A.Where(x => x is Reptile).ToArray() });

var stages = Translate(collection, queryable);
AssertStages(
stages,
"{ $group : { _id : 1, _elements : { $push : '$$ROOT' } } }",
"{ $project : { A : '$_elements', _id : 0 } }",
"{ $project : { A : { $filter : { input : '$A', as : 'x', cond : { $in : ['$$x._t', ['Reptile', 'Snake']] } } }, _id : 0 } }");

var result = queryable.Single();
result.A.Select(x => x.Id).Should().Equal(3);
}

[Fact]
public void Array_Where_is_Snake_should_work()
{
var collection = GetCollection();

var queryable = collection.AsQueryable()
.GroupBy(x => 1)
.Select(x => new { A = x.ToArray() })
.Select(x => new { A = x.A.Where(x => x is Snake).ToArray() });

var stages = Translate(collection, queryable);
AssertStages(
stages,
"{ $group : { _id : 1, _elements : { $push : '$$ROOT' } } }",
"{ $project : { A : '$_elements', _id : 0 } }",
"{ $project : { A : { $filter : { input : '$A', as : 'x', cond : { $eq : ['$$x._t', 'Snake'] } } }, _id : 0 } }");

var result = queryable.Single();
result.A.Select(x => x.Id).Should().Equal(3);
}

[Fact]
public void Array_Where_not_is_Animal_should_work()
{
var collection = GetCollection();

var queryable = collection.AsQueryable()
.GroupBy(x => 1)
.Select(x => new { A = x.ToArray() })
.Select(x => new { A = x.A.Where(x => !(x is Animal)).ToArray() });

var stages = Translate(collection, queryable);
AssertStages(
stages,
"{ $group : { _id : 1, _elements : { $push : '$$ROOT' } } }",
"{ $project : { A : '$_elements', _id : 0 } }",
"{ $project : { A : { $filter : { input : '$A', as : 'x', cond : { $not : true } } }, _id : 0 } }");
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was surprised to see { $not : true } instead of false.

Is it worth simplifying?


var result = queryable.Single();
result.A.Select(x => x.Id).Should().Equal();
}

[Fact]
public void Array_Where_not_is_Mammal_should_work()
{
var collection = GetCollection();

var queryable = collection.AsQueryable()
.GroupBy(x => 1)
.Select(x => new { A = x.ToArray() })
.Select(x => new { A = x.A.Where(x => !(x is Mammal)).ToArray() });

var stages = Translate(collection, queryable);
AssertStages(
stages,
"{ $group : { _id : 1, _elements : { $push : '$$ROOT' } } }",
"{ $project : { A : '$_elements', _id : 0 } }",
"{ $project : { A : { $filter : { input : '$A', as : 'x', cond : { $not : { $in : ['$$x._t', ['Cat', 'Dog', 'Mammal']] } } } }, _id : 0 } }");
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I was expecting to see $nin but turns out the server doesn't have $nin in aggregation expressions (it does have it in filters).


var result = queryable.Single();
result.A.Select(x => x.Id).Should().Equal(3);
}

[Fact]
public void Array_Where_not_is_Cat_should_work()
{
var collection = GetCollection();

var queryable = collection.AsQueryable()
.GroupBy(x => 1)
.Select(x => new { A = x.ToArray() })
.Select(x => new { A = x.A.Where(x => !(x is Cat)).ToArray() });

var stages = Translate(collection, queryable);
AssertStages(
stages,
"{ $group : { _id : 1, _elements : { $push : '$$ROOT' } } }",
"{ $project : { A : '$_elements', _id : 0 } }",
"{ $project : { A : { $filter : { input : '$A', as : 'x', cond : { $not : { $eq : ['$$x._t', 'Cat'] } } } }, _id : 0 } }");
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think $not/$eq could be simplified to $ne.

We'd have to convince ourselves that there are no edge cases where the results would differ.


var result = queryable.Single();
result.A.Select(x => x.Id).Should().Equal(2, 3);
}

[Fact]
public void Array_Where_not_is_Reptile_should_work()
{
var collection = GetCollection();

var queryable = collection.AsQueryable()
.GroupBy(x => 1)
.Select(x => new { A = x.ToArray() })
.Select(x => new { A = x.A.Where(x => !(x is Reptile)).ToArray() });

var stages = Translate(collection, queryable);
AssertStages(
stages,
"{ $group : { _id : 1, _elements : { $push : '$$ROOT' } } }",
"{ $project : { A : '$_elements', _id : 0 } }",
"{ $project : { A : { $filter : { input : '$A', as : 'x', cond : { $not : { $in : ['$$x._t', ['Reptile', 'Snake']] } } } }, _id : 0 } }");

var result = queryable.Single();
result.A.Select(x => x.Id).Should().Equal(1, 2);
}

[Fact]
public void Array_Where_not_is_Snake_should_work()
{
var collection = GetCollection();

var queryable = collection.AsQueryable()
.GroupBy(x => 1)
.Select(x => new { A = x.ToArray() })
.Select(x => new { A = x.A.Where(x => !(x is Snake)).ToArray() });

var stages = Translate(collection, queryable);
AssertStages(
stages,
"{ $group : { _id : 1, _elements : { $push : '$$ROOT' } } }",
"{ $project : { A : '$_elements', _id : 0 } }",
"{ $project : { A : { $filter : { input : '$A', as : 'x', cond : { $not : { $eq : ['$$x._t', 'Snake'] } } } }, _id : 0 } }");

var result = queryable.Single();
result.A.Select(x => x.Id).Should().Equal(1, 2);
}

private IMongoCollection<Animal> GetCollection()
{
var collection = GetCollection<Animal>("test");